| Message ID | e0179c46-face-4c64-9aeb-186be65ab77b@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | evtchn: (not so) recent XSAs follow-on | expand |
Hi Jan, On 05/01/2021 13:10, Jan Beulich wrote: > While there don't look to be any problems with this right now, the lock > order implications from holding the lock can be very difficult to follow > (and may be easy to violate unknowingly). The present callbacks don't > (and no such callback should) have any need for the lock to be held. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com> > --- > v4: Go back to v2. > v3: Drain callbacks before proceeding with closing. Re-base. > > --- a/xen/common/event_channel.c > +++ b/xen/common/event_channel.c > @@ -767,9 +767,18 @@ int evtchn_send(struct domain *ld, unsig > rport = lchn->u.interdomain.remote_port; > rchn = evtchn_from_port(rd, rport); > if ( consumer_is_xen(rchn) ) > - xen_notification_fn(rchn)(rd->vcpu[rchn->notify_vcpu_id], rport); > - else > - evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn); > + { > + /* Don't keep holding the lock for the call below. */ > + xen_event_channel_notification_t fn = xen_notification_fn(rchn); > + struct vcpu *rv = rd->vcpu[rchn->notify_vcpu_id]; > + > + rcu_lock_domain(rd); > + evtchn_read_unlock(lchn); > + fn(rv, rport); > + rcu_unlock_domain(rd); > + return 0; > + } > + evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn); > break; > case ECS_IPI: > evtchn_port_set_pending(ld, lchn->notify_vcpu_id, lchn); > Cheers,
--- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -767,9 +767,18 @@ int evtchn_send(struct domain *ld, unsig rport = lchn->u.interdomain.remote_port; rchn = evtchn_from_port(rd, rport); if ( consumer_is_xen(rchn) ) - xen_notification_fn(rchn)(rd->vcpu[rchn->notify_vcpu_id], rport); - else - evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn); + { + /* Don't keep holding the lock for the call below. */ + xen_event_channel_notification_t fn = xen_notification_fn(rchn); + struct vcpu *rv = rd->vcpu[rchn->notify_vcpu_id]; + + rcu_lock_domain(rd); + evtchn_read_unlock(lchn); + fn(rv, rport); + rcu_unlock_domain(rd); + return 0; + } + evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn); break; case ECS_IPI: evtchn_port_set_pending(ld, lchn->notify_vcpu_id, lchn);
While there don't look to be any problems with this right now, the lock order implications from holding the lock can be very difficult to follow (and may be easy to violate unknowingly). The present callbacks don't (and no such callback should) have any need for the lock to be held. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v4: Go back to v2. v3: Drain callbacks before proceeding with closing. Re-base.