[Xen-devel] [PATCH 1/4] x86/vmx: introduce __vmwrite_safe()

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

Any fail during the original __vmwrite() leads to BUG() which can be
easily exploited from a guest in the nested vmx mode.

The new function returns error code depending on the outcome:

          VMsucceed: 0
        VMfailValid: VM Instruction Error Number
      VMfailInvalid: a new VMX_INSN_FAIL_INVALID

A new macro GAS_VMX_OP is introduced in order to improve the readability
of asm.  Existing ASM_FLAG_OUT macro is reused and moved into lib.h
and x86_emulate.h

Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
---
 tools/tests/x86_emulator/x86_emulate.h |  6 ++++++
 xen/arch/x86/hvm/vmx/vvmx.c            |  3 ++-
 xen/arch/x86/x86_emulate/x86_emulate.c |  6 ------
 xen/include/asm-x86/hvm/vmx/vmcs.h     |  1 +
 xen/include/asm-x86/hvm/vmx/vmx.h      | 29 +++++++++++++++++++++++++++++
 xen/include/xen/lib.h                  |  6 ++++++
 6 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/tools/tests/x86_emulator/x86_emulate.h 
b/tools/tests/x86_emulator/x86_emulate.h
index 3a6bade..6524bb3 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -46,6 +46,12 @@
 #define MMAP_SZ 16384
 bool emul_test_make_stack_executable(void);
 
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+# define ASM_FLAG_OUT(yes, no) yes
+#else
+# define ASM_FLAG_OUT(yes, no) no
+#endif
+
 #include "x86_emulate/x86_emulate.h"
 
 static inline uint64_t xgetbv(uint32_t xcr)
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 9caebe5..105a3c0 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -483,7 +483,8 @@ static void vmfail_invalid(struct cpu_user_regs *regs)
 
 static void vmfail(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
 {
-    if ( vcpu_nestedhvm(current).nv_vvmcxaddr != INVALID_PADDR )
+    if ( vcpu_nestedhvm(current).nv_vvmcxaddr != INVALID_PADDR &&
+         errno != VMX_INSN_FAIL_INVALID )
         vmfail_valid(regs, errno);
     else
         vmfail_invalid(regs);
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 5bb5bdf..d133f09 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -409,12 +409,6 @@ typedef union {
         (void *)((long)(__##var + __alignof(type) - __alignof(long)) \
                  & -__alignof(type))
 
-#ifdef __GCC_ASM_FLAG_OUTPUTS__
-# define ASM_FLAG_OUT(yes, no) yes
-#else
-# define ASM_FLAG_OUT(yes, no) no
-#endif
-
 /* MSRs. */
 #define MSR_TSC          0x00000010
 #define MSR_SYSENTER_CS  0x00000174
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
b/xen/include/asm-x86/hvm/vmx/vmcs.h
index d71de04..cb02d53 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -526,6 +526,7 @@ enum vmx_insn_errno
     VMX_INSN_VMPTRLD_INVALID_PHYADDR       = 9,
     VMX_INSN_UNSUPPORTED_VMCS_COMPONENT    = 12,
     VMX_INSN_VMXON_IN_VMX_ROOT             = 15,
+    VMX_INSN_FAIL_INVALID                  = ~0UL,
 };
 
 void vmx_disable_intercept_for_msr(struct vcpu *v, u32 msr, int type);
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h 
b/xen/include/asm-x86/hvm/vmx/vmx.h
index e5c6499..b0f3f3c 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -306,6 +306,12 @@ extern uint8_t posted_intr_vector;
 #define INVVPID_ALL_CONTEXT                     2
 #define INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 3
 
+#ifdef HAVE_GAS_VMX
+# define GAS_VMX_OP(yes, no) yes
+#else
+# define GAS_VMX_OP(yes, no) no
+#endif
+
 static always_inline void __vmptrld(u64 addr)
 {
     asm volatile (
@@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long field, 
unsigned long *value)
     return okay;
 }
 
+static always_inline unsigned long __vmwrite_safe(unsigned long field,
+                                                  unsigned long value)
+{
+    unsigned long ret = 0;
+    bool fail_invalid, fail_valid;
+
+    asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n",
+                              VMWRITE_OPCODE MODRM_EAX_ECX)
+                   ASM_FLAG_OUT(, "setb %[fail_invalid]\n")
+                   ASM_FLAG_OUT(, "sete %[fail_valid]\n")
+                   : ASM_FLAG_OUT("=@ccb", [fail_invalid] "=rm") 
(fail_invalid),
+                     ASM_FLAG_OUT("=@cce", [fail_valid] "=rm") (fail_valid)
+                   : [field] GAS_VMX_OP("r", "a") (field),
+                     [value] GAS_VMX_OP("rm", "c") (value));
+
+    if ( fail_invalid )
+        ret = VMX_INSN_FAIL_INVALID;
+    else if ( fail_valid )
+        __vmread(VM_INSTRUCTION_ERROR, &ret);
+
+    return ret;
+}
+
 static always_inline void __invept(unsigned long type, u64 eptp, u64 gpa)
 {
     struct {
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index d1171b7..6f6cac2 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -162,4 +162,10 @@ void init_constructors(void);
 void *bsearch(const void *key, const void *base, size_t num, size_t size,
               int (*cmp)(const void *key, const void *elt));
 
+#ifdef __GCC_ASM_FLAG_OUTPUTS__
+# define ASM_FLAG_OUT(yes, no) yes
+#else
+# define ASM_FLAG_OUT(yes, no) no
+#endif
+
 #endif /* __LIB_H__ */
-- 
2.7.4


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