From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Pau Monne Subject: Re: [PATCH 1/2] libxl: fix reentrancy hazard in fd event processing Date: Wed, 25 Jul 2012 17:49:42 +0100 Message-ID: <501023A6.8050805@citrix.com> References: <1342808310-25480-1-git-send-email-ian.jackson@eu.citrix.com> <1342808310-25480-2-git-send-email-ian.jackson@eu.citrix.com> <1343232346.18971.133.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1343232346.18971.133.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org Ian Campbell wrote: > On Fri, 2012-07-20 at 19:18 +0100, Ian Jackson wrote: >> In afterpoll_internal, the callback functions may register and >> deregister events arbitrarily. This means that we need to consider >> the reentrancy-safety of the event machinery state variables. >> >> Most of the code is safe but the fd handling is not. Fix this by >> arranging to restart the fd scan loop every time we call one of these >> callback functions. >> >> For this loop to terminate, we modify afterpoll_check_fd so that it >> returns only once for each of afterpoll's efds. >> >> Another possible solution would be simply to return from >> afterpoll_internal after calling efd->func. That would be a small and >> more obviously correct change but would prevent the process from >> handling more than one fd event with a single call to poll. >> >> This is apropos of a report from Roger Pau Monne to me (pers.comm.) >> of this crash on NetBSD: >> >> Program terminated with signal 11, Segmentation fault. >> #0 0x00007f7ff743131b in afterpoll_check_fd (poller=, fds=0x7f7ff7b241c0, nfds=7, fd=-1, events=1) >> at libxl_event.c:856 >> 856 if (fds[slot].fd != fd) > > Has Roger or you tested this now? This works ok. > It looks plausible to me. > >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h >> index 2781398..e938660 100644 >> --- a/tools/libxl/libxl_internal.h >> +++ b/tools/libxl/libxl_internal.h >> @@ -272,7 +272,7 @@ struct libxl__poller { >> int fd_polls_allocd; >> >> int fd_rindices_allocd; >> - int (*fd_rindices)[3]; /* see libxl_osevent_beforepoll */ >> + int (*fd_rindices)[3]; /* see libxl_event.c:beforepoll_internal */ > > do you mean afterpoll here? > > Acked-by: Ian Campbell > >