Fix a crash if a transfer outlives closing the device

Resolves: #2050638

.gitignore

@@ -1 +1,2 @@
 /libusb-1.0.24.tar.bz2
+/libusb-1.0.25.tar.bz2

0001-linux_usbfs-Accept-sysfs-attributes-not-terminated-w.patch

@@ -1,60 +0,0 @@
-From c486d01297a366aae8dcd3f715d0bfd8b995949b Mon Sep 17 00:00:00 2001
-From: Chris Dickens <christopher.a.dickens@gmail.com>
-Date: Mon, 8 Feb 2021 09:27:20 -0800
-Subject: [PATCH 1/2] linux_usbfs: Accept sysfs attributes not terminated with
- newline
-
-Benjamin Berg reports that some CI systems that simulate sysfs devices
-are causing libusb to report errors because such systems are not
-conforming to the sysfs pattern of terminating attribute values with a
-newline character. Reduce the severity of encountering such
-non-conforming attibute values from an error that prevents enumeration
-to a warning message.
-
-Closes #857
-
-Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
----
- libusb/os/linux_usbfs.c | 12 +++++++-----
- libusb/version_nano.h   |  2 +-
- 2 files changed, 8 insertions(+), 6 deletions(-)
-
-diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
-index 4882c0f..ebf8cfe 100644
---- a/libusb/os/linux_usbfs.c
-+++ b/libusb/os/linux_usbfs.c
-@@ -505,7 +505,7 @@ static int read_sysfs_attr(struct libusb_context *ctx,
- 	if (fd < 0)
- 		return fd;
- 
--	r = read(fd, buf, sizeof(buf));
-+	r = read(fd, buf, sizeof(buf) - 1);
- 	if (r < 0) {
- 		r = errno;
- 		close(fd);
-@@ -523,16 +523,18 @@ static int read_sysfs_attr(struct libusb_context *ctx,
- 		return 0;
- 	}
- 
--	/* The kernel does *not* NULL-terminate the string, but every attribute
-+	/* The kernel does *not* NUL-terminate the string, but every attribute
- 	 * should be terminated with a newline character. */
- 	if (!isdigit(buf[0])) {
- 		usbi_err(ctx, "attribute %s doesn't have numeric value?", attr);
- 		return LIBUSB_ERROR_IO;
- 	} else if (buf[r - 1] != '\n') {
--		usbi_err(ctx, "attribute %s doesn't end with newline?", attr);
--		return LIBUSB_ERROR_IO;
-+		usbi_warn(ctx, "attribute %s doesn't end with newline?", attr);
-+	} else {
-+		/* Remove the terminating newline character */
-+		r--;
- 	}
--	buf[r - 1] = '\0';
-+	buf[r] = '\0';
- 
- 	errno = 0;
- 	value = strtol(buf, &endptr, 10);
--- 
-2.29.2
-

0001-linux_usbfs-Fix-parsing-of-descriptors-for-multi-con.patch

@@ -1,61 +0,0 @@
-From f6d2cb561402c3b6d3627c0eb89e009b503d9067 Mon Sep 17 00:00:00 2001
-From: Chris Dickens <christopher.a.dickens@gmail.com>
-Date: Sun, 13 Dec 2020 15:49:19 -0800
-Subject: [PATCH] linux_usbfs: Fix parsing of descriptors for
- multi-configuration devices
-
-Commit e2be556bd2 ("linux_usbfs: Parse config descriptors during device
-initialization") introduced a regression for devices with multiple
-configurations. The logic that verifies the reported length of the
-configuration descriptors failed to count the length of the
-configuration descriptor itself and would truncate the actual length by
-9 bytes, leading to a parsing error for subsequent descriptors.
-
-Closes #825
-
-Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
----
- libusb/os/linux_usbfs.c | 12 ++++++++----
- libusb/version_nano.h   |  2 +-
- 2 files changed, 9 insertions(+), 5 deletions(-)
-
-diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
-index fb2ed53..4d2dc8d 100644
---- a/libusb/os/linux_usbfs.c
-+++ b/libusb/os/linux_usbfs.c
-@@ -641,7 +641,12 @@ static int seek_to_next_config(struct libusb_context *ctx,
- 	uint8_t *buffer, size_t len)
- {
- 	struct usbi_descriptor_header *header;
--	int offset = 0;
-+	int offset;
-+
-+	/* Start seeking past the config descriptor */
-+	offset = LIBUSB_DT_CONFIG_SIZE;
-+	buffer += LIBUSB_DT_CONFIG_SIZE;
-+	len -= LIBUSB_DT_CONFIG_SIZE;
- 
- 	while (len > 0) {
- 		if (len < 2) {
-@@ -718,7 +723,7 @@ static int parse_config_descriptors(struct libusb_device *dev)
- 		}
- 
- 		if (priv->sysfs_dir) {
--			 /*
-+			/*
- 			 * In sysfs wTotalLength is ignored, instead the kernel returns a
- 			 * config descriptor with verified bLength fields, with descriptors
- 			 * with an invalid bLength removed.
-@@ -727,8 +732,7 @@ static int parse_config_descriptors(struct libusb_device *dev)
- 			int offset;
- 
- 			if (num_configs > 1 && idx < num_configs - 1) {
--				offset = seek_to_next_config(ctx, buffer + LIBUSB_DT_CONFIG_SIZE,
--							     remaining - LIBUSB_DT_CONFIG_SIZE);
-+				offset = seek_to_next_config(ctx, buffer, remaining);
- 				if (offset < 0)
- 					return offset;
- 				sysfs_config_len = (uint16_t)offset;
--- 
-2.29.2
-

0002-linux_usbfs-Gracefully-handle-buggy-devices-with-a-c.patch

@@ -1,220 +0,0 @@
-From f38f09da98acc63966b65b72029b1f7f81166bef Mon Sep 17 00:00:00 2001
-From: Chris Dickens <christopher.a.dickens@gmail.com>
-Date: Mon, 8 Feb 2021 11:56:13 -0800
-Subject: [PATCH 2/2] linux_usbfs: Gracefully handle buggy devices with a
- configuration 0
-
-The USB spec states that a configuration value of 0 is reserved and is
-used to indicate the device in not configured (e.g. is in the address
-state). Unfortunately some devices do exist that violate this and use 0
-as the bConfigurationValue of the configuration descriptor.
-
-Improve how the Linux backend handles such non-conformant devices by
-adding special handling around the configuration value 0. Most devices
-will not require this special handling, but for those that do there is
-no way to distinguish between the device being unconfigured and using
-configuration 0.
-
-Closes #850
-
-Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
----
- libusb/os/linux_usbfs.c | 94 ++++++++++++++++++++++++++---------------
- libusb/version_nano.h   |  2 +-
- 2 files changed, 60 insertions(+), 36 deletions(-)
-
-diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
-index ebf8cfe..3a1894c 100644
---- a/libusb/os/linux_usbfs.c
-+++ b/libusb/os/linux_usbfs.c
-@@ -128,7 +128,7 @@ struct linux_device_priv {
- 	void *descriptors;
- 	size_t descriptors_len;
- 	struct config_descriptor *config_descriptors;
--	uint8_t active_config; /* cache val for !sysfs_available  */
-+	int active_config; /* cache val for !sysfs_available  */
- };
- 
- struct linux_device_handle_priv {
-@@ -169,6 +169,21 @@ struct linux_transfer_priv {
- 	int iso_packet_offset;
- };
- 
-+static int dev_has_config0(struct libusb_device *dev)
-+{
-+	struct linux_device_priv *priv = usbi_get_device_priv(dev);
-+	struct config_descriptor *config;
-+	uint8_t idx;
-+
-+	for (idx = 0; idx < dev->device_descriptor.bNumConfigurations; idx++) {
-+		config = &priv->config_descriptors[idx];
-+		if (config->desc->bConfigurationValue == 0)
-+			return 1;
-+	}
-+
-+	return 0;
-+}
-+
- static int get_usbfs_fd(struct libusb_device *dev, mode_t mode, int silent)
- {
- 	struct libusb_context *ctx = DEVICE_CTX(dev);
-@@ -574,22 +589,12 @@ static int sysfs_scan_device(struct libusb_context *ctx, const char *devname)
- }
- 
- /* read the bConfigurationValue for a device */
--static int sysfs_get_active_config(struct libusb_device *dev, uint8_t *config)
-+static int sysfs_get_active_config(struct libusb_device *dev, int *config)
- {
- 	struct linux_device_priv *priv = usbi_get_device_priv(dev);
--	int ret, tmp;
--
--	ret = read_sysfs_attr(DEVICE_CTX(dev), priv->sysfs_dir, "bConfigurationValue",
--			      UINT8_MAX, &tmp);
--	if (ret < 0)
--		return ret;
- 
--	if (tmp == -1)
--		tmp = 0;	/* unconfigured */
--
--	*config = (uint8_t)tmp;
--
--	return 0;
-+	return read_sysfs_attr(DEVICE_CTX(dev), priv->sysfs_dir, "bConfigurationValue",
-+			UINT8_MAX, config);
- }
- 
- int linux_get_device_address(struct libusb_context *ctx, int detached,
-@@ -765,6 +770,9 @@ static int parse_config_descriptors(struct libusb_device *dev)
- 			}
- 		}
- 
-+		if (config_desc->bConfigurationValue == 0)
-+			usbi_warn(ctx, "device has configuration 0");
-+
- 		priv->config_descriptors[idx].desc = config_desc;
- 		priv->config_descriptors[idx].actual_len = config_len;
- 
-@@ -798,7 +806,7 @@ static int op_get_active_config_descriptor(struct libusb_device *dev,
- {
- 	struct linux_device_priv *priv = usbi_get_device_priv(dev);
- 	void *config_desc;
--	uint8_t active_config;
-+	int active_config;
- 	int r;
- 
- 	if (priv->sysfs_dir) {
-@@ -810,12 +818,12 @@ static int op_get_active_config_descriptor(struct libusb_device *dev,
- 		active_config = priv->active_config;
- 	}
- 
--	if (active_config == 0) {
-+	if (active_config == -1) {
- 		usbi_err(DEVICE_CTX(dev), "device unconfigured");
- 		return LIBUSB_ERROR_NOT_FOUND;
- 	}
- 
--	r = op_get_config_descriptor_by_value(dev, active_config, &config_desc);
-+	r = op_get_config_descriptor_by_value(dev, (uint8_t)active_config, &config_desc);
- 	if (r < 0)
- 		return r;
- 
-@@ -863,17 +871,26 @@ static int usbfs_get_active_config(struct libusb_device *dev, int fd)
- 
- 		/* we hit this error path frequently with buggy devices :( */
- 		usbi_warn(DEVICE_CTX(dev), "get configuration failed, errno=%d", errno);
-+
-+		/* assume the current configuration is the first one if we have
-+		 * the configuration descriptors, otherwise treat the device
-+		 * as unconfigured. */
-+		if (priv->config_descriptors)
-+			priv->active_config = (int)priv->config_descriptors[0].desc->bConfigurationValue;
-+		else
-+			priv->active_config = -1;
- 	} else if (active_config == 0) {
--		/* some buggy devices have a configuration 0, but we're
--		 * reaching into the corner of a corner case here, so let's
--		 * not support buggy devices in these circumstances.
--		 * stick to the specs: a configuration value of 0 means
--		 * unconfigured. */
--		usbi_warn(DEVICE_CTX(dev), "active cfg 0? assuming unconfigured device");
-+		if (dev_has_config0(dev)) {
-+			/* some buggy devices have a configuration 0, but we're
-+			 * reaching into the corner of a corner case here. */
-+			priv->active_config = 0;
-+		} else {
-+			priv->active_config = -1;
-+		}
-+	} else {
-+		priv->active_config = (int)active_config;
- 	}
- 
--	priv->active_config = active_config;
--
- 	return LIBUSB_SUCCESS;
- }
- 
-@@ -1004,9 +1021,9 @@ static int initialize_device(struct libusb_device *dev, uint8_t busnum,
- 		usbi_warn(ctx, "Missing rw usbfs access; cannot determine "
- 			       "active configuration descriptor");
- 		if (priv->config_descriptors)
--			priv->active_config = priv->config_descriptors[0].desc->bConfigurationValue;
-+			priv->active_config = (int)priv->config_descriptors[0].desc->bConfigurationValue;
- 		else
--			priv->active_config = 0; /* No config dt */
-+			priv->active_config = -1; /* No config dt */
- 
- 		return LIBUSB_SUCCESS;
- 	}
-@@ -1428,22 +1445,27 @@ static int op_get_configuration(struct libusb_device_handle *handle,
- 	uint8_t *config)
- {
- 	struct linux_device_priv *priv = usbi_get_device_priv(handle->dev);
-+	int active_config;
- 	int r;
- 
- 	if (priv->sysfs_dir) {
--		r = sysfs_get_active_config(handle->dev, config);
-+		r = sysfs_get_active_config(handle->dev, &active_config);
- 	} else {
- 		struct linux_device_handle_priv *hpriv = usbi_get_device_handle_priv(handle);
- 
- 		r = usbfs_get_active_config(handle->dev, hpriv->fd);
- 		if (r == LIBUSB_SUCCESS)
--			*config = priv->active_config;
-+			active_config = priv->active_config;
- 	}
- 	if (r < 0)
- 		return r;
- 
--	if (*config == 0)
--		usbi_err(HANDLE_CTX(handle), "device unconfigured");
-+	if (active_config == -1) {
-+		usbi_warn(HANDLE_CTX(handle), "device unconfigured");
-+		active_config = 0;
-+	}
-+
-+	*config = (uint8_t)active_config;
- 
- 	return 0;
- }
-@@ -1467,11 +1489,13 @@ static int op_set_configuration(struct libusb_device_handle *handle, int config)
- 		return LIBUSB_ERROR_OTHER;
- 	}
- 
--	if (config == -1)
--		config = 0;
-+	/* if necessary, update our cached active config descriptor */
-+	if (!priv->sysfs_dir) {
-+		if (config == 0 && !dev_has_config0(handle->dev))
-+			config = -1;
- 
--	/* update our cached active config descriptor */
--	priv->active_config = (uint8_t)config;
-+		priv->active_config = config;
-+	}
- 
- 	return LIBUSB_SUCCESS;
- }
--- 
-2.29.2
-

1058.patch

@@ -0,0 +1,24 @@
+From 2529a3fc4f987f93e0774af865ac7cb6557bd0c2 Mon Sep 17 00:00:00 2001
+From: Benjamin Berg <bberg@redhat.com>
+Date: Fri, 4 Feb 2022 22:50:28 +0100
+Subject: [PATCH] core: Unset device ctx if it has been destroyed
+
+Devices can outlive their context in some cases (in particular with
+python garbage collection). Guard against this happening by clearing the
+ctx pointer so that it is not pointing to uninitialized memory.
+---
+ libusb/core.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/libusb/core.c b/libusb/core.c
+index 7893ac23..1c1ada14 100644
+--- a/libusb/core.c
++++ b/libusb/core.c
+@@ -2441,6 +2441,7 @@ void API_EXPORTED libusb_exit(libusb_context *ctx)
+ 	for_each_device(_ctx, dev) {
+ 		usbi_warn(_ctx, "device %d.%d still referenced",
+ 			dev->bus_number, dev->device_address);
++		DEVICE_CTX(dev) = NULL;
+ 	}
+ 
+ 	if (!list_empty(&_ctx->open_devs))

1073.patch

@@ -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;

libusb1.spec

@@ -1,6 +1,6 @@
 Summary:        Library for accessing USB devices
 Name:           libusb1
-Version:        1.0.24
+Version:        1.0.25
 Release:        3%{?dist}
 Source0:        https://github.com/libusb/libusb/releases/download/v%{version}/libusb-%{version}.tar.bz2
 License:        LGPLv2+
@@ -12,9 +12,11 @@ BuildRequires:  gcc
 Provides:       libusbx = %{version}-%{release}
 Obsoletes:      libusbx < %{version}-%{release}
 
-Patch001:       0001-linux_usbfs-Accept-sysfs-attributes-not-terminated-w.patch
-Patch002:       0001-linux_usbfs-Fix-parsing-of-descriptors-for-multi-con.patch
-Patch003:       0002-linux_usbfs-Gracefully-handle-buggy-devices-with-a-c.patch
+# Fix a crash after libusb_exit API has been misused
+# https://bugzilla.redhat.com/show_bug.cgi?id=2050638
+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
 
 %description
 This package provides a way for applications to access USB devices.
@@ -123,6 +125,16 @@ LD_LIBRARY_PATH=libusb/.libs $RPM_BUILD_ROOT%{_bindir}/libusb-example-listdevs
 
 
 %changelog
+* Tue Feb 15 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-3
+- Fix a crash if a transfer outlives closing the device
+
+* Thu Feb 10 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-2
+-Fix a crash after libusb_exit API has been misused
+ Resolves: #2050638
+
+* Wed Feb 02 2022 Benjamin Berg <bberg@redhat.com> - 1.0.25-1
+- Update to 1.0.25
+
 * Tue Jan 19 18:47:55 CET 2021 Benjamin Berg <bberg@redhat.com> - 1.0.24-3
 - New libusb1 package replacing libusbx
   Resolves: #1918269

sources

@@ -1 +1 @@
-SHA512 (libusb-1.0.24.tar.bz2) = 5aea36a530aaa15c6dd656d0ed3ce204522c9946d8d39ffbb290dab4a98cda388a2598da4995123d1032324056090bd429e702459626d3e8d7daeebc4e7ff3dc
+SHA512 (libusb-1.0.25.tar.bz2) = f1e6e5577d4bd1ff136927dc66c615014a06ac332ddd797b1d1ad5f7b68e2405e66068dcb210e2f0ae3e31681603ef72efbd88bf7fbe0eb41ce700fdc3f92f9d