[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 09/28/2015 06:25 PM, 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. > >> + v->arch.user_regs.eip = rsp->data.regs.x86.rip; > > Similarly here I wonder whether this shouldn't be at least > range checked. I'm assuming that the userspace vm_event client application will use the interface wisely. A typical scenario would be: - vm_event request received; - reply.registers = request.registers; - if this event requires setting registers, set only relevant ones; - send the reply to the hypervisor (with the set registers flag). So with this usage it shouldn't be possible to accidentally send garbage back. >> --- 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? That's a good question. I'm not sure how that would affect an attached debugger type scenario. I'm also unsure of what a good solution to this issue would be. I could make the flags exclusive, but that would prevent, for example, setting EAX and singlestepping, which should not be a problem. I could also remove the eflags assignment from vm_event_set_registers() (maybe replace it with a comment). Tamas, do you need eflags set? What do you think? Again, I'm happy with just setting EIP, what scenarios are you interested in? Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |