Without this change the Windows backend needed to call usbi_fd_notification()
from within the backend's submit_transfer. This could cause deadlock
when attempting to lock the event lock if another thread was processing
events on the just-submitted transfer.
The deadlock comes about as the thread calling libusb_submit_transfer acquires
the transfer mutex before trying to acquire the event lock; this is the other
order of lock acquisition from an event thread handling activity on the just
submitted transfer. This could lead to one of two deadlocks:
1) If the transfer completes while usbi_fd_notification() is waiting for
the event lock and the callback attempts to resubmit the transfer.
2) If the transfer timeout is hit while usbi_fd_notification() is waiting
for the event lock then the attempt to cancel the transfer will deadlock.
This patch fixes both of these deadlocks by having libusb_submit_transfer()
only call usbi_fd_notification() after having released the transfer mutex.
Signed-off-by: Toby Gray <toby.g...@realvnc.com>
---
libusb/io.c | 7 ++++++-
libusb/libusbi.h | 10 +++++++++-
libusb/os/darwin_usb.c | 2 +-
libusb/os/linux_usbfs.c | 2 +-
libusb/os/openbsd_usb.c | 4 ++--
libusb/os/windows_usb.c | 22 ++++++++++------------
6 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index d7fae7e..ccdc402 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1292,6 +1292,7 @@ int API_EXPORTED libusb_submit_transfer(struct
libusb_transfer *transfer)
LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
int r;
int first;
+ int fd_updated = 0;
usbi_mutex_lock(&itransfer->lock);
itransfer->transferred = 0;
@@ -1303,7 +1304,7 @@ int API_EXPORTED libusb_submit_transfer(struct
libusb_transfer *transfer)
}
first = add_to_flying_list(itransfer);
- r = usbi_backend->submit_transfer(itransfer);
+ r = usbi_backend->submit_transfer(itransfer, &fd_updated);
if (r) {
usbi_mutex_lock(&ctx->flying_transfers_lock);
list_del(&itransfer->list);
@@ -1326,6 +1327,10 @@ int API_EXPORTED libusb_submit_transfer(struct
libusb_transfer *transfer)
out:
usbi_mutex_unlock(&itransfer->lock);
+ /* Now that the lock is released it's safe to call
+ * usbi_fd_notification() if needed. */
+ if (fd_updated)
+ usbi_fd_notification(ctx);
return r;
}
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index c3d2158..68bddf4 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -792,12 +792,20 @@ struct usbi_os_backend {
*
* This function must not block.
*
+ * This function is provided with a pointer to an int. It should
+ * set this int to 1 if a call to usbi_fd_notification is
+ * required (due to changes to the list of fds).
+ *
+ * The implementation of submit_transfer must not call
+ * usbi_fd_notification itself due to it blocking while waiting
+ * for the event lock.
+ *
* Return:
* - 0 on success
* - LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
* - another LIBUSB_ERROR code on other failure
*/
- int (*submit_transfer)(struct usbi_transfer *itransfer);
+ int (*submit_transfer)(struct usbi_transfer *itransfer, int
*updated_fds);
/* Cancel a previously submitted transfer.
*
diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c
index e6e514e..6b49acf 100644
--- a/libusb/os/darwin_usb.c
+++ b/libusb/os/darwin_usb.c
@@ -1444,7 +1444,7 @@ static int submit_control_transfer(struct usbi_transfer
*itransfer) {
return darwin_to_libusb (kresult);
}
-static int darwin_submit_transfer(struct usbi_transfer *itransfer) {
+static int darwin_submit_transfer(struct usbi_transfer *itransfer, int *
/*updated_fds*/) {
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
switch (transfer->type) {
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index a6114ca..eb4a1e9 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1893,7 +1893,7 @@ static int submit_control_transfer(struct usbi_transfer
*itransfer)
return 0;
}
-static int op_submit_transfer(struct usbi_transfer *itransfer)
+static int op_submit_transfer(struct usbi_transfer *itransfer, int *
/*updated_fds*/)
{
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
diff --git a/libusb/os/openbsd_usb.c b/libusb/os/openbsd_usb.c
index 9f909aa..8686424 100644
--- a/libusb/os/openbsd_usb.c
+++ b/libusb/os/openbsd_usb.c
@@ -71,7 +71,7 @@ static int obsd_clear_halt(struct libusb_device_handle *,
unsigned char);
static int obsd_reset_device(struct libusb_device_handle *);
static void obsd_destroy_device(struct libusb_device *);
-static int obsd_submit_transfer(struct usbi_transfer *);
+static int obsd_submit_transfer(struct usbi_transfer *, int * /*updated_fds*/);
static int obsd_cancel_transfer(struct usbi_transfer *);
static void obsd_clear_transfer_priv(struct usbi_transfer *);
static int obsd_handle_events(struct libusb_context *ctx, struct pollfd *,
@@ -421,7 +421,7 @@ obsd_destroy_device(struct libusb_device *dev)
}
int
-obsd_submit_transfer(struct usbi_transfer *itransfer)
+obsd_submit_transfer(struct usbi_transfer *itransfer, int * /*updated_fds*/)
{
struct libusb_transfer *transfer;
struct handle_priv *hpriv;
diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 1957964..db1ef98 100644
--- a/libusb/os/windows_usb.c
+++ b/libusb/os/windows_usb.c
@@ -46,8 +46,6 @@
#define LOOP_CHECK(fcall) { r=fcall; if (r != LIBUSB_SUCCESS) continue; }
#define LOOP_BREAK(err) { r=err; continue; }
-extern void usbi_fd_notification(struct libusb_context *ctx);
-
// Helper prototypes
static int windows_get_active_config_descriptor(struct libusb_device *dev,
unsigned char *buffer, size_t len, int *host_endian);
static int windows_clock_gettime(int clk_id, struct timespec *tp);
@@ -1754,7 +1752,7 @@ static void windows_clear_transfer_priv(struct
usbi_transfer *itransfer)
auto_release(itransfer);
}
-static int submit_bulk_transfer(struct usbi_transfer *itransfer)
+static int submit_bulk_transfer(struct usbi_transfer *itransfer, int
*updated_fds)
{
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
struct libusb_context *ctx = DEVICE_CTX(transfer->dev_handle->dev);
@@ -1770,11 +1768,11 @@ static int submit_bulk_transfer(struct usbi_transfer
*itransfer)
usbi_add_pollfd(ctx, transfer_priv->pollable_fd.fd,
(short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT));
- usbi_fd_notification(ctx);
+ *updated_fds = 1;
return LIBUSB_SUCCESS;
}
-static int submit_iso_transfer(struct usbi_transfer *itransfer)
+static int submit_iso_transfer(struct usbi_transfer *itransfer, int
*updated_fds)
{
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
struct libusb_context *ctx = DEVICE_CTX(transfer->dev_handle->dev);
@@ -1790,11 +1788,11 @@ static int submit_iso_transfer(struct usbi_transfer
*itransfer)
usbi_add_pollfd(ctx, transfer_priv->pollable_fd.fd,
(short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT));
- usbi_fd_notification(ctx);
+ *updated_fds = 1;
return LIBUSB_SUCCESS;
}
-static int submit_control_transfer(struct usbi_transfer *itransfer)
+static int submit_control_transfer(struct usbi_transfer *itransfer, int
*updated_fds)
{
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
struct libusb_context *ctx = DEVICE_CTX(transfer->dev_handle->dev);
@@ -1809,26 +1807,26 @@ static int submit_control_transfer(struct usbi_transfer
*itransfer)
usbi_add_pollfd(ctx, transfer_priv->pollable_fd.fd, POLLIN);
- usbi_fd_notification(ctx);
+ *updated_fds = 1;
return LIBUSB_SUCCESS;
}
-static int windows_submit_transfer(struct usbi_transfer *itransfer)
+static int windows_submit_transfer(struct usbi_transfer *itransfer, int
*updated_fds)
{
struct libusb_transfer *transfer =
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
switch (transfer->type) {
case LIBUSB_TRANSFER_TYPE_CONTROL:
- return submit_control_transfer(itransfer);
+ return submit_control_transfer(itransfer, updated_fds);
case LIBUSB_TRANSFER_TYPE_BULK:
case LIBUSB_TRANSFER_TYPE_INTERRUPT:
if (IS_XFEROUT(transfer) &&
transfer->flags & LIBUSB_TRANSFER_ADD_ZERO_PACKET)
return LIBUSB_ERROR_NOT_SUPPORTED;
- return submit_bulk_transfer(itransfer);
+ return submit_bulk_transfer(itransfer, updated_fds);
case LIBUSB_TRANSFER_TYPE_ISOCHRONOUS:
- return submit_iso_transfer(itransfer);
+ return submit_iso_transfer(itransfer, updated_fds);
default:
usbi_err(TRANSFER_CTX(transfer), "unknown endpoint type %d",
transfer->type);
return LIBUSB_ERROR_INVALID_PARAM;
--
1.7.8.msysgit.0
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
