Re: [Xen-devel] [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

On 6/16/2016 7:11 PM, Tamas K Lengyel wrote:
> On Thu, Jun 16, 2016 at 8:07 AM, Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx> wrote:
> > For VM_EVENT_FLAG_DENY to work, the vcpu must be paused (sync = 1) until the
> > vm-event is handled. A vm-event response having VM_EVENT_FLAG_DENY flag set
> > should also set the VM_EVENT_FLAG_VCPU_PAUSED flag. Enforce that in
> > vm_event_register_write_resume().
> Well, the problem with this is that the user can set the VCPU_PAUSED
> flag any time it wants. It can happen that Xen hasn't paused the vCPU
> but the user still sends that flag, in which case the unpause the flag
> induces will not actually do anything. You should also check if the
> vCPU is in fact paused rather then just relying on this flag.
>
> Tamas
Tamas,

Isn't that also the case with the following block @ vm_event_resume:

        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);

            vm_event_vcpu_unpause(v);
        }

, i.e. shouldn't it be fixed to:

        /* check flags which apply only when the vCPU is paused */
        if ( atomic_read(&v->vm_event_pause_count) )
        {
            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);
            if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
                vm_event_vcpu_unpause(v);
        }
?

If this holds, the check for vCPU pause can also be removed from 
vm_event_toggle_singlestep (maybe turned into an ASSERT which could also 
be added to vm_event_set_registers).
Corneliu.

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