Fix a crash if a transfer outlives closing the device

Related: #1938801
This commit is contained in:
Benjamin Berg 2022-02-15 12:15:16 +01:00
parent 1eaf5782f0
commit fb8e249b1e
2 changed files with 79 additions and 1 deletions

72
1073.patch Normal file
View File

@ -0,0 +1,72 @@
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;

View File

@ -1,7 +1,7 @@
Summary: Library for accessing USB devices Summary: Library for accessing USB devices
Name: libusbx Name: libusbx
Version: 1.0.25 Version: 1.0.25
Release: 1%{?dist} Release: 2%{?dist}
# upstream libusbx has merged back with libusb and is now called libusb again # 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, renaming # but we already have a libusb package for the old libusb-compat-0.1, renaming
# that to libusb-compat while at the same time giving this its name is a bit # that to libusb-compat while at the same time giving this its name is a bit
@ -17,6 +17,8 @@ Obsoletes: libusb1 <= 1.0.9
# Fix a crash after libusb_exit API has been misused # Fix a crash after libusb_exit API has been misused
# https://bugzilla.redhat.com/show_bug.cgi?id=2050638 # https://bugzilla.redhat.com/show_bug.cgi?id=2050638
Patch0001: https://github.com/libusb/libusb/pull/1058.patch Patch0001: https://github.com/libusb/libusb/pull/1058.patch
# Fix a crash if a transfer outlives closing the device
Patch0002: https://github.com/libusb/libusb/pull/1073.patch
# Pull in coverity related fixes # Pull in coverity related fixes
Patch9999: https://github.com/libusb/libusb/pull/1067.patch Patch9999: https://github.com/libusb/libusb/pull/1067.patch
@ -126,6 +128,10 @@ LD_LIBRARY_PATH=libusb/.libs $RPM_BUILD_ROOT%{_bindir}/libusb-example-listdevs
%changelog %changelog
* Tue Feb 15 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-2
- Fix a crash if a transfer outlives closing the device
Related: #1938801
* Thu Feb 10 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-1 * Thu Feb 10 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-1
- Update to 1.0.25 - Update to 1.0.25
- Fix a crash after libusb_exit API has been misused - Fix a crash after libusb_exit API has been misused