Re: [Xen-devel] [PATCH 1/3] libxl: events: Pass correct nfds to poll

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-01-17 at 17:34 +0000, Ian Jackson wrote:
> libxl_event.c:eventloop_iteration would pass the allocated pollfds
> array size, rather than the used size, to poll (and to
> afterpoll_internal).
> 
> The effect is that if the number of fds to poll on reduces, libxl will
> poll on stale entries.  Because of the way the return value from poll
> is processed these stale entries are often harmless because any events
> coming back from poll ignored by libxl.  However, it could cause
> malfunctions:
> 
> It could result in unwanted SIGTTIN/SIGTTOU/SIGPIPE, for example, if
> the fd has been reused to refer to an object which can generate those
> signals.  Alternatively, it could result in libxl spinning if the
> stale entry refers to an fd which happens now to be ready for the
> previously-requested operation.
> 
> I have tested this with a localhost migration and inspected the strace
> output.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Jim Fehlig <jfehlig@xxxxxxxx>

Acked-by: Ian Campbell <Ian.Campbell@xxxxxxxxxx>

> ---
>  tools/libxl/libxl_event.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index bdef7ac..1c48fee 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -1386,7 +1386,7 @@ static int eventloop_iteration(libxl__egc *egc, 
> libxl__poller *poller) {
>       * can unlock it when it polls.
>       */
>      EGC_GC;
> -    int rc;
> +    int rc, nfds;
>      struct timeval now;
>      
>      rc = libxl__gettimeofday(gc, &now);
> @@ -1395,7 +1395,7 @@ static int eventloop_iteration(libxl__egc *egc, 
> libxl__poller *poller) {
>      int timeout;
>  
>      for (;;) {
> -        int nfds = poller->fd_polls_allocd;
> +        nfds = poller->fd_polls_allocd;
>          timeout = -1;
>          rc = beforepoll_internal(gc, poller, &nfds, poller->fd_polls,
>                                   &timeout, now);
> @@ -1413,7 +1413,7 @@ static int eventloop_iteration(libxl__egc *egc, 
> libxl__poller *poller) {
>      }
>  
>      CTX_UNLOCK;
> -    rc = poll(poller->fd_polls, poller->fd_polls_allocd, timeout);
> +    rc = poll(poller->fd_polls, nfds, timeout);
>      CTX_LOCK;
>  
>      if (rc < 0) {
> @@ -1428,8 +1428,7 @@ static int eventloop_iteration(libxl__egc *egc, 
> libxl__poller *poller) {
>      rc = libxl__gettimeofday(gc, &now);
>      if (rc) goto out;
>  
> -    afterpoll_internal(egc, poller,
> -                       poller->fd_polls_allocd, poller->fd_polls, now);
> +    afterpoll_internal(egc, poller, nfds, poller->fd_polls, now);
>  
>      rc = 0;
>   out:



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