core: Fix handling of duplicate transfer submission/cancellation

In the docs, we claim to report LIBUSB_ERROR_BUSY if submitting a
transfer that has already been submitted. Linux was the only backend
that actually checked and reported this, but unfortunately the code
in libusb_submit_transfer() resulted in a segfault. This is because
submission failure would delete the (active) transfer from the
flying_transfers list, and when the transfer finally completes it
would be deleted from the flying_transfers list a second time.

Instead of modifying each backend to check for a busy transfer, this
patch adds a flag to indicate when a transfer is in-flight. The core
library will check this flag and return LIBUSB_ERROR_BUSY immediately.

This patch also modifies libusb_cancel_transfer() to check that a
transfer is in-flight before cancelling and to check that a transfer
has not already been cancelled, returning LIBUSB_ERROR_NOT_FOUND in
both cases.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/io.c b/libusb/io.c
index 4ea6ad2..9614086 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1432,6 +1432,10 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
        usbi_dbg("transfer %p", transfer);
        usbi_mutex_lock(&ctx->flying_transfers_lock);
        usbi_mutex_lock(&itransfer->lock);
+       if (itransfer->flags & USBI_TRANSFER_IN_FLIGHT) {
+               r = LIBUSB_ERROR_BUSY;
+               goto out;
+       }
        itransfer->transferred = 0;
        itransfer->flags = 0;
        r = calculate_timeout(itransfer);
@@ -1448,6 +1452,7 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
                list_del(&itransfer->list);
                arm_timerfd_for_next_timeout(ctx);
        } else {
+               itransfer->flags |= USBI_TRANSFER_IN_FLIGHT;
                /* keep a reference to this device */
                libusb_ref_device(transfer->dev_handle->dev);
        }
@@ -1467,8 +1472,8 @@ out:
  *
  * \param transfer the transfer to cancel
  * \returns 0 on success
- * \returns LIBUSB_ERROR_NOT_FOUND if the transfer is already complete or
- * cancelled.
+ * \returns LIBUSB_ERROR_NOT_FOUND if the transfer is not in progress,
+ * already complete, or already cancelled.
  * \returns a LIBUSB_ERROR code on failure
  */
 int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
@@ -1479,6 +1484,11 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
 
        usbi_dbg("transfer %p", transfer );
        usbi_mutex_lock(&itransfer->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 &&
@@ -1494,6 +1504,7 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
 
        itransfer->flags |= USBI_TRANSFER_CANCELLING;
 
+out:
        usbi_mutex_unlock(&itransfer->lock);
        return r;
 }
@@ -1566,6 +1577,10 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
        if (usbi_using_timerfd(ctx) && (r < 0))
                return r;
 
+       usbi_mutex_lock(&itransfer->lock);
+       itransfer->flags &= ~USBI_TRANSFER_IN_FLIGHT;
+       usbi_mutex_unlock(&itransfer->lock);
+
        if (status == LIBUSB_TRANSFER_COMPLETED
                        && transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) {
                int rqlen = transfer->length;

diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index cb86564..6d25884 100644 (file)
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -417,6 +417,9 @@ enum usbi_transfer_flags {
 
        /* Operation on the transfer failed because the device disappeared */
        USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 3,
+
+       /* Transfer is currently active */
+       USBI_TRANSFER_IN_FLIGHT = 1 << 4,
 };
 
 #define USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer) \

diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 63f4105..05cf909 100644 (file)
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1766,9 +1766,6 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer)
        int i;
        size_t alloc_size;
 
-       if (tpriv->urbs)
-               return LIBUSB_ERROR_BUSY;
-
        if (is_out && (transfer->flags & LIBUSB_TRANSFER_ADD_ZERO_PACKET) &&
                        !(dpriv->caps & USBFS_CAP_ZERO_PACKET))
                return LIBUSB_ERROR_NOT_SUPPORTED;
@@ -1945,9 +1942,6 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
        unsigned int packet_len;
        unsigned char *urb_buffer = transfer->buffer;
 
-       if (tpriv->iso_urbs)
-               return LIBUSB_ERROR_BUSY;
-
        /* usbfs places a 32kb limit on iso URBs. we divide up larger requests
         * into smaller units to meet such restriction, then fire off all the
         * units at once. it would be simpler if we just fired one unit at a time,
@@ -2092,9 +2086,6 @@ static int submit_control_transfer(struct usbi_transfer *itransfer)
        struct usbfs_urb *urb;
        int r;
 
-       if (tpriv->urbs)
-               return LIBUSB_ERROR_BUSY;
-
        if (transfer->length - LIBUSB_CONTROL_SETUP_SIZE > MAX_CTRL_BUFFER_LENGTH)
                return LIBUSB_ERROR_INVALID_PARAM;
 

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index b2e9924..a5c7b98 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10944
+#define LIBUSB_NANO 10946