Re: [PATCH 1/2] libxl: fix reentrancy hazard in fd event processing

[Roger Pau Monne <roger.pau@citrix.com>]

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=<optimized out>, 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 <ian.campbell@citrix.com>
> 
>