From bf833ee6adf58bd4a4a468aa729cdc78bdc13ede Mon Sep 17 00:00:00 2001
From: Benjamin Berg <bberg@redhat.com>
Date: Tue, 15 Feb 2022 11:13:41 +0100
Subject: [PATCH 1/2] core: Catch NULL dev_handle when getting a transfer's
context
The dev_handle will be set to NULL when the transfer is still in-flight
while the device is closed. In that case, the transfer free function
will try to access the context and would run into a NULL pointer
dereference.
Add a test for dev_handle being valid before dereferencing it further.
---
libusb/libusbi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 158a9af5..dde43df2 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -330,7 +330,7 @@ void usbi_log(struct libusb_context *ctx, enum libusb_log_level level,
#define DEVICE_CTX(dev) ((dev)->ctx)
#define HANDLE_CTX(handle) (DEVICE_CTX((handle)->dev))
-#define TRANSFER_CTX(transfer) (HANDLE_CTX((transfer)->dev_handle))
+#define TRANSFER_CTX(transfer) ((transfer)->dev_handle ? HANDLE_CTX((transfer)->dev_handle) : NULL)
#define ITRANSFER_CTX(itransfer) \
(TRANSFER_CTX(USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer)))
From 6428090ea77dfb80906a146977ea7fd6de4718c8 Mon Sep 17 00:00:00 2001
From: Benjamin Berg <bberg@redhat.com>
Date: Tue, 15 Feb 2022 10:59:00 +0100
Subject: [PATCH 2/2] io: Unset dev_handle when removing transfer from flying
list
API users might hold on to transfers a bit longer than they are in the
flying list. If they then close the device prior to freeing all
transfers, we would end up with invalid pointers to the device.
Fix this by setting the device handle to NULL when removing the device
from the flying list. This matches the behaviour when the device is
closed while the transfer is still in the flying list.
Specifically, the libgusb wrapper will currently only free the
underlying transfer in a later mainloop iteration (as a side effect on
how GTask does memory management). It is possible to fix this, but it
would make memory management within libgusb much more error prone.
---
libusb/io.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libusb/io.c b/libusb/io.c
index 0d2ac9ea..4e6d8984 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1456,6 +1456,7 @@ static int add_to_flying_list(struct usbi_transfer *itransfer)
* if it fails to update the timer for the next timeout. */
static int remove_from_flying_list(struct usbi_transfer *itransfer)
{
+ struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
struct libusb_context *ctx = ITRANSFER_CTX(itransfer);
int rearm_timer;
int r = 0;
@@ -1466,6 +1467,7 @@ static int remove_from_flying_list(struct usbi_transfer *itransfer)
list_del(&itransfer->list);
if (rearm_timer)
r = arm_timer_for_next_timeout(ctx);
+ transfer->dev_handle = NULL;
usbi_mutex_unlock(&ctx->flying_transfers_lock);
return r;