Re: [Xen-devel] [PATCH V3 2/2] xen: Introduce VM_EVENT_FLAG_SET_REGISTERS

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Sep 28, 2015 at 10:06 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 28.09.15 at 17:57, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 28/09/15 16:25, Jan Beulich wrote:
>>>>> On 28.09.15 at 12:16, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
>>> +{
>>> +Â Â v->arch.user_regs.eax = rsp->data.regs.x86.rax;
>>> +Â Â v->arch.user_regs.ebx = rsp->data.regs.x86.rbx;
>>> +Â Â v->arch.user_regs.ecx = rsp->data.regs.x86.rcx;
>>> +Â Â v->arch.user_regs.edx = rsp->data.regs.x86.rdx;
>>> +Â Â v->arch.user_regs.esp = rsp->data.regs.x86.rsp;
>>> +Â Â v->arch.user_regs.ebp = rsp->data.regs.x86.rbp;
>>> +Â Â v->arch.user_regs.esi = rsp->data.regs.x86.rsi;
>>> +Â Â v->arch.user_regs.edi = rsp->data.regs.x86.rdi;
>>> +
>>> +Â Â v->arch.user_regs.r8 = rsp->data.regs.x86.r8;
>>> +Â Â v->arch.user_regs.r9 = rsp->data.regs.x86.r9;
>>> +Â Â v->arch.user_regs.r10 = rsp->data.regs.x86.r10;
>>> +Â Â v->arch.user_regs.r11 = rsp->data.regs.x86.r11;
>>> +Â Â v->arch.user_regs.r12 = rsp->data.regs.x86.r12;
>>> +Â Â v->arch.user_regs.r13 = rsp->data.regs.x86.r13;
>>> +Â Â v->arch.user_regs.r14 = rsp->data.regs.x86.r14;
>>> +Â Â v->arch.user_regs.r15 = rsp->data.regs.x86.r15;
>>> +
>>> +Â Â v->arch.user_regs.eflags = rsp->data.regs.x86.rflags;
>> Shouldn't you sanitize the value? I can't immediately see anything
>> putting Xen at risk (but it also doesn't seem impossible that I'm
>> overlooking something), but surely putting insane values here
>> can lead to hard to debug guest crashes.
>
> I had the same thought (e.g. XSA-111), but all modifications like this
> are already possible with a cunningly-crafted sethvmcontext so we are at
> no more risk than before.

By or for HVM guests. But how about PV?

> Furthermore, I can't think of any plausible validation which could be
> done. It is entirely possible that this interface could be used to
> bounce execution into a hidden introspection agent.

Flipping VM, AC, NT or altering IOPL would all seem bogus to me.

Jan

The entire interface is only available from a privileged control domain already able to perform all of the steps that this option does. This option just streamlines that operation so we don't have to do two hypercalls. So while certainly a buggy vm_event application can crash the guest in spectacular ways I don't think Xen could (or should) really prevent that from happening. Currently vm_events are not implemented for PV guests so that IMHO is not a problem we need to consider right now. Furthermore, even if we add support for PV guests somewhere down the line the interface should be on par with the current HVM implementation.

Tamas

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