| Message ID | 20170208100935.30332-2-sergey.dyasli@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 08.02.17 at 11:09, <sergey.dyasli@citrix.com> wrote: > 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 copied > into asm_defns.h > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com] > Sent: Wednesday, February 08, 2017 6:10 PM > > 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 copied > into asm_defns.h > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> Acked-by: Kevin Tian <kevin.tian@intel.com>
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/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h index f1c6fa1..220ae2e 100644 --- a/xen/include/asm-x86/asm_defns.h +++ b/xen/include/asm-x86/asm_defns.h @@ -413,4 +413,10 @@ static always_inline void stac(void) #define REX64_PREFIX "rex64/" #endif +#ifdef __GCC_ASM_FLAG_OUTPUTS__ +# define ASM_FLAG_OUT(yes, no) yes +#else +# define ASM_FLAG_OUT(yes, no) no +#endif + #endif /* __X86_ASM_DEFNS_H__ */ diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index d71de04..8d43efd 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 = ~0, }; 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 4bf4d50..9bfe71a 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 inline enum vmx_insn_errno 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\t", + VMWRITE_OPCODE MODRM_EAX_ECX) + ASM_FLAG_OUT(, "setc %[invalid]\n\t") + ASM_FLAG_OUT(, "setz %[valid]\n\t") + : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid), + ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid) + : [field] GAS_VMX_OP("r", "a") (field), + [value] GAS_VMX_OP("rm", "c") (value)); + + if ( unlikely(fail_invalid) ) + ret = VMX_INSN_FAIL_INVALID; + else if ( unlikely(fail_valid) ) + __vmread(VM_INSTRUCTION_ERROR, &ret); + + return ret; +} + static always_inline void __invept(unsigned long type, u64 eptp, u64 gpa) { struct {
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 copied into asm_defns.h Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> --- v2 --> v3: * vmwrite_safe() now returns "enum vmx_insn_errno" (32-bit value) * vmwrite_safe() is changed from always_inline to plain inline xen/arch/x86/hvm/vmx/vvmx.c | 3 ++- xen/include/asm-x86/asm_defns.h | 6 ++++++ xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + xen/include/asm-x86/hvm/vmx/vmx.h | 29 +++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-)