[Pcsclite-muscle] Race condition in hotplug_libusb.c

[Ludovic Rousseau]

2014-11-05 15:40 GMT+01:00 Maximilian Stein <maximilian.stein at secunet.com>:
> Hi,
> we ran into a race condition in the libusb-1.0 hotplug module of
> pcsc-lite-1.8.11. As usual with race conditions the error was not easy
> to reproduce and to locate. Therefore I would like to contribute the
> attached patch which fixes the race condition.
>
> Problem description:
> On start-up pcscd would seem to work correctly, but when connecting a
> client process to it the client would hang/freeze on
> SCardEstablishContext() while the pcscd remained motionless, waiting on
> the communication socket for client connections.
> This was happening only when using a certain combination of serial and
> USB readers and only when using pcsc-lite with --enable-libusb
> (libusb-1.0 support).
>
>
> Problem solution:
> On start-up of pcscd HPSearchHotPluggables() is called in the main
> thread. This spawns the USB polling thread with function
> HPEstablishUSBNotifications(int pipefd[2]) and then uses the pipe
> pipefd to wait for the spawned thread with read(pipefd[0], &c, 1) in
> line 511 [1].
> HPEstablishUSBNotifications takes the POINTER to the locally allocated
> pipefd from HPSearchHotPluggables and NOT a copy of the pipefd array.
> What can happen is, that after the USB polling thread performed the
> write on the pipe, the execution continues with the main thread, leaving
> the scope and invalidating the pipefd array.
> When the USB polling thread regains the execution it calls close [2] on
> the file descriptor that is at memory position pipefd[1] which now is
> invalid because HPSearchHotPluggables() was left long ago. The memory
> might be reused and in our particular case it held the integer '4' which
> was the file descriptor of the communication socket file.
> Since threads share the same file descriptors the USB polling thread
> closed the socket on which the main thread was listening without causing
> any handled error conditions (checking eventfds in the main loop
> select() would provide information about the closed fd).
>
>
> [1]
> LONG HPSearchHotPluggables(void)
> {
>   ...
>   if (HPReadBundleValues())
>   {
>     int pipefd[2];
>     char c;
>
>     if (pipe(pipefd) == -1)
>     {
>       Log2(PCSC_LOG_ERROR, "pipe: %s", strerror(errno));
>       return -1;
>     }
>
>     ThreadCreate(&usbNotifyThread, THREAD_ATTR_DETACHED,
>      (PCSCLITE_THREAD_FUNCTION( )) HPEstablishUSBNotifications, pipefd);
>
>     /* Wait for initial readers to setup */
>     read(pipefd[0], &c, 1);
>     close(pipefd[0]);
>   }
> ...
> }
>
>
> [2]
> static void HPEstablishUSBNotifications(int pipefd[2])
> {
>   ...
>   /* scan the USB bus for devices at startup */
>   HPRescanUsbBus();
>
>   /* signal that the initially connected readers are now visible */
>   write(pipefd[1], &c, 1);
>   close(pipefd[1]);
>   ...
> }
>
> In my opinion there is no need for the USB polling thread to close the
> pipefd[1] and it should/has to be done by the function that created the
> array.

Fixed in revision 7029.
http://lists.alioth.debian.org/pipermail/pcsclite-cvs-commit/2014-November/006584.html

Thanks

-- 
 Dr. Ludovic Rousseau