Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Ross Lagerwall writes ("Re: [PATCH for-4.10] libs/evtchn: Remove active handler 
on clean-up or failure"):
> On 11/10/2017 05:10 PM, Julien Grall wrote:
> > Commit 89d55473ed16543044a31d1e0d4660cf5a3f49df "xentoolcore_restrict_all:
> > Implement for libxenevtchn" added a call to register allowing to
> > restrict the event channel.
> > 
> > However, the call to deregister the handler was not performed if open
> > failed or when closing the event channel. This will result to corrupt
> > the list of handlers and potentially crash the application later one.

Sorry for not spotting this during review.
The fix is correct as far as it goes, so:

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

> > The call to xentoolcore_deregister_active_handle is done at the same
> > place as for the grants. But I am not convinced this is thread safe as
> > there are potential race between close the event channel and restict
> > handler. Do we care about that?
...
> However, I think it should call xentoolcore__deregister_active_handle() 
> _before_ calling osdep_evtchn_close() to avoid trying to restrict a 
> closed fd or some other fd that happens to have the same number.

You are right.  But this slightly weakens the guarantee provided by
xentoolcore_restrict_all.

> I think all the other libs need to be fixed as well, unless there was a 
> reason it was done this way.

I will send a further patch.  In the meantime I suggest we apply
Julien's fix.

Ian.

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