| Message ID | 50d76d71-7e76-8c4d-0546-bf690085036b@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:11, Jan Beulich wrote: > port_is_valid() and evtchn_from_port() are fine to use without holding > any locks. Per my remark in patch #1, currently, this only holds as long as we are checking the port for the current domain. If it were a different domain, then the risk a risk that port_is_valid() may return valid and then invalid. AFAICT, evtchn_close() may be called with d != current. So there is some racyness in the code as well. Therefore, I will defer my ack until we agree on whether port_is_valid() can be used locklessly when current != domain. Cheers,
--- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -604,17 +604,14 @@ int evtchn_close(struct domain *d1, int int port2; long rc = 0; - again: - write_lock(&d1->event_lock); - if ( !port_is_valid(d1, port1) ) - { - rc = -EINVAL; - goto out; - } + return -EINVAL; chn1 = evtchn_from_port(d1, port1); + again: + write_lock(&d1->event_lock); + /* Guest cannot close a Xen-attached event channel. */ if ( unlikely(consumer_is_xen(chn1)) && guest ) { @@ -1039,16 +1036,13 @@ long evtchn_bind_vcpu(unsigned int port, if ( (v = domain_vcpu(d, vcpu_id)) == NULL ) return -ENOENT; - write_lock(&d->event_lock); - if ( !port_is_valid(d, port) ) - { - rc = -EINVAL; - goto out; - } + return -EINVAL; chn = evtchn_from_port(d, port); + write_lock(&d->event_lock); + /* Guest cannot re-bind a Xen-attached event channel. */ if ( unlikely(consumer_is_xen(chn)) ) {
port_is_valid() and evtchn_from_port() are fine to use without holding any locks. Accordingly acquire the per-domain lock slightly later in evtchn_close() and evtchn_bind_vcpu(). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v4: New.