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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/03/17 12:55, 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()?

Lonterm, I'd like to see us move to a model where the base version is
safe, and error handling is along the lines of:

if ( on_behalf_of_nested )
    propagate_to_guest();
else
{
    dump_relevent_info();
    domain_crash(current->domain);
}


This is far more useful for error isolation (not taking the host down
even if it is Xen's fault the failure occured), and diagnosing what
actually went wrong, than simply having hit a BUG().

~Andrew

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