[Pcsclite-muscle] Race condition in hotplug_libusb.c

[Maximilian Stein]

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.


Kind regards
  Maximilian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-libusb-reader-init-lock-race-condition.patch
Type: text/x-patch
Size: 511 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pcsclite-muscle/attachments/20141105/c0ee7571/attachment.bin>