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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.03.17 at 14:22, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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().

I completely agree, but I don't think you or I mean to ask Sergey to
do the full conversion right away. I.e. I think your reply is somewhat
orthogonal to (or is going way beyond) my request.

Jan


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