1. 10 Mar, 2021 4 commits
  2. 09 Mar, 2021 3 commits
  3. 08 Mar, 2021 1 commit
  4. 06 Mar, 2021 1 commit
  5. 05 Mar, 2021 1 commit
  6. 04 Mar, 2021 5 commits
  7. 03 Mar, 2021 6 commits
  8. 02 Mar, 2021 6 commits
    • Jan Beulich's avatar
      x86/shadow: replace bogus return path in shadow_get_page_from_l1e() · 48349365
      Jan Beulich authored
      Prior to be640b18 ("x86: make get_page_from_l1e() return a proper
      error code") a positive return value did indicate an error. Said commit
      failed to adjust this return path, but luckily the only caller has
      always been inside a shadow_mode_refcounts() conditional.
      
      Subsequent changes caused 1 to end up at the default (error) label in
      the caller's switch() again, but the returning of 1 (== _PAGE_PRESENT)
      is still rather confusing here, and a latent risk.
      
      Convert to an ASSERT() instead, just in case any new caller would
      appear.
      Signed-off-by: default avatarJan Beulich <jbeulich@suse.com>
      Reviewed-by: default avatarAndrew Cooper <andrew.cooper3@citrix.com>
      Acked-by: default avatarTim Deegan <tim@xen.org>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      48349365
    • Jan Beulich's avatar
      sched: fix build when NR_CPUS == 1 · 2de43f83
      Jan Beulich authored
      In this case the compiler is recognizing that no valid array indexes
      remain, and hence e.g. reports:
      
      core.c: In function 'cpu_schedule_up':
      core.c:2769:19: error: array subscript 1 is above array bounds
      of 'struct vcpu *[1]' [-Werror=array-bounds]
       2769 |     if ( idle_vcpu[cpu] == NULL )
            |          ~~~~~~~~~^~~~~
      Reported-by: default avatarConnor Davis <connojdavis@gmail.com>
      Signed-off-by: default avatarJan Beulich <jbeulich@suse.com>
      Reviewed-by: default avatarDario Faggioli <dfaggioli@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      2de43f83
    • Julien Grall's avatar
      xen/iommu: x86: Clear the root page-table before freeing the page-tables · 9bd9695a
      Julien Grall authored
      The new per-domain IOMMU page-table allocator will now free the
      page-tables when domain's resources are relinquished. However, the
      per-domain IOMMU structure will still contain a dangling pointer to
      the root page-table.
      
      Xen may access the IOMMU page-tables afterwards at least in the case of
      PV domain:
      
      (XEN) Xen call trace:
      (XEN)    [<ffff82d04025b4b2>] R iommu.c#addr_to_dma_page_maddr+0x12e/0x1d8
      (XEN)    [<ffff82d04025b695>] F iommu.c#intel_iommu_unmap_page+0x5d/0xf8
      (XEN)    [<ffff82d0402695f3>] F iommu_unmap+0x9c/0x129
      (XEN)    [<ffff82d0402696a6>] F iommu_legacy_unmap+0x26/0x63
      (XEN)    [<ffff82d04033c5c7>] F mm.c#cleanup_page_mappings+0x139/0x144
      (XEN)    [<ffff82d04033c61d>] F put_page+0x4b/0xb3
      (XEN)    [<ffff82d04033c87f>] F put_page_from_l1e+0x136/0x13b
      (XEN)    [<ffff82d04033cada>] F devalidate_page+0x256/0x8dc
      (XEN)    [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
      (XEN)    [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
      (XEN)    [<ffff82d04033d8d6>] F mm.c#put_page_from_l2e+0x8a/0xcf
      (XEN)    [<ffff82d04033cc27>] F devalidate_page+0x3a3/0x8dc
      (XEN)    [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
      (XEN)    [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
      (XEN)    [<ffff82d04033d807>] F mm.c#put_page_from_l3e+0x8a/0xcf
      (XEN)    [<ffff82d04033cdf0>] F devalidate_page+0x56c/0x8dc
      (XEN)    [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
      (XEN)    [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
      (XEN)    [<ffff82d04033d6c7>] F mm.c#put_page_from_l4e+0x69/0x6d
      (XEN)    [<ffff82d04033cf24>] F devalidate_page+0x6a0/0x8dc
      (XEN)    [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
      (XEN)    [<ffff82d04033d92e>] F put_page_type_preemptible+0x13/0x15
      (XEN)    [<ffff82d04032598a>] F domain.c#relinquish_memory+0x1ff/0x4e9
      (XEN)    [<ffff82d0403295f2>] F domain_relinquish_resources+0x2b6/0x36a
      (XEN)    [<ffff82d040205cdf>] F domain_kill+0xb8/0x141
      (XEN)    [<ffff82d040236cac>] F do_domctl+0xb6f/0x18e5
      (XEN)    [<ffff82d04031d098>] F pv_hypercall+0x2f0/0x55f
      (XEN)    [<ffff82d04039b432>] F lstar_enter+0x112/0x120
      
      This will result to a use after-free and possibly an host crash or
      memory corruption.
      
      It would not be possible to free the page-tables further down in
      domain_relinquish_resources() because cleanup_page_mappings() will only
      be called when the last reference on the page dropped. This may happen
      much later if another domain still hold a reference.
      
      After all the PCI devices have been de-assigned, nobody should use the
      IOMMU page-tables and it is therefore pointless to try to modify them.
      
      So we can simply clear any reference to the root page-table in the
      per-domain IOMMU structure. This requires to introduce a new callback of
      the method will depend on the IOMMU driver used.
      
      Take the opportunity to add an ASSERT() in arch_iommu_domain_destroy()
      to check if we freed all the IOMMU page tables.
      
      Fixes: 3eef6d07 ("x86/iommu: convert VT-d code to use new page table allocator")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      9bd9695a
    • Julien Grall's avatar
      xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying · 1ef48c82
      Julien Grall authored
      The new x86 IOMMU page-tables allocator will release the pages when
      relinquishing the domain resources. However, this is not sufficient
      when the domain is dying because nothing prevents page-table to be
      allocated.
      
      As the domain is dying, it is not necessary to continue to modify the
      IOMMU page-tables as they are going to be destroyed soon.
      
      At the moment, page-table allocates will only happen when iommu_map().
      So after this change there will be no more page-table allocation
      happening because we don't use superpage mappings yet when not sharing
      page tables.
      
      In order to observe d->is_dying correctly, we need to rely on per-arch
      locking, so the check to ignore IOMMU mapping is added on the per-driver
      map_page() callback.
      
      Fixes: 15bc9a1e ("x86/iommu: add common page-table allocator")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Reviewed-by: default avatarKevin Tian <kevin.tian@intel.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      1ef48c82
    • Julien Grall's avatar
      xen/iommu: x86: Don't try to free page tables is the IOMMU is not enabled · f4cf483e
      Julien Grall authored
      When using CONFIG_BIGMEM=y, the page_list cannot be accessed whilst it
      is is unitialized. However, iommu_free_pgtables() will be called even if
      the domain is not using an IOMMU.
      
      Consequently, Xen will try to go through the page list and deference a
      NULL pointer.
      
      Bail out early if the domain is not using an IOMMU.
      
      Fixes: 15bc9a1e ("x86/iommu: add common page-table allocator")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      f4cf483e
    • Julien Grall's avatar
      tools/xenstored: Avoid dereferencing a NULL pointer if LiveUpdate is failing · 29fae90b
      Julien Grall authored
      In case of failure in do_lu_start(), XenStored will first free lu_start
      and then try to dereference it.
      
      This will result to a NULL dereference as the destruction callback will
      set lu_start to NULL.
      
      The crash can be avoided by freeing lu_start *after* the reply has been
      set.
      
      Fixes: af216a99 ("tools/xenstore: add the basic framework for doing the live update")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      29fae90b
  9. 01 Mar, 2021 6 commits
  10. 26 Feb, 2021 7 commits
    • Andrew Cooper's avatar
      dmop: Add XEN_DMOP_nr_vcpus · c4441ab1
      Andrew Cooper authored
      Curiously absent from the stable API/ABIs is an ability to query the number of
      vcpus which a domain has.  Emulators need to know this information in
      particular to know how many stuct ioreq's live in the ioreq server mappings.
      
      In practice, this forces all userspace to link against libxenctrl to use
      xc_domain_getinfo(), which rather defeats the purpose of the stable libraries.
      
      Introduce a DMOP to retrieve this information and surface it in
      libxendevicemodel to help emulators shed their use of unstable interfaces.
      Signed-off-by: default avatarAndrew Cooper <andrew.cooper3@citrix.com>
      Reviewed-by: default avatarIan Jackson <iwj@xenproject.org>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      ---
      CC: Jan Beulich <JBeulich@suse.com>
      CC: Roger Pau Monné <roger.pau@citrix.com>
      CC: Wei Liu <wl@xen.org>
      CC: Paul Durrant <paul@xen.org>
      CC: Stefano Stabellini <sstabellini@kernel.org>
      CC: Julien Grall <julien@xen.org>
      CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
      CC: Ian Jackson <iwj@xenproject.org>
      
      For 4.15.  This was a surprise discovery in the massive ABI untangling effort
      I'm currently doing for XenServer's new build system.
      
      This is one new read-only op to obtain information which isn't otherwise
      available under a stable API/ABI.  As such, its risk for 4.15 is very low,
      with a very real quality-of-life improvement for downstreams.
      
      I realise this is technically a new feature and we're long past feature
      freeze, but I'm hoping that "really lets some emulators move off the unstable
      libraries" is sufficiently convincing argument.
      
      It's not sufficient to let Qemu move off unstable libraries yet - at a
      minimum, the add_to_phymap hypercalls need stabilising to support PCI
      Passthrough and BAR remapping.
      
      I'd prefer not to duplicate the op handling between ARM and x86, and if this
      weren't a release window, I'd submit a prereq patch to dedup the common dmop
      handling.  That can wait to 4.16 at this point.  Also, this op ought to work
      against x86 PV guests, but fixing that up will also need this rearrangement
      into common code, so needs to wait.
      c4441ab1
    • Andrew Cooper's avatar
      x86/dmop: Properly fail for PV guests · 615367b5
      Andrew Cooper authored
      The current code has an early exit for PV guests, but it returns 0 having done
      nothing.
      
      Fixes: 524a98c2 ("public / x86: introduce __HYPERCALL_dm_op...")
      Signed-off-by: default avatarAndrew Cooper <andrew.cooper3@citrix.com>
      Reviewed-by: default avatarIan Jackson <iwj@xenproject.org>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      615367b5
    • Julien Grall's avatar
      xen/sched: Add missing memory barrier in vcpu_block() · 109e8177
      Julien Grall authored
      The comment in vcpu_block() states that the events should be checked
      /after/ blocking to avoids wakeup waiting race. However, from a generic
      perspective, set_bit() doesn't prevent re-ordering. So the following
      could happen:
      
      CPU0  (blocking vCPU A)         |   CPU1 ( unblock vCPU A)
                                      |
      A <- read local events          |
                                      |   set local events
                                      |   test_and_clear_bit(_VPF_blocked)
                                      |       -> Bail out as the bit if not set
                                      |
      set_bit(_VFP_blocked)           |
                                      |
      check A                         |
      
      The variable A will be 0 and therefore the vCPU will be blocked when it
      should continue running.
      
      vcpu_block() is now gaining an smp_mb__after_atomic() to prevent the CPU
      to read any information about local events before the flag _VPF_blocked
      is set.
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
      Reviewed-by: default avatarAsh Wilding <ash.j.wilding@gmail.com>
      Acked-by: default avatarStefano Stabellini <sstabellini@kernel.org>
      Acked-by: default avatarDario Faggioli <dfaggioli@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      109e8177
    • Julien Grall's avatar
      tools/xenstore-control: Don't leak buf in live_update_start() · 79b6574f
      Julien Grall authored
      All the error paths but one will free buf. Cover the remaining path so
      buf can't be leaked.
      
      This bug was discovered and resolved using Coverity Static Analysis
      Security Testing (SAST) by Synopsys, Inc.
      
      Fixes: 7f97193e ("tools/xenstore: add live update command to xenstore-control")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      79b6574f
    • Julien Grall's avatar
      tools/xenstored: control: Store the save filename in lu_dump_state · 11d9933f
      Julien Grall authored
      The function lu_close_dump_state() will use talloc_asprintf() without
      checking whether the allocation succeeded. In the unlikely case we are
      out of memory, we would dereference a NULL pointer.
      
      As we already computed the filename in lu_get_dump_state(), we can store
      the name in the lu_dump_state. This is avoiding to deal with memory file
      in the close path and also reduce the risk to use the different
      filename.
      
      This bug was discovered and resolved using Coverity Static Analysis
      Security Testing (SAST) by Synopsys, Inc.
      
      Fixes: c0dc6a3e ("tools/xenstore: read internal state when doing live upgrade")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      11d9933f
    • Julien Grall's avatar
      tools/xenstored: Avoid unnecessary talloc_strdup() in do_lu_start() · 702b44be
      Julien Grall authored
      At the moment, the return of talloc_strdup() is not checked. This means
      we may dereference a NULL pointer if the allocation failed.
      
      However, it is pointless to allocate the memory as send_reply() will
      copy the data to a different buffer. So drop the use of talloc_strdup().
      
      This bug was discovered and resolved using Coverity Static Analysis
      Security Testing (SAST) by Synopsys, Inc.
      
      Fixes: af216a99 ("tools/xenstore: add the basic framework for doing the live update")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      702b44be
    • Julien Grall's avatar
      tools/xenstored: Avoid unnecessary talloc_strdup() in do_control_lu() · 2fc7939e
      Julien Grall authored
      At the moment, the return of talloc_strdup() is not checked. This means
      we may dereference a NULL pointer if the allocation failed.
      
      However, it is pointless to allocate the memory as send_reply() will
      copy the data to a different buffer. So drop the use of talloc_strdup().
      
      This bug was discovered and resolved using Coverity Static Analysis
      Security Testing (SAST) by Synopsys, Inc.
      
      Fixes: fecab256 ("tools/xenstore: add basic live-update command parsing")
      Signed-off-by: default avatarJulien Grall <jgrall@amazon.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Release-Acked-by: default avatarIan Jackson <iwj@xenproject.org>
      2fc7939e