| Message ID | 20170301091355.24742-3-sergey.dyasli@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/03/17 09:13, Sergey Dyasli wrote: > If a guest will do vmptrld with an incorrect vmcs id: > > (XEN) Xen BUG at .../git/upstream/xen/xen/include/asm/hvm/vmx/vmx.h:333 > (XEN) ----[ Xen-4.9-unstable x86_64 debug=y Tainted: H ]---- > (XEN) Xen call trace: > (XEN) [<ffff82d0801f925e>] vmcs.c#arch/x86/hvm/vmx/vmcs.o.unlikely+0x28/0x19a > (XEN) [<ffff82d0801f602c>] virtual_vmcs_vmread+0x11/0x2c > (XEN) [<ffff82d0802002cc>] vvmx.c#_map_io_bitmap+0x86/0x88 > (XEN) [<ffff82d080202399>] nvmx_handle_vmptrld+0xf0/0x1fb > (XEN) [<ffff82d0801fe93c>] vmx_vmexit_handler+0x132b/0x1c49 > (XEN) [<ffff82d080203e6c>] vmx_asm_vmexit_handler+0x3c/0x120 > > Fix this by adding appropriate checks for vmcs id during vmptrld > emulation. > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> > --- > xen/arch/x86/hvm/vmx/vvmx.c | 11 +++++++++++ > xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 4f5ee5a..580fd3e 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1633,6 +1633,17 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs) > { > if ( writable ) > { > + struct vmcs_struct *vvmcs = vvmcx; > + > + if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) & > + VMX_BASIC_REVISION_MASK) || > + (!cpu_has_vmx_vmcs_shadowing && > + (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) ) > + { > + hvm_unmap_guest_frame(vvmcx, 1); > + vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID); A newline here please (can be fixed on commit if there are no other issues). Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > + return X86EMUL_OKAY; > + } > nvcpu->nv_vvmcx = vvmcx; > nvcpu->nv_vvmcxaddr = gpa; > v->arch.hvm_vmx.vmcs_shadow_maddr = > diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h > index 4ee01da..6308e4b 100644 > --- a/xen/include/asm-x86/hvm/vmx/vmcs.h > +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h > @@ -521,6 +521,7 @@ enum vmx_insn_errno > VMX_INSN_INVALID_CONTROL_STATE = 7, > VMX_INSN_INVALID_HOST_STATE = 8, > VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9, > + VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID = 11, > VMX_INSN_UNSUPPORTED_VMCS_COMPONENT = 12, > VMX_INSN_VMXON_IN_VMX_ROOT = 15, > VMX_INSN_FAIL_INVALID = ~0,
> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com] > Sent: Wednesday, March 01, 2017 5:14 PM > > If a guest will do vmptrld with an incorrect vmcs id: > > (XEN) Xen BUG at .../git/upstream/xen/xen/include/asm/hvm/vmx/vmx.h:333 > (XEN) ----[ Xen-4.9-unstable x86_64 debug=y Tainted: H ]---- > (XEN) Xen call trace: > (XEN) [<ffff82d0801f925e>] > vmcs.c#arch/x86/hvm/vmx/vmcs.o.unlikely+0x28/0x19a > (XEN) [<ffff82d0801f602c>] virtual_vmcs_vmread+0x11/0x2c > (XEN) [<ffff82d0802002cc>] vvmx.c#_map_io_bitmap+0x86/0x88 > (XEN) [<ffff82d080202399>] nvmx_handle_vmptrld+0xf0/0x1fb > (XEN) [<ffff82d0801fe93c>] vmx_vmexit_handler+0x132b/0x1c49 > (XEN) [<ffff82d080203e6c>] vmx_asm_vmexit_handler+0x3c/0x120 > > Fix this by adding appropriate checks for vmcs id during vmptrld > emulation. > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> Acked-by: Kevin Tian <kevin.tian@intel.com>
>>> On 01.03.17 at 12:01, <andrew.cooper3@citrix.com> wrote: > On 01/03/17 09:13, Sergey Dyasli wrote: >> --- a/xen/arch/x86/hvm/vmx/vvmx.c >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >> @@ -1633,6 +1633,17 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs) >> { >> if ( writable ) >> { >> + struct vmcs_struct *vvmcs = vvmcx; >> + >> + if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) & >> + VMX_BASIC_REVISION_MASK) || >> + (!cpu_has_vmx_vmcs_shadowing && >> + (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) ) >> + { >> + hvm_unmap_guest_frame(vvmcx, 1); >> + vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID); > > A newline here please (can be fixed on commit if there are no other > issues). Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > >> + return X86EMUL_OKAY; >> + } I've added one, but commonly we require such only on the main (final) return statement of a function. Jan
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 4f5ee5a..580fd3e 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1633,6 +1633,17 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs) { if ( writable ) { + struct vmcs_struct *vvmcs = vvmcx; + + if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) & + VMX_BASIC_REVISION_MASK) || + (!cpu_has_vmx_vmcs_shadowing && + (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) ) + { + hvm_unmap_guest_frame(vvmcx, 1); + vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID); + return X86EMUL_OKAY; + } nvcpu->nv_vvmcx = vvmcx; nvcpu->nv_vvmcxaddr = gpa; v->arch.hvm_vmx.vmcs_shadow_maddr = diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 4ee01da..6308e4b 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -521,6 +521,7 @@ enum vmx_insn_errno VMX_INSN_INVALID_CONTROL_STATE = 7, VMX_INSN_INVALID_HOST_STATE = 8, VMX_INSN_VMPTRLD_INVALID_PHYADDR = 9, + VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID = 11, VMX_INSN_UNSUPPORTED_VMCS_COMPONENT = 12, VMX_INSN_VMXON_IN_VMX_ROOT = 15, VMX_INSN_FAIL_INVALID = ~0,
If a guest will do vmptrld with an incorrect vmcs id: (XEN) Xen BUG at .../git/upstream/xen/xen/include/asm/hvm/vmx/vmx.h:333 (XEN) ----[ Xen-4.9-unstable x86_64 debug=y Tainted: H ]---- (XEN) Xen call trace: (XEN) [<ffff82d0801f925e>] vmcs.c#arch/x86/hvm/vmx/vmcs.o.unlikely+0x28/0x19a (XEN) [<ffff82d0801f602c>] virtual_vmcs_vmread+0x11/0x2c (XEN) [<ffff82d0802002cc>] vvmx.c#_map_io_bitmap+0x86/0x88 (XEN) [<ffff82d080202399>] nvmx_handle_vmptrld+0xf0/0x1fb (XEN) [<ffff82d0801fe93c>] vmx_vmexit_handler+0x132b/0x1c49 (XEN) [<ffff82d080203e6c>] vmx_asm_vmexit_handler+0x3c/0x120 Fix this by adding appropriate checks for vmcs id during vmptrld emulation. Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> --- xen/arch/x86/hvm/vmx/vvmx.c | 11 +++++++++++ xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + 2 files changed, 12 insertions(+)