[Xen-devel] [PATCH v2 4/4] x86/vvmx: correctly emulate VMREAD

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

There is an issue with the original __vmread() in nested vmx mode:
emulation of a guest's VMREAD with invalid arguments leads to BUG().

Fix this by using vmread_safe() and reporting any kind of VMfail back
to the guest.

A new safe versions of get_vvmcs() macro and related functions are
introduced because of new function signatures and lots of existing
users.

Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
---
 xen/arch/x86/hvm/vmx/vmcs.c        |  9 +++++----
 xen/arch/x86/hvm/vmx/vvmx.c        | 26 +++++++++++++++++++++++---
 xen/include/asm-x86/hvm/vmx/vmcs.h |  3 ++-
 xen/include/asm-x86/hvm/vmx/vvmx.h |  7 +++++++
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 1d83a69..e04b002 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -932,15 +932,16 @@ void virtual_vmcs_exit(const struct vcpu *v)
         __vmptrld(cur);
 }
 
-u64 virtual_vmcs_vmread(const struct vcpu *v, u32 vmcs_encoding)
+unsigned long virtual_vmcs_vmread(const struct vcpu *v, u32 vmcs_encoding,
+                                  u64 *val)
 {
-    u64 res;
+    unsigned long ret;
 
     virtual_vmcs_enter(v);
-    __vmread(vmcs_encoding, &res);
+    ret = vmread_safe(vmcs_encoding, val);
     virtual_vmcs_exit(v);
 
-    return res;
+    return ret;
 }
 
 unsigned long virtual_vmcs_vmwrite(const struct vcpu *v, u32 vmcs_encoding,
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 1a3d1d2..6fda44a 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -261,7 +261,23 @@ u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
 
 u64 get_vvmcs_real(const struct vcpu *v, u32 encoding)
 {
-    return virtual_vmcs_vmread(v, encoding);
+    u64 val;
+
+    virtual_vmcs_vmread(v, encoding, &val);
+
+    return val;
+}
+
+unsigned long get_vvmcs_virtual_safe(void *vvmcs, u32 encoding, u64 *val)
+{
+    *val = get_vvmcs_virtual(vvmcs, encoding);
+
+    return 0;
+}
+
+unsigned long get_vvmcs_real_safe(const struct vcpu *v, u32 encoding, u64 *val)
+{
+    return virtual_vmcs_vmread(v, encoding, val);
 }
 
 unsigned long set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
@@ -1710,13 +1726,17 @@ int nvmx_handle_vmread(struct cpu_user_regs *regs)
     struct vmx_inst_decoded decode;
     pagefault_info_t pfinfo;
     u64 value = 0;
-    int rc;
+    unsigned long rc;
 
     rc = decode_vmx_inst(regs, &decode, NULL, 0);
     if ( rc != X86EMUL_OKAY )
         return rc;
 
-    value = get_vvmcs(v, reg_read(regs, decode.reg2));
+    if ( (rc = get_vvmcs_safe(v, reg_read(regs, decode.reg2), &value)) )
+    {
+        vmfail(regs, rc);
+        return X86EMUL_OKAY;
+    }
 
     switch ( decode.type ) {
     case VMX_INST_MEMREG_TYPE_MEMORY:
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 72dc5fc..4f16250 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -540,7 +540,8 @@ void vmx_clear_eoi_exit_bitmap(struct vcpu *v, u8 vector);
 int vmx_check_msr_bitmap(unsigned long *msr_bitmap, u32 msr, int access_type);
 void virtual_vmcs_enter(const struct vcpu *);
 void virtual_vmcs_exit(const struct vcpu *);
-u64 virtual_vmcs_vmread(const struct vcpu *, u32 encoding);
+unsigned long virtual_vmcs_vmread(const struct vcpu *v, u32 vmcs_encoding,
+                                  u64 *val);
 unsigned long virtual_vmcs_vmwrite(const struct vcpu *, u32 encoding, u64 val);
 
 static inline int vmx_add_guest_msr(u32 msr)
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h 
b/xen/include/asm-x86/hvm/vmx/vvmx.h
index d60e0bb..28e2503 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -181,6 +181,8 @@ enum vvmcs_encoding_type {
 
 u64 get_vvmcs_virtual(void *vvmcs, u32 encoding);
 u64 get_vvmcs_real(const struct vcpu *, u32 encoding);
+unsigned long get_vvmcs_virtual_safe(void *vvmcs, u32 encoding, u64 *val);
+unsigned long get_vvmcs_real_safe(const struct vcpu *, u32 encoding, u64 *val);
 unsigned long set_vvmcs_virtual(void *vvmcs, u32 encoding, u64 val);
 unsigned long set_vvmcs_real(const struct vcpu *, u32 encoding, u64 val);
 
@@ -194,6 +196,11 @@ unsigned long set_vvmcs_real(const struct vcpu *, u32 
encoding, u64 val);
    set_vvmcs_real(vcpu, encoding, val) : \
    set_vvmcs_virtual(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
 
+#define get_vvmcs_safe(vcpu, encoding, val) \
+  (cpu_has_vmx_vmcs_shadowing ? \
+   get_vvmcs_real_safe(vcpu, encoding, val) : \
+   get_vvmcs_virtual_safe(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
+
 uint64_t get_shadow_eptp(struct vcpu *v);
 
 void nvmx_destroy_vmcs(struct vcpu *v);
-- 
2.9.3


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel