[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> 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.

> +    v->arch.user_regs.eip = rsp->data.regs.x86.rip;

Similarly here I wonder whether this shouldn't be at least
range checked.

> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -417,6 +417,9 @@ void vm_event_resume(struct domain *d, struct 
> vm_event_domain *ved)
>  
>          if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
>          {
> +            if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
> +                vm_event_set_registers(v, &rsp);
> +
>              if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
>                  vm_event_toggle_singlestep(d, v);
 
What meaning has setting both flags and EFLAGS.TF in the new
register values?

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.