Re: [Xen-devel] xc_hvm_inject_trap() races

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 11/07/2016 06:10 PM, Jan Beulich wrote:
>>>> On 07.11.16 at 16:24, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> The one-shot vm_event does sound reasonable. I could set a flag
>> per-VCPU, basically similar to v->arch.hvm_vcpu.inject_trap.vector, and
>> fire a single event from hvm_inject_trap() if it's set (then unset it) -
>> the flag would be set via an xc_monitor_next_interrupt() call in libxc.
> 
> Doing this in hvm_inject_trap() would not cover all cases afict.
> I'd suggest doing this from hvm_do_resume() _after_ the
> (conditional) call to hvm_inject_trap(), if there is _any_ event
> pending.

But that would only cover the hypercall-injected traps. The condition in
hvm_do_resume() is "if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )",
and inject_trap.vector seems to only ever be set by the hypercall:

5876     case HVMOP_inject_trap:
5877     {
5878         xen_hvm_inject_trap_t tr;
5879         struct domain *d;
5880         struct vcpu *v;
5881
5882         if ( copy_from_guest(&tr, arg, 1 ) )
5883             return -EFAULT;
5884
5885         rc = rcu_lock_remote_domain_by_id(tr.domid, &d);
5886         if ( rc != 0 )
5887             return rc;
5888
5889         rc = -EINVAL;
5890         if ( !is_hvm_domain(d) )
5891             goto injtrap_fail;
5892
5893         rc = xsm_hvm_control(XSM_DM_PRIV, d, op);
5894         if ( rc )
5895             goto injtrap_fail;
5896
5897         rc = -ENOENT;
5898         if ( tr.vcpuid >= d->max_vcpus || (v = d->vcpu[tr.vcpuid])
== NULL )
5899             goto injtrap_fail;
5900
5901         if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
5902             rc = -EBUSY;
5903         else
5904         {
5905             v->arch.hvm_vcpu.inject_trap.vector = tr.vector;
5906             v->arch.hvm_vcpu.inject_trap.type = tr.type;
5907             v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
5908             v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
5909             v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
5910             rc = 0;
5911         }
5912
5913     injtrap_fail:
5914         rcu_unlock_domain(d);
5915         break;
5916     }

So if the next interrupt is not caused by the hypercall, we'll never get
another event. Am I reading the code wrong?


Thanks,
Razvan

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