[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.10] libs/evtchn: Remove active handler on clean-up or failure
On Tue, Nov 14, 2017 at 12:14:14PM +0000, Julien Grall wrote: > Hi, > > On 14/11/17 11:51, Ian Jackson wrote: > > 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. > > I am going to leave the decision to you and Wei. It feels a bit odd to > release-ack my patch :). We can only commit patches that are both acked and release-acked. The latter gives RM control over when the patch should be applied. Sometimes it is better to wait until something else happens (like getting the tree to a stable state). That's how I used release-ack anyway. For this particular patch, my interpretation of what you just said is you've given us release-ack and we can apply this patch anytime. I will commit it soon. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |