io: Fix race condition in handle_timeout()
authorJoost Muller <joostmuller@gmail.com>
Wed, 20 Jul 2016 08:58:57 +0000 (10:58 +0200)
committerNathan Hjelm <hjelmn@me.com>
Fri, 22 Jul 2016 04:59:35 +0000 (22:59 -0600)
There is a race between handle_timeout() the completion functions.
When one thread is in handle_timeout() and another thread wakes
up from a poll(), there exists a window where the transfer has been
cancelled, but its USBI_TRANSFER_TIMED_OUT flag is not set yet.
Therefore, usbi_handle_transfer_completion() is sometimes called
with LIBUSB_TRANSFER_CANCELLED instead of the expected
LIBUSB_TRANSFER_TIMED_OUT.

This change adds transfer and flag locks to the handle_timeout()
function.

Closes #197

Signed-off-by: Nathan Hjelm <hjelmn@me.com>
libusb/io.c
libusb/version_nano.h

index 4d03b8b..8944461 100644 (file)
@@ -1530,6 +1530,34 @@ out:
        return r;
 }
 
+static int cancel_transfer_locked(struct libusb_transfer *transfer)
+{
+       struct usbi_transfer *itransfer =
+               LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
+       int r;
+       if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
+                       || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
+               return LIBUSB_ERROR_NOT_FOUND;
+       }
+
+       r = usbi_backend->cancel_transfer(itransfer);
+       if (r < 0) {
+               if (r != LIBUSB_ERROR_NOT_FOUND &&
+                   r != LIBUSB_ERROR_NO_DEVICE)
+                       usbi_err(TRANSFER_CTX(transfer),
+                               "cancel transfer failed error %d", r);
+               else
+                       usbi_dbg("cancel transfer failed error %d", r);
+
+               if (r == LIBUSB_ERROR_NO_DEVICE)
+                       itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
+       }
+
+       itransfer->flags |= USBI_TRANSFER_CANCELLING;
+
+       return r;
+}
+
 /** \ingroup libusb_asyncio
  * Asynchronously cancel a previously submitted transfer.
  * This function returns immediately, but this does not indicate cancellation
@@ -1553,27 +1581,9 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
        usbi_dbg("transfer %p", transfer );
        usbi_mutex_lock(&itransfer->lock);
        usbi_mutex_lock(&itransfer->flags_lock);
-       if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
-                       || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
-               r = LIBUSB_ERROR_NOT_FOUND;
-               goto out;
-       }
-       r = usbi_backend->cancel_transfer(itransfer);
-       if (r < 0) {
-               if (r != LIBUSB_ERROR_NOT_FOUND &&
-                   r != LIBUSB_ERROR_NO_DEVICE)
-                       usbi_err(TRANSFER_CTX(transfer),
-                               "cancel transfer failed error %d", r);
-               else
-                       usbi_dbg("cancel transfer failed error %d", r);
 
-               if (r == LIBUSB_ERROR_NO_DEVICE)
-                       itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
-       }
-
-       itransfer->flags |= USBI_TRANSFER_CANCELLING;
+       r = cancel_transfer_locked(transfer);
 
-out:
        usbi_mutex_unlock(&itransfer->flags_lock);
        usbi_mutex_unlock(&itransfer->lock);
        return r;
@@ -1967,13 +1977,20 @@ static void handle_timeout(struct usbi_transfer *itransfer)
                USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
        int r;
 
+       usbi_mutex_lock(&itransfer->lock);
+       usbi_mutex_lock(&itransfer->flags_lock);
+
        itransfer->flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
-       r = libusb_cancel_transfer(transfer);
+       r = cancel_transfer_locked(transfer);
+
        if (r == 0)
                itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
        else
                usbi_warn(TRANSFER_CTX(transfer),
                        "async cancel failed %d errno=%d", r, errno);
+
+       usbi_mutex_unlock(&itransfer->flags_lock);
+       usbi_mutex_unlock(&itransfer->lock);
 }
 
 static int handle_timeouts_locked(struct libusb_context *ctx)
index 75b9b06..3687c25 100644 (file)
@@ -1 +1 @@
-#define LIBUSB_NANO 11119
+#define LIBUSB_NANO 11120