diff options
| author | Joost Muller <joostmuller@gmail.com> | 2016-07-20 10:58:57 +0200 |
|---|---|---|
| committer | Nathan Hjelm <hjelmn@me.com> | 2016-07-21 22:59:35 -0600 |
| commit | bd8d5b5019b72b2dc2d074d96c9992e2f6e7e0b7 (patch) | |
| tree | 0a6b6d04620e03e683d858c79cb824873049b07a | |
| parent | 9afb5c0caf7778d8757ddabadca16aebd7819810 (diff) | |
| download | libusb-bd8d5b5019b72b2dc2d074d96c9992e2f6e7e0b7.tar.gz | |
io: Fix race condition in handle_timeout()
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>
| -rw-r--r-- | libusb/io.c | 57 | ||||
| -rw-r--r-- | libusb/version_nano.h | 2 |
2 files changed, 38 insertions, 21 deletions
diff --git a/libusb/io.c b/libusb/io.c index 4d03b8b..8944461 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -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) diff --git a/libusb/version_nano.h b/libusb/version_nano.h index 75b9b06..3687c25 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11119 +#define LIBUSB_NANO 11120 |