[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v1] x86/nSVM: Check the L1 IOPM_BASE and MSRPM_BASE assigned physical addresses



The Xen nested virtualization code maps the physical addresses assigned by
the L1 guests, for IOPM_BASE and MSRPM_BASE directly to valid host addresses
without sanity checks. Add sanity checks to verify the L1 assigned addresses
are valid guest addresses. This check also makes the bahavior compliant with
the Hardware handling of the assigned addresses. The hardware is expected to
cause VMEXIT_INVALID with IOPM_BASE and MSRPM_BASE addresses greater than or
equal to the maximum supported physical address, see[1].

While at it, clean up the code. Remove the unused bool viopm and the
svm_vcpu::ns_oiomap_pa. 

[1] https://docs.amd.com/v/u/en-US/24593_3.44_APM_Vol2

Signed-off-by: Abdelkareem Abdelsaamad <abdelkareem.abdelsaamad@xxxxxxxxxx>
---
Testing:
 - Using a locally developed XTF nested virt setup, I manually tested VMRUN
   instruction handling with the address value (0xffffffffffffffffUL) assigned
   to VMCB::iopm_base_pa:
   - Without the changes the address is mapped by the Xen code to the address
     0x604b634000 and it completes the execution without any reported errors.
   - With the changes, the VMRUN execution fails VMEXIT_INVALID is reported
     back in the ns_vmexit.exitcode.
 - CI tests:
https://gitlab.com/xen-project/people/aabdelsa/xen/-/pipelines/2709640880
---
 xen/arch/x86/hvm/svm/nestedsvm.c         | 24 +++++++++++++++++++++---
 xen/arch/x85/include/asm/hvm/svm-types.h |  2 +-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index b06124c2c9..25b91a6b07 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -18,6 +18,8 @@
 
 #define NSVM_ERROR_VVMCB        1
 #define NSVM_ERROR_VMENTRY      2
+#define IOPM_MAX_PAGES_DIFF     2
+#define MSRPM_MAX_PAGES_DIFF    1
 
 int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr)
 {
@@ -282,7 +284,7 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct 
cpu_user_regs *regs)
     return 0;
 }
 
-static int nsvm_vmrun_permissionmap(struct vcpu *v, bool viopm)
+static int nsvm_vmrun_permissionmap(struct vcpu *v)
 {
     struct svm_vcpu *arch_svm = &v->arch.hvm.svm;
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
@@ -295,6 +297,23 @@ static int nsvm_vmrun_permissionmap(struct vcpu *v, bool 
viopm)
     unsigned long *ns_viomap;
     bool ioport_80 = true, ioport_ed = true;
 
+    if ( (domain_get_maximum_gpfn(v->domain) < IOPM_MAX_PAGES_DIFF) ||
+         (gfn_x(_gfn(ns_vmcb->_iopm_base_pa >> PAGE_SHIFT)) >
+          (domain_get_maximum_gpfn(v->domain) - IOPM_MAX_PAGES_DIFF)) )
+    {
+        gdprintk(XENLOG_ERR, "nsvm_vmrun_permissionmap invalid _iopm_base_pa"
+                 " address (%#"PRIx64")\n", ns_vmcb->_iopm_base_pa);
+        return 1;
+    }
+
+    if ( (gfn_x(_gfn(ns_vmcb->_msrpm_base_pa >> PAGE_SHIFT)) >
+          (domain_get_maximum_gpfn(v->domain) - MSRPM_MAX_PAGES_DIFF)) )
+    {
+        gdprintk(XENLOG_ERR, "nsvm_vmrun_permissionmap invalid _msrpm_base_pa"
+                " address (%#"PRIx64")\n", ns_vmcb->_msrpm_base_pa);
+        return 1;
+    }
+
     ns_msrpm_ptr = (unsigned long *)svm->ns_cached_msrpm;
 
     ret = hvm_copy_from_guest_phys(svm->ns_cached_msrpm,
@@ -308,7 +327,6 @@ static int nsvm_vmrun_permissionmap(struct vcpu *v, bool 
viopm)
     /* Check l1 guest io permission map and get a shadow one based on
      * if l1 guest intercepts io ports 0x80 and/or 0xED.
      */
-    svm->ns_oiomap_pa = svm->ns_iomap_pa;
     svm->ns_iomap_pa = ns_vmcb->_iopm_base_pa;
 
     ns_viomap = hvm_map_guest_frame_ro(svm->ns_iomap_pa >> PAGE_SHIFT, 0);
@@ -418,7 +436,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct 
cpu_user_regs *regs)
     n2vmcb->_tsc_offset = n1vmcb->_tsc_offset + ns_vmcb->_tsc_offset;
 
     /* Nested IO permission bitmaps */
-    rc = nsvm_vmrun_permissionmap(v, clean.iopm);
+    rc = nsvm_vmrun_permissionmap(v);
     if ( rc )
         return rc;
 
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h 
b/xen/arch/x86/include/asm/hvm/svm-types.h
index 8acadb9dcc..beab9a3af2 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -51,7 +51,7 @@ struct nestedsvm {
     unsigned long *ns_merged_msrpm;
 
     /* guest physical address of virtual io permission map */
-    paddr_t ns_iomap_pa, ns_oiomap_pa;
+    paddr_t ns_iomap_pa;
     /* Shadow io permission map */
     unsigned long *ns_iomap;
 
-- 
2.53.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.