diff --git a/.gitignore b/.gitignore index 7cd2193..8fccf6a 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ /libusb-1.0.19.tar.bz2 /libusb-1.0.20.tar.bz2 /libusb-1.0.21-448584a.tar.gz +/libusb-1.0.21-rc2.tar.gz diff --git a/libusb-1.0.21-rc2-locking-fixes.patch b/libusb-1.0.21-rc2-locking-fixes.patch new file mode 100644 index 0000000..dab8f79 --- /dev/null +++ b/libusb-1.0.21-rc2-locking-fixes.patch @@ -0,0 +1,1228 @@ +From d246f2e9880f8969b031775c1c0abff1bec2ce3f Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 10 Aug 2016 12:17:00 +0200 +Subject: [PATCH 01/11] Revert "io: Fix race condition in handle_timeout()" + +This reverts commit bd8d5b5019b72b2dc2d074d96c9992e2f6e7e0b7. + +Chris Dickens and me have been working on a patch-set refactoring +the transfer flag handling which fixes this differently. Revert +this commit so that the refactoring changes can be merged cleanly. + +Signed-off-by: Hans de Goede +--- + libusb/io.c | 57 ++++++++++++++++++--------------------------------- + libusb/version_nano.h | 2 +- + 2 files changed, 21 insertions(+), 38 deletions(-) + +diff --git a/libusb/io.c b/libusb/io.c +index 8944461..4d03b8b 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1530,34 +1530,6 @@ 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 +@@ -1581,9 +1553,27 @@ 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); + +- r = cancel_transfer_locked(transfer); ++ if (r == LIBUSB_ERROR_NO_DEVICE) ++ itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED; ++ } ++ ++ itransfer->flags |= USBI_TRANSFER_CANCELLING; + ++out: + usbi_mutex_unlock(&itransfer->flags_lock); + usbi_mutex_unlock(&itransfer->lock); + return r; +@@ -1977,20 +1967,13 @@ 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 = cancel_transfer_locked(transfer); +- ++ r = libusb_cancel_transfer(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) +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index a806b19..e92bc71 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11127 ++#define LIBUSB_NANO 11128 +-- +2.7.4 + +From cdea26fe7efeac3c41e2668d32c4ef7873298a9a Mon Sep 17 00:00:00 2001 +From: Chris Dickens +Date: Mon, 26 Oct 2015 14:11:53 +0100 +Subject: [PATCH 02/11] core: Change event handling lock to traditional + (non-recursive) type + +The event handling lock was previously required to be of the recursive +type because the libusb_close() path requires the lock and may be +called by a thread that is handling events (e.g. from within a +transfer or hotplug callback). With commit 960a6e75, it is possible to +determine whether the current function is being called from an event +handling context, thus the recursive lock type is no longer necessary. + +References: +* http://libusb.org/ticket/82 +* 74282582cc879f091ad1d847411337bc3fa78a2b +* c775c2f43037cd235b65410583179195e25f9c4a + +Signed-off-by: Chris Dickens +Reviewed-by: Hans de Goede +[hdegoede@redhat.com: rebase on top of current master] +Signed-off-by: Hans de Goede +--- + libusb/core.c | 60 ++++++++++++++++++++++++--------------------- + libusb/io.c | 2 +- + libusb/os/threads_posix.c | 22 ----------------- + libusb/os/threads_posix.h | 2 -- + libusb/os/threads_windows.h | 3 --- + libusb/version_nano.h | 2 +- + 6 files changed, 34 insertions(+), 57 deletions(-) + +diff --git a/libusb/core.c b/libusb/core.c +index 06342c8..2e3816c 100644 +--- a/libusb/core.c ++++ b/libusb/core.c +@@ -1340,8 +1340,6 @@ static void do_close(struct libusb_context *ctx, + struct usbi_transfer *itransfer; + struct usbi_transfer *tmp; + +- libusb_lock_events(ctx); +- + /* remove any transfers in flight that are for this device */ + usbi_mutex_lock(&ctx->flying_transfers_lock); + +@@ -1380,8 +1378,6 @@ static void do_close(struct libusb_context *ctx, + } + usbi_mutex_unlock(&ctx->flying_transfers_lock); + +- libusb_unlock_events(ctx); +- + usbi_mutex_lock(&ctx->open_devs_lock); + list_del(&dev_handle->list); + usbi_mutex_unlock(&ctx->open_devs_lock); +@@ -1406,6 +1402,7 @@ static void do_close(struct libusb_context *ctx, + void API_EXPORTED libusb_close(libusb_device_handle *dev_handle) + { + struct libusb_context *ctx; ++ int handling_events; + int pending_events; + + if (!dev_handle) +@@ -1413,39 +1410,46 @@ void API_EXPORTED libusb_close(libusb_device_handle *dev_handle) + usbi_dbg(""); + + ctx = HANDLE_CTX(dev_handle); ++ handling_events = usbi_handling_events(ctx); + + /* Similarly to libusb_open(), we want to interrupt all event handlers + * at this point. More importantly, we want to perform the actual close of + * the device while holding the event handling lock (preventing any other + * thread from doing event handling) because we will be removing a file +- * descriptor from the polling loop. */ +- +- /* Record that we are closing a device. +- * Only signal an event if there are no prior pending events. */ +- usbi_mutex_lock(&ctx->event_data_lock); +- pending_events = usbi_pending_events(ctx); +- ctx->device_close++; +- if (!pending_events) +- usbi_signal_event(ctx); +- usbi_mutex_unlock(&ctx->event_data_lock); +- +- /* take event handling lock */ +- libusb_lock_events(ctx); ++ * descriptor from the polling loop. If this is being called by the current ++ * event handler, we can bypass the interruption code because we already ++ * hold the event handling lock. */ ++ ++ if (!handling_events) { ++ /* Record that we are closing a device. ++ * Only signal an event if there are no prior pending events. */ ++ usbi_mutex_lock(&ctx->event_data_lock); ++ pending_events = usbi_pending_events(ctx); ++ ctx->device_close++; ++ if (!pending_events) ++ usbi_signal_event(ctx); ++ usbi_mutex_unlock(&ctx->event_data_lock); ++ ++ /* take event handling lock */ ++ libusb_lock_events(ctx); ++ } + + /* Close the device */ + do_close(ctx, dev_handle); + +- /* We're done with closing this device. +- * Clear the event pipe if there are no further pending events. */ +- usbi_mutex_lock(&ctx->event_data_lock); +- ctx->device_close--; +- pending_events = usbi_pending_events(ctx); +- if (!pending_events) +- usbi_clear_event(ctx); +- usbi_mutex_unlock(&ctx->event_data_lock); +- +- /* Release event handling lock and wake up event waiters */ +- libusb_unlock_events(ctx); ++ if (!handling_events) { ++ /* We're done with closing this device. ++ * Clear the event pipe if there are no further pending events. */ ++ usbi_mutex_lock(&ctx->event_data_lock); ++ ctx->device_close--; ++ pending_events = usbi_pending_events(ctx); ++ if (!pending_events) ++ usbi_clear_event(ctx); ++ usbi_mutex_unlock(&ctx->event_data_lock); ++ ++ /* Release event handling lock and wake up event waiters */ ++ libusb_unlock_events(ctx); ++ } + } + + /** \ingroup libusb_dev +diff --git a/libusb/io.c b/libusb/io.c +index 4d03b8b..3bd1675 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1125,7 +1125,7 @@ int usbi_io_init(struct libusb_context *ctx) + int r; + + usbi_mutex_init(&ctx->flying_transfers_lock); +- usbi_mutex_init_recursive(&ctx->events_lock); ++ usbi_mutex_init(&ctx->events_lock); + usbi_mutex_init(&ctx->event_waiters_lock); + usbi_cond_init(&ctx->event_waiters_cond); + usbi_mutex_init(&ctx->event_data_lock); +diff --git a/libusb/os/threads_posix.c b/libusb/os/threads_posix.c +index 3908907..a4f270b 100644 +--- a/libusb/os/threads_posix.c ++++ b/libusb/os/threads_posix.c +@@ -37,28 +37,6 @@ + #include "threads_posix.h" + #include "libusbi.h" + +-int usbi_mutex_init_recursive(pthread_mutex_t *mutex) +-{ +- int err; +- pthread_mutexattr_t attr; +- +- err = pthread_mutexattr_init(&attr); +- if (err != 0) +- return err; +- +- /* mutexattr_settype requires _GNU_SOURCE or _XOPEN_SOURCE >= 500 on Linux */ +- err = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); +- if (err != 0) +- goto finish; +- +- err = pthread_mutex_init(mutex, &attr); +- +-finish: +- pthread_mutexattr_destroy(&attr); +- +- return err; +-} +- + int usbi_cond_timedwait(pthread_cond_t *cond, + pthread_mutex_t *mutex, const struct timeval *tv) + { +diff --git a/libusb/os/threads_posix.h b/libusb/os/threads_posix.h +index 2abb820..7ec70b1 100644 +--- a/libusb/os/threads_posix.h ++++ b/libusb/os/threads_posix.h +@@ -50,8 +50,6 @@ + #define usbi_tls_key_set pthread_setspecific + #define usbi_tls_key_delete pthread_key_delete + +-int usbi_mutex_init_recursive(pthread_mutex_t *mutex); +- + int usbi_cond_timedwait(pthread_cond_t *cond, + pthread_mutex_t *mutex, const struct timeval *tv); + +diff --git a/libusb/os/threads_windows.h b/libusb/os/threads_windows.h +index 8b7faec..e97ee78 100644 +--- a/libusb/os/threads_windows.h ++++ b/libusb/os/threads_windows.h +@@ -71,9 +71,6 @@ void *usbi_tls_key_get(usbi_tls_key_t key); + int usbi_tls_key_set(usbi_tls_key_t key, void *value); + int usbi_tls_key_delete(usbi_tls_key_t key); + +-// all Windows mutexes are recursive +-#define usbi_mutex_init_recursive usbi_mutex_init +- + int usbi_get_tid(void); + + #endif /* LIBUSB_THREADS_WINDOWS_H */ +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index e92bc71..74e5d3b 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11128 ++#define LIBUSB_NANO 11129 +-- +2.7.4 + +From 3253bd9d5d583ce4a9b32d13be3bf038accee977 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 16:40:54 +0200 +Subject: [PATCH 03/11] wince: Do not differ between cancel / timeout ourselves + +(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by +usbi_handle_transfer_cancellation(), which wince_transfer_callback() +will call when status == LIBUSB_TRANSFER_CANCELLED. Leave this up +to the core, so that future changes to timeout handling do no break +wince. + +Signed-off-by: Hans de Goede +--- +Note: untested +--- + libusb/os/wince_usb.c | 9 ++------- + libusb/version_nano.h | 2 +- + 2 files changed, 3 insertions(+), 8 deletions(-) + +diff --git a/libusb/os/wince_usb.c b/libusb/os/wince_usb.c +index 85c584b..89b5e65 100644 +--- a/libusb/os/wince_usb.c ++++ b/libusb/os/wince_usb.c +@@ -720,13 +720,8 @@ static void wince_transfer_callback( + status = LIBUSB_TRANSFER_TIMED_OUT; + break; + case ERROR_OPERATION_ABORTED: +- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT) { +- usbi_dbg("detected timeout"); +- status = LIBUSB_TRANSFER_TIMED_OUT; +- } else { +- usbi_dbg("detected operation aborted"); +- status = LIBUSB_TRANSFER_CANCELLED; +- } ++ usbi_dbg("detected operation aborted"); ++ status = LIBUSB_TRANSFER_CANCELLED; + break; + default: + usbi_err(ITRANSFER_CTX(itransfer), "detected I/O error: %s", windows_error_str(io_result)); +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 74e5d3b..335ae07 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11129 ++#define LIBUSB_NANO 11130 +-- +2.7.4 + +From 916c1f49a20cbd151a8972bc21582dc311ed1124 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 16:43:18 +0200 +Subject: [PATCH 04/11] winnt: Do not differ between cancel / timeout ourselves + +(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by +usbi_handle_transfer_cancellation(), make windows_transfer_callback() +call usbi_handle_transfer_cancellation() when +status == LIBUSB_TRANSFER_CANCELLED like all other os backends do, and +leave USBI_TRANSFER_TIMED_OUT handling up to the core, so that future +changes to timeout handling do no break winnt. + +Signed-off-by: Hans de Goede +--- +Note: untested +--- + libusb/os/windows_nt_common.c | 16 +++++++--------- + libusb/version_nano.h | 2 +- + 2 files changed, 8 insertions(+), 10 deletions(-) + +diff --git a/libusb/os/windows_nt_common.c b/libusb/os/windows_nt_common.c +index 93e6278..68eb4a7 100644 +--- a/libusb/os/windows_nt_common.c ++++ b/libusb/os/windows_nt_common.c +@@ -501,14 +501,9 @@ static void windows_transfer_callback(struct usbi_transfer *itransfer, uint32_t + istatus = windows_copy_transfer_data(itransfer, io_size); + if (istatus != LIBUSB_TRANSFER_COMPLETED) + usbi_dbg("Failed to copy partial data in aborted operation: %d", istatus); +- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT) { +- usbi_dbg("detected timeout"); +- status = LIBUSB_TRANSFER_TIMED_OUT; +- } +- else { +- usbi_dbg("detected operation aborted"); +- status = LIBUSB_TRANSFER_CANCELLED; +- } ++ ++ usbi_dbg("detected operation aborted"); ++ status = LIBUSB_TRANSFER_CANCELLED; + break; + default: + usbi_err(ITRANSFER_CTX(itransfer), "detected I/O error %u: %s", io_result, windows_error_str(io_result)); +@@ -516,7 +511,10 @@ static void windows_transfer_callback(struct usbi_transfer *itransfer, uint32_t + break; + } + windows_clear_transfer_priv(itransfer); // Cancel polling +- usbi_handle_transfer_completion(itransfer, (enum libusb_transfer_status)status); ++ if (status == LIBUSB_TRANSFER_CANCELLED) ++ usbi_handle_transfer_cancellation(itransfer); ++ else ++ usbi_handle_transfer_completion(itransfer, (enum libusb_transfer_status)status); + } + + void windows_handle_callback(struct usbi_transfer *itransfer, uint32_t io_result, uint32_t io_size) +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 335ae07..3fdb625 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11130 ++#define LIBUSB_NANO 11131 +-- +2.7.4 + +From 5db9f1f0adb5556457859f329217fb031f2f5fb4 Mon Sep 17 00:00:00 2001 +From: Chris Dickens +Date: Mon, 26 Oct 2015 14:18:33 +0100 +Subject: [PATCH 05/11] core: Refactor code related to transfer flags and + timeout handling + +Commit a886bb02 sped up the library a bit by removing the serialization +of transfer submission with respect to the flying_transfers list, but +it introduced two separate issues. + +1) A deadlock scenario is possible given the following sequence: + + - Thread A submits transfer with very short timeout (say 1ms) + -> takes transfer->lock + -> adds transfer to flying_transfers list and arms timerfd + -> actually calls backend to submit transfer, but it fails + + - Thread B is doing event handling and sees the timerfd trigger + -> takes ctx->flying_transfers_lock + -> finds the transfer above on the list + -> calls libusb_cancel_transfer() for this transfer + --> takes transfer->lock + + - Thread A sees the transfer failed to submit + -> removes transfer from flying_transfers list + --> takes ctx->flying_transfers_lock (still holding transfer->lock) + ** DEADLOCK ** + +2) The transfer state flags (e.g. submitting, in-flight) were protected + by transfer->flags_lock, but the timeout-related flags were OR'ed in + during timeout handling operations outside of the lock. This leads to + the possibility that transfer state might get overwritten. + +This change corrects these issues and simplifies the transfer submission +code a bit by separating the state and timeout flags into their own flag +variables. The state flags are protected by the transfer lock. The timeout +flags are protected by the flying_transfers_lock. + +The transfer submission code sheds some weight because it no longer needs +to worry about the timing of events that modify the transfer state flags. +These flags are always viewed and modified under the protection of the +transfer lock. Since libusb_submit_transfer() holds the transfer lock for +the entire duration of the operation, the other code paths that would +possibly touch the transfer (e.g. usbi_handle_disconnect() and +usbi_handle_transfer_completion()) have to wait for transfer submission +to fully complete. This eliminates any possible race conditions. + +Signed-off-by: Chris Dickens +[hdegoede@redhat.com: Reworked libusb_submit_transfer changes so that in + case both flying_transfer_lock and itransfer->lock are taken + flying_transfers_lock is always taken first] +[hdegoede@redhat.com: Removed some unrelated changes (will be submitted + as separate patches)] +Signed-off-by: Hans de Goede +--- + libusb/core.c | 4 +- + libusb/io.c | 144 ++++++++++++++++++++++++++----------------------- + libusb/libusbi.h | 43 +++++++-------- + libusb/os/darwin_usb.c | 10 ++-- + libusb/version_nano.h | 2 +- + 5 files changed, 104 insertions(+), 99 deletions(-) + +diff --git a/libusb/core.c b/libusb/core.c +index 2e3816c..d1e93c8 100644 +--- a/libusb/core.c ++++ b/libusb/core.c +@@ -1351,10 +1351,10 @@ static void do_close(struct libusb_context *ctx, + if (transfer->dev_handle != dev_handle) + continue; + +- if (!(itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) { ++ if (!(itransfer->state_flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) { + usbi_err(ctx, "Device handle closed while transfer was still being processed, but the device is still connected as far as we know"); + +- if (itransfer->flags & USBI_TRANSFER_CANCELLING) ++ if (itransfer->state_flags & USBI_TRANSFER_CANCELLING) + usbi_warn(ctx, "A cancellation for an in-flight transfer hasn't completed but closing the device handle"); + else + usbi_err(ctx, "A cancellation hasn't even been scheduled on the transfer for which the device is closing"); +diff --git a/libusb/io.c b/libusb/io.c +index 3bd1675..3757f44 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1269,7 +1269,6 @@ struct libusb_transfer * LIBUSB_CALL libusb_alloc_transfer( + + itransfer->num_iso_packets = iso_packets; + usbi_mutex_init(&itransfer->lock); +- usbi_mutex_init(&itransfer->flags_lock); + transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); + usbi_dbg("transfer %p", transfer); + return transfer; +@@ -1304,7 +1303,6 @@ void API_EXPORTED libusb_free_transfer(struct libusb_transfer *transfer) + + itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer); + usbi_mutex_destroy(&itransfer->lock); +- usbi_mutex_destroy(&itransfer->flags_lock); + free(itransfer); + } + +@@ -1339,8 +1337,8 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) + if (!timerisset(cur_tv)) + goto disarm; + +- /* act on first transfer that is not already cancelled */ +- if (!(transfer->flags & USBI_TRANSFER_TIMEOUT_HANDLED)) { ++ /* act on first transfer that has not already been handled */ ++ if (!(transfer->timeout_flags & USBI_TRANSFER_TIMEOUT_HANDLED)) { + int r; + const struct itimerspec it = { {0, 0}, + { cur_tv->tv_sec, cur_tv->tv_usec * 1000 } }; +@@ -1374,8 +1372,6 @@ static int add_to_flying_list(struct usbi_transfer *transfer) + int r = 0; + int first = 1; + +- usbi_mutex_lock(&ctx->flying_transfers_lock); +- + /* if we have no other flying transfers, start the list with this one */ + if (list_empty(&ctx->flying_transfers)) { + list_add(&transfer->list, &ctx->flying_transfers); +@@ -1428,7 +1424,6 @@ out: + if (r) + list_del(&transfer->list); + +- usbi_mutex_unlock(&ctx->flying_transfers_lock); + return r; + } + +@@ -1471,62 +1466,79 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer) + { + struct usbi_transfer *itransfer = + LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer); +- int remove = 0; ++ struct libusb_context *ctx = TRANSFER_CTX(transfer); + int r; + + usbi_dbg("transfer %p", transfer); ++ ++ /* ++ * Important note on locking, this function takes / releases locks ++ * in the following order: ++ * take flying_transfers_lock ++ * take itransfer->lock ++ * clear transfer ++ * add to flying_transfers list ++ * release flying_transfers_lock ++ * submit transfer ++ * release itransfer->lock ++ * if submit failed: ++ * take flying_transfers_lock ++ * remove from flying_transfers list ++ * release flying_transfers_lock ++ * ++ * Note that it takes locks in the order a-b and then releases them ++ * in the same order a-b. This is somewhat unusual but not wrong, ++ * release order is not important as long as *all* locks are released ++ * before re-acquiring any locks. ++ * ++ * This means that the ordering of first releasing itransfer->lock ++ * and then re-acquiring the flying_transfers_list on error is ++ * important and must not be changed! ++ * ++ * This is done this way because when we take both locks we must always ++ * take flying_transfers_lock first to avoid ab-ba style deadlocks with ++ * the timeout handling and usbi_handle_disconnect paths. ++ * ++ * And we cannot release itransfer->lock before the submission is ++ * complete otherwise timeout handling for transfers with short ++ * timeouts may run before submission. ++ */ ++ usbi_mutex_lock(&ctx->flying_transfers_lock); + usbi_mutex_lock(&itransfer->lock); +- usbi_mutex_lock(&itransfer->flags_lock); +- if (itransfer->flags & USBI_TRANSFER_IN_FLIGHT) { +- r = LIBUSB_ERROR_BUSY; +- goto out; ++ if (itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT) { ++ usbi_mutex_unlock(&ctx->flying_transfers_lock); ++ usbi_mutex_unlock(&itransfer->lock); ++ return LIBUSB_ERROR_BUSY; + } + itransfer->transferred = 0; +- itransfer->flags = 0; ++ itransfer->state_flags = 0; ++ itransfer->timeout_flags = 0; + r = calculate_timeout(itransfer); + if (r < 0) { +- r = LIBUSB_ERROR_OTHER; +- goto out; ++ usbi_mutex_unlock(&ctx->flying_transfers_lock); ++ usbi_mutex_unlock(&itransfer->lock); ++ return LIBUSB_ERROR_OTHER; + } +- itransfer->flags |= USBI_TRANSFER_SUBMITTING; +- usbi_mutex_unlock(&itransfer->flags_lock); + + r = add_to_flying_list(itransfer); + if (r) { +- usbi_mutex_lock(&itransfer->flags_lock); +- itransfer->flags = 0; +- goto out; ++ usbi_mutex_unlock(&ctx->flying_transfers_lock); ++ usbi_mutex_unlock(&itransfer->lock); ++ return r; + } ++ usbi_mutex_unlock(&ctx->flying_transfers_lock); + +- /* keep a reference to this device */ +- libusb_ref_device(transfer->dev_handle->dev); + r = usbi_backend->submit_transfer(itransfer); +- +- usbi_mutex_lock(&itransfer->flags_lock); +- itransfer->flags &= ~USBI_TRANSFER_SUBMITTING; + if (r == LIBUSB_SUCCESS) { +- /* check for two possible special conditions: +- * 1) device disconnect occurred immediately after submission +- * 2) transfer completed before we got here to update the flags +- */ +- if (itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED) { +- usbi_backend->clear_transfer_priv(itransfer); +- remove = 1; +- r = LIBUSB_ERROR_NO_DEVICE; +- } +- else if (!(itransfer->flags & USBI_TRANSFER_COMPLETED)) { +- itransfer->flags |= USBI_TRANSFER_IN_FLIGHT; +- } +- } else { +- remove = 1; +- } +-out: +- usbi_mutex_unlock(&itransfer->flags_lock); +- if (remove) { +- libusb_unref_device(transfer->dev_handle->dev); +- remove_from_flying_list(itransfer); ++ itransfer->state_flags |= USBI_TRANSFER_IN_FLIGHT; ++ /* keep a reference to this device */ ++ libusb_ref_device(transfer->dev_handle->dev); + } + usbi_mutex_unlock(&itransfer->lock); ++ ++ if (r != LIBUSB_SUCCESS) ++ remove_from_flying_list(itransfer); ++ + return r; + } + +@@ -1552,9 +1564,8 @@ 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)) { ++ if (!(itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT) ++ || (itransfer->state_flags & USBI_TRANSFER_CANCELLING)) { + r = LIBUSB_ERROR_NOT_FOUND; + goto out; + } +@@ -1568,13 +1579,12 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer) + usbi_dbg("cancel transfer failed error %d", r); + + if (r == LIBUSB_ERROR_NO_DEVICE) +- itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED; ++ itransfer->state_flags |= USBI_TRANSFER_DEVICE_DISAPPEARED; + } + +- itransfer->flags |= USBI_TRANSFER_CANCELLING; ++ itransfer->state_flags |= USBI_TRANSFER_CANCELLING; + + out: +- usbi_mutex_unlock(&itransfer->flags_lock); + usbi_mutex_unlock(&itransfer->lock); + return r; + } +@@ -1637,10 +1647,9 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer, + if (r < 0) + usbi_err(ITRANSFER_CTX(itransfer), "failed to set timer for next timeout, errno=%d", errno); + +- usbi_mutex_lock(&itransfer->flags_lock); +- itransfer->flags &= ~USBI_TRANSFER_IN_FLIGHT; +- itransfer->flags |= USBI_TRANSFER_COMPLETED; +- usbi_mutex_unlock(&itransfer->flags_lock); ++ usbi_mutex_lock(&itransfer->lock); ++ itransfer->state_flags &= ~USBI_TRANSFER_IN_FLIGHT; ++ usbi_mutex_unlock(&itransfer->lock); + + if (status == LIBUSB_TRANSFER_COMPLETED + && transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) { +@@ -1676,7 +1685,7 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer, + int usbi_handle_transfer_cancellation(struct usbi_transfer *transfer) + { + /* if the URB was cancelled due to timeout, report timeout to the user */ +- if (transfer->flags & USBI_TRANSFER_TIMED_OUT) { ++ if (transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) { + usbi_dbg("detected timeout cancellation"); + return usbi_handle_transfer_completion(transfer, LIBUSB_TRANSFER_TIMED_OUT); + } +@@ -1967,10 +1976,10 @@ static void handle_timeout(struct usbi_transfer *itransfer) + USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); + int r; + +- itransfer->flags |= USBI_TRANSFER_TIMEOUT_HANDLED; ++ itransfer->timeout_flags |= USBI_TRANSFER_TIMEOUT_HANDLED; + r = libusb_cancel_transfer(transfer); + if (r == 0) +- itransfer->flags |= USBI_TRANSFER_TIMED_OUT; ++ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT; + else + usbi_warn(TRANSFER_CTX(transfer), + "async cancel failed %d errno=%d", r, errno); +@@ -2003,7 +2012,7 @@ static int handle_timeouts_locked(struct libusb_context *ctx) + return 0; + + /* ignore timeouts we've already handled */ +- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT)) ++ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT)) + continue; + + /* if transfer has non-expired timeout, nothing more to do */ +@@ -2549,7 +2558,7 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx, + + /* find next transfer which hasn't already been processed as timed out */ + list_for_each_entry(transfer, &ctx->flying_transfers, list, struct usbi_transfer) { +- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT)) ++ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT)) + continue; + + /* if we've reached transfers of infinte timeout, we're done looking */ +@@ -2766,9 +2775,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *dev_handle) + * possible scenarios: + * 1. the transfer is currently in-flight, in which case we terminate the + * transfer here +- * 2. the transfer is not in-flight (or is but hasn't been marked as such), +- * in which case we record that the device disappeared and this will be +- * handled by libusb_submit_transfer() ++ * 2. the transfer has been added to the flying transfer list by ++ * libusb_submit_transfer, has failed to submit and ++ * libusb_submit_transfer is waiting for us to release the ++ * flying_transfers_lock to remove it, so we ignore it + */ + + while (1) { +@@ -2776,12 +2786,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *dev_handle) + usbi_mutex_lock(&HANDLE_CTX(dev_handle)->flying_transfers_lock); + list_for_each_entry(cur, &HANDLE_CTX(dev_handle)->flying_transfers, list, struct usbi_transfer) + if (USBI_TRANSFER_TO_LIBUSB_TRANSFER(cur)->dev_handle == dev_handle) { +- usbi_mutex_lock(&cur->flags_lock); +- if (cur->flags & USBI_TRANSFER_IN_FLIGHT) ++ usbi_mutex_lock(&cur->lock); ++ if (cur->state_flags & USBI_TRANSFER_IN_FLIGHT) + to_cancel = cur; +- else +- cur->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED; +- usbi_mutex_unlock(&cur->flags_lock); ++ usbi_mutex_unlock(&cur->lock); + + if (to_cancel) + break; +diff --git a/libusb/libusbi.h b/libusb/libusbi.h +index ca54a46..cc0906c 100644 +--- a/libusb/libusbi.h ++++ b/libusb/libusbi.h +@@ -277,6 +277,8 @@ struct libusb_context { + * the list, URBs that will time out later are placed after, and urbs with + * infinite timeout are always placed at the very end. */ + struct list_head flying_transfers; ++ /* Note paths taking both this and usbi_transfer->lock must always ++ * take this lock first */ + usbi_mutex_t flying_transfers_lock; + + /* user callbacks for pollfd changes */ +@@ -440,7 +442,8 @@ struct usbi_transfer { + struct timeval timeout; + int transferred; + uint32_t stream_id; +- uint8_t flags; ++ uint8_t state_flags; /* Protected by usbi_transfer->lock */ ++ uint8_t timeout_flags; /* Protected by the flying_stransfers_lock */ + + /* this lock is held during libusb_submit_transfer() and + * libusb_cancel_transfer() (allowing the OS backend to prevent duplicate +@@ -448,38 +451,32 @@ struct usbi_transfer { + * should also take this lock in the handle_events path, to prevent the user + * cancelling the transfer from another thread while you are processing + * its completion (presumably there would be races within your OS backend +- * if this were possible). */ ++ * if this were possible). ++ * Note paths taking both this and the flying_transfers_lock must ++ * always take the flying_transfers_lock first */ + usbi_mutex_t lock; +- +- /* this lock should be held whenever viewing or modifying flags +- * relating to the transfer state */ +- usbi_mutex_t flags_lock; + }; + +-enum usbi_transfer_flags { +- /* The transfer has timed out */ +- USBI_TRANSFER_TIMED_OUT = 1 << 0, +- +- /* Set by backend submit_transfer() if the OS handles timeout */ +- USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 1, ++enum usbi_transfer_state_flags { ++ /* Transfer successfully submitted by backend */ ++ USBI_TRANSFER_IN_FLIGHT = 1 << 0, + + /* Cancellation was requested via libusb_cancel_transfer() */ +- USBI_TRANSFER_CANCELLING = 1 << 2, ++ USBI_TRANSFER_CANCELLING = 1 << 1, + + /* Operation on the transfer failed because the device disappeared */ +- USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 3, +- +- /* Transfer is currently being submitted */ +- USBI_TRANSFER_SUBMITTING = 1 << 4, +- +- /* Transfer successfully submitted by backend */ +- USBI_TRANSFER_IN_FLIGHT = 1 << 5, ++ USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 2, ++}; + +- /* Completion handler has run */ +- USBI_TRANSFER_COMPLETED = 1 << 6, ++enum usbi_transfer_timeout_flags { ++ /* Set by backend submit_transfer() if the OS handles timeout */ ++ USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 0, + + /* The transfer timeout has been handled */ +- USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 7, ++ USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 1, ++ ++ /* The transfer timeout was successfully processed */ ++ USBI_TRANSFER_TIMED_OUT = 1 << 2, + }; + + #define USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer) \ +diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c +index 4e4ab04..7912282 100644 +--- a/libusb/os/darwin_usb.c ++++ b/libusb/os/darwin_usb.c +@@ -1561,7 +1561,7 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer) { + ret = (*(cInterface->interface))->WritePipeAsync(cInterface->interface, pipeRef, transfer->buffer, + transfer->length, darwin_async_io_callback, itransfer); + } else { +- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; ++ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; + + if (IS_XFERIN(transfer)) + ret = (*(cInterface->interface))->ReadPipeAsyncTO(cInterface->interface, pipeRef, transfer->buffer, +@@ -1593,7 +1593,7 @@ static int submit_stream_transfer(struct usbi_transfer *itransfer) { + return LIBUSB_ERROR_NOT_FOUND; + } + +- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; ++ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; + + if (IS_XFERIN(transfer)) + ret = (*(cInterface->interface))->ReadStreamsPipeAsyncTO(cInterface->interface, pipeRef, itransfer->stream_id, +@@ -1721,7 +1721,7 @@ static int submit_control_transfer(struct usbi_transfer *itransfer) { + tpriv->req.completionTimeout = transfer->timeout; + tpriv->req.noDataTimeout = transfer->timeout; + +- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; ++ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT; + + /* all transfers in libusb-1.0 are async */ + +@@ -1870,7 +1870,7 @@ static void darwin_async_io_callback (void *refcon, IOReturn result, void *arg0) + } + + static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_t result) { +- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT) ++ if (itransfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) + result = kIOUSBTransactionTimeout; + + switch (result) { +@@ -1887,7 +1887,7 @@ static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_ + return LIBUSB_TRANSFER_OVERFLOW; + case kIOUSBTransactionTimeout: + usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: timed out"); +- itransfer->flags |= USBI_TRANSFER_TIMED_OUT; ++ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT; + return LIBUSB_TRANSFER_TIMED_OUT; + default: + usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: %s (value = 0x%08x)", darwin_error_str (result), result); +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 3fdb625..de73e86 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11131 ++#define LIBUSB_NANO 11132 +-- +2.7.4 + +From df1d10813702d572e55a27dbd9eea918dd02d24d Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 16:53:53 +0200 +Subject: [PATCH 06/11] core: Do not arm timer-fd for transfers where the os + handles timeout + +Signed-off-by: Hans de Goede +--- + libusb/io.c | 2 +- + libusb/version_nano.h | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/libusb/io.c b/libusb/io.c +index 3757f44..13388cc 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1338,7 +1338,7 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) + goto disarm; + + /* act on first transfer that has not already been handled */ +- if (!(transfer->timeout_flags & USBI_TRANSFER_TIMEOUT_HANDLED)) { ++ if (!(transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))) { + int r; + const struct itimerspec it = { {0, 0}, + { cur_tv->tv_sec, cur_tv->tv_usec * 1000 } }; +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index de73e86..f39bf48 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11132 ++#define LIBUSB_NANO 11133 +-- +2.7.4 + +From 1277fdd207fe041d0afc5e70b80aeb803f71f402 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 16:54:42 +0200 +Subject: [PATCH 07/11] core: Test for LIBUSB_SUCCESS instead of 0 in + handle_timeout() + +Signed-off-by: Hans de Goede +--- + libusb/io.c | 2 +- + libusb/version_nano.h | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/libusb/io.c b/libusb/io.c +index 13388cc..aa6cfca 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1978,7 +1978,7 @@ static void handle_timeout(struct usbi_transfer *itransfer) + + itransfer->timeout_flags |= USBI_TRANSFER_TIMEOUT_HANDLED; + r = libusb_cancel_transfer(transfer); +- if (r == 0) ++ if (r == LIBUSB_SUCCESS) + itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT; + else + usbi_warn(TRANSFER_CTX(transfer), +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index f39bf48..9fb8855 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11133 ++#define LIBUSB_NANO 11134 +-- +2.7.4 + +From ac98fdac04d409c63cae5416356414c14a37809a Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 17:42:12 +0200 +Subject: [PATCH 08/11] core: Fix do_close locking + +Put the lock / unlock calls around the part of the code which actually +checks the flags which the lock protect. + +Signed-off-by: Hans de Goede +--- + libusb/core.c | 4 ++-- + libusb/version_nano.h | 2 +- + 2 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/libusb/core.c b/libusb/core.c +index d1e93c8..99aab7b 100644 +--- a/libusb/core.c ++++ b/libusb/core.c +@@ -1351,6 +1351,7 @@ static void do_close(struct libusb_context *ctx, + if (transfer->dev_handle != dev_handle) + continue; + ++ usbi_mutex_lock(&itransfer->lock); + if (!(itransfer->state_flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) { + usbi_err(ctx, "Device handle closed while transfer was still being processed, but the device is still connected as far as we know"); + +@@ -1359,15 +1360,14 @@ static void do_close(struct libusb_context *ctx, + else + usbi_err(ctx, "A cancellation hasn't even been scheduled on the transfer for which the device is closing"); + } ++ usbi_mutex_unlock(&itransfer->lock); + + /* remove from the list of in-flight transfers and make sure + * we don't accidentally use the device handle in the future + * (or that such accesses will be easily caught and identified as a crash) + */ +- usbi_mutex_lock(&itransfer->lock); + list_del(&itransfer->list); + transfer->dev_handle = NULL; +- usbi_mutex_unlock(&itransfer->lock); + + /* it is up to the user to free up the actual transfer struct. this is + * just making sure that we don't attempt to process the transfer after +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 9fb8855..6995b33 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11134 ++#define LIBUSB_NANO 11135 +-- +2.7.4 + +From 8380f1a8a41eab5afb9b5240b0b7db22dce28f8e Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Mon, 6 Jun 2016 17:43:23 +0200 +Subject: [PATCH 09/11] core: Fix unlocked access to timeout_flags + +There is a race between handle_timeout() and 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 USBI_TRANSFER_TIMED_OUT is not yet set in timeout_flags. +Therefore, usbi_handle_transfer_completion() is sometimes called +with LIBUSB_TRANSFER_CANCELLED instead of the expected +LIBUSB_TRANSFER_TIMED_OUT. + +timeout_flags is protected by the flying_transfers_lock, this commit +makes usbi_handle_transfer_cancellation() take that lock before +checking for USBI_TRANSFER_TIMED_OUT in timeout_flags, fixing this. + +Reported-by: Joost Muller +Signed-off-by: Hans de Goede +--- + libusb/io.c | 9 ++++++++- + libusb/version_nano.h | 2 +- + 2 files changed, 9 insertions(+), 2 deletions(-) + +diff --git a/libusb/io.c b/libusb/io.c +index aa6cfca..b3f7df0 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1684,8 +1684,15 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer, + * will attempt to take the lock. */ + int usbi_handle_transfer_cancellation(struct usbi_transfer *transfer) + { ++ struct libusb_context *ctx = ITRANSFER_CTX(transfer); ++ uint8_t timed_out; ++ ++ usbi_mutex_lock(&ctx->flying_transfers_lock); ++ timed_out = transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT; ++ usbi_mutex_unlock(&ctx->flying_transfers_lock); ++ + /* if the URB was cancelled due to timeout, report timeout to the user */ +- if (transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) { ++ if (timed_out) { + usbi_dbg("detected timeout cancellation"); + return usbi_handle_transfer_completion(transfer, LIBUSB_TRANSFER_TIMED_OUT); + } +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 6995b33..bb7624f 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11135 ++#define LIBUSB_NANO 11137 +-- +2.7.4 + +From 23a8fb757f7eeca4850116b88aac2ea8d34b935f Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 8 Jun 2016 10:34:05 +0200 +Subject: [PATCH 10/11] core: Move calculate_timeout call to + add_to_flying_transfers + +This cleans-up libusb_submit_transfer a bit by avoiding an error +exit path with unlock calls. + +Signed-off-by: Hans de Goede +--- + libusb/io.c | 13 +++++-------- + 1 file changed, 5 insertions(+), 8 deletions(-) + +diff --git a/libusb/io.c b/libusb/io.c +index b3f7df0..8363628 100644 +--- a/libusb/io.c ++++ b/libusb/io.c +@@ -1369,9 +1369,13 @@ static int add_to_flying_list(struct usbi_transfer *transfer) + struct usbi_transfer *cur; + struct timeval *timeout = &transfer->timeout; + struct libusb_context *ctx = ITRANSFER_CTX(transfer); +- int r = 0; ++ int r; + int first = 1; + ++ r = calculate_timeout(transfer); ++ if (r) ++ return r; ++ + /* if we have no other flying transfers, start the list with this one */ + if (list_empty(&ctx->flying_transfers)) { + list_add(&transfer->list, &ctx->flying_transfers); +@@ -1513,13 +1517,6 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer) + itransfer->transferred = 0; + itransfer->state_flags = 0; + itransfer->timeout_flags = 0; +- r = calculate_timeout(itransfer); +- if (r < 0) { +- usbi_mutex_unlock(&ctx->flying_transfers_lock); +- usbi_mutex_unlock(&itransfer->lock); +- return LIBUSB_ERROR_OTHER; +- } +- + r = add_to_flying_list(itransfer); + if (r) { + usbi_mutex_unlock(&ctx->flying_transfers_lock); +-- +2.7.4 + +From de378ea51c22eb7529812a69ab74a3b3eef33423 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Tue, 7 Jun 2016 12:12:38 +0200 +Subject: [PATCH 11/11] linux_usbfs: Deal with receiving POLLERR before all + transfers have completed + +The linux kernel will set its internal device state to USB_STATE_NOTATTACHED +as soon as it detects the disconnect, and then start a worker thread to +deal with the actual disconnection, kill outstanding urbs, etc. + +The usbfs poll implementation will return POLL_ERR as soon as +ps->dev->state == USB_STATE_NOTATTACHED. The kernel will not wakeup +the poll until it is done with processing the disconnection. But if +we happen to call poll() between the state change and the disconnection +being fully processed, we may not be able to reap all outstanding transfers, +even on kernels with the USBFS_CAP_REAP_AFTER_DISCONNECT capability. + +This commit deals with this by trying to reap as many transfers as possible +on disconnect on USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels and then +calling usbi_handle_disconnect(handle) to deal with any remaining ones. +On USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels this will be a no-op +unless we hit the race. + +Signed-off-by: Hans de Goede +--- + libusb/os/linux_usbfs.c | 10 +++++++--- + libusb/version_nano.h | 2 +- + 2 files changed, 8 insertions(+), 4 deletions(-) + +diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c +index 0689894..9cbeb80 100644 +--- a/libusb/os/linux_usbfs.c ++++ b/libusb/os/linux_usbfs.c +@@ -2638,10 +2638,14 @@ static int op_handle_events(struct libusb_context *ctx, + handle->dev->device_address); + usbi_mutex_static_unlock(&linux_hotplug_lock); + +- if (!(hpriv->caps & USBFS_CAP_REAP_AFTER_DISCONNECT)) { +- usbi_handle_disconnect(handle); +- continue; ++ if (hpriv->caps & USBFS_CAP_REAP_AFTER_DISCONNECT) { ++ do { ++ r = reap_for_handle(handle); ++ } while (r == 0); + } ++ ++ usbi_handle_disconnect(handle); ++ continue; + } + + do { +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index bb7624f..b394a2e 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 11137 ++#define LIBUSB_NANO 11138 +-- +2.7.4 + diff --git a/libusbx.spec b/libusbx.spec index 3cb3005..89cd16b 100644 --- a/libusbx.spec +++ b/libusbx.spec @@ -1,15 +1,13 @@ -%global commit 448584afd1b012210e8e9038f888597588450d7a -%global shortcommit %(c=%{commit}; echo ${c:0:7}) - Summary: Library for accessing USB devices Name: libusbx Version: 1.0.21 -Release: 0.1.git%{shortcommit}%{?dist} +Release: 0.2.rc2%{?dist} # upstream libusbx has merged back with libusb and is now called libusb again # but we already have a libusb package for the old libusb-compat-0.1, so # lets stick with the libusbx name for now #Source0: http://downloads.sourceforge.net/libusb/libusb-%{version}.tar.bz2 -Source0: https://github.com/libusb/libusb/archive/%{commit}/libusb-%{version}-%{shortcommit}.tar.gz +Source0: https://github.com/libusb/libusb/archive/v1.0.21-rc2/libusb-%{version}-rc2.tar.gz +Patch0: libusb-1.0.21-rc2-locking-fixes.patch License: LGPLv2+ Group: System Environment/Libraries URL: http://libusb.info @@ -53,7 +51,8 @@ This package contains API documentation for %{name}. %prep -%setup -q -n libusb-%{commit} +%setup -q -n libusb-%{version}-rc2 +%patch0 -p1 ./bootstrap.sh @@ -76,7 +75,7 @@ rm $RPM_BUILD_ROOT%{_libdir}/*.la %files %license COPYING -%doc AUTHORS README ChangeLog +%doc AUTHORS README.md ChangeLog %{_libdir}/*.so.* %files devel @@ -89,6 +88,10 @@ rm $RPM_BUILD_ROOT%{_libdir}/*.la %changelog +* Wed Aug 10 2016 Hans de Goede - 1.0.21-0.2.rc2 +- Update to 1.0.21-rc2 upstream release +- Add a bunch of locking fixes which are pending upstream + * Tue Feb 23 2016 Hans de Goede - 1.0.21-0.1.git448584a - Update to a pre 1.0.21 git snapshot to bring in libusb_interrupt_event_handler which chromium needs (rhbz#1270324) diff --git a/sources b/sources index afd6d32..8718167 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -da854095c07c35f2100bcfbd363c1b36 libusb-1.0.21-448584a.tar.gz +e64daaa06e11c0e95b1f8fb5715d50f7 libusb-1.0.21-rc2.tar.gz