Re: [Xen-devel] [PATCH v1 1/2] x86/vvmx: check vmcs address in vmread/vmwrite

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

On Wed, 2017-03-01 at 05:55 -0700, Jan Beulich wrote:
> > > > On 01.03.17 at 10:13, <sergey.dyasli@xxxxxxxxxx> wrote:
> > 
> > If nested vmcs's address is invalid, virtual_vmcs_enter() will fail
> > during vmread/vmwrite:
> > 
> > (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)    [<ffff82d0801f60e3>] virtual_vmcs_vmwrite_safe+0x16/0x52
> > (XEN)    [<ffff82d080202cb2>] nvmx_handle_vmwrite+0x70/0xfe
> > (XEN)    [<ffff82d0801fe98a>] vmx_vmexit_handler+0x1379/0x1c49
> > (XEN)    [<ffff82d08020427c>] vmx_asm_vmexit_handler+0x3c/0x120
> > 
> > Fix this by emulating VMfailInvalid if the address is invalid.
> 
> So just like in patch 2 this is __vmptrld() not properly dealing with
> errors. Instead of doing checks in software which hardware does
> anyway, wouldn't it be better to introduce (and use here and
> there) vmptrld_safe()?

Currently it's assumed that virtual_vmcs_enter/exit() never fail.
It's easy to maintain that assumption with one simple check:

    nv_vvmcxaddr != INVALID_PADDR

as long as nvmx_handle_vmptrld() correctly checks the validity of
provided pointer.

Additionally, it would be painful to return the correct error value
all the way back to nvmx_handle_vmptrld().

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