From 859d6a961e11171ce06d2e5b761ec096a3d0bd26 Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Thu, 8 Jan 2015 12:45:56 -0800 Subject: [PATCH] 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 --- libusb/io.c | 19 +++++++++++++++++-- libusb/libusbi.h | 3 +++ libusb/os/linux_usbfs.c | 9 --------- libusb/version_nano.h | 2 +- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/libusb/io.c b/libusb/io.c index 4ea6ad2..9614086 100644 --- 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 --- 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 --- 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 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10944 +#define LIBUSB_NANO 10946 -- 2.7.4