All: ref dev->parent_dev

An app can hold a reference to just a leave device, if then an entire
hub goes away, all removed devices will get there final unref, except for
the one ref-ed by the app. If the app then tries to use parent_dev in anyway
after this, we've a use-after-free bug. This fixes this.

Also remove the lets re-enumerate fix for this from libusb_get_port_path,
I'm not sure what the exact idea behind this fix was, but after this patch
it is no longer needed, and this patch also fixes usage of for example
libusb_get_parent().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

5 files changed, 6 insertions, 13 deletions

diff --git a/libusb/core.c b/libusb/core.c
index c8932c3..95c80b9 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -751,13 +751,6 @@ uint8_t API_EXPORTED libusb_get_port_number(libusb_device *dev)
 int API_EXPORTED libusb_get_port_path(libusb_context *ctx, libusb_device *dev, uint8_t* path, uint8_t path_len)
 {
 	int i = path_len;
-	ssize_t r;
-	struct libusb_device **devs = NULL;
-
-	/* The device needs to be open, else the parents may have been destroyed */
-	r = libusb_get_device_list(ctx, &devs);
-	if (r < 0)
-		return (int)r;
 
 	while(dev) {
 		// HCDs can be listed as devices and would have port #0
@@ -766,13 +759,11 @@ int API_EXPORTED libusb_get_port_path(libusb_context *ctx, libusb_device *dev, u
 			break;
 		i--;
 		if (i < 0) {
-			libusb_free_device_list(devs, 1);
 			return LIBUSB_ERROR_OVERFLOW;
 		}
 		path[i] = dev->port_number;
 		dev = dev->parent_dev;
 	}
-	libusb_free_device_list(devs, 1);
 	memmove(path, &path[i], path_len-i);
 	return path_len-i;
 }
@@ -969,6 +960,8 @@ void API_EXPORTED libusb_unref_device(libusb_device *dev)
 	if (refcnt == 0) {
 		usbi_dbg("destroy device %d.%d", dev->bus_number, dev->device_address);
 
+		libusb_unref_device(dev->parent_dev);
+
 		if (usbi_backend->destroy_device)
 			usbi_backend->destroy_device(dev);
 
diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c
index 07d8c5a..74e34cf 100644
--- a/libusb/os/darwin_usb.c
+++ b/libusb/os/darwin_usb.c
@@ -767,7 +767,7 @@ static int process_new_device (struct libusb_context *ctx, usb_device_t **device
     if (ret < 0)
       break;
 
-    dev->parent_dev     = usbi_get_device_by_session_id (ctx, parent_location);
+    dev->parent_dev     = libusb_ref_device (usbi_get_device_by_session_id (ctx, parent_location));
     dev->port_number    = port;
     dev->bus_number     = locationID >> 24;
     dev->device_address = address;
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index cdafb7b..de4d741 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1137,7 +1137,7 @@ retry:
 	list_for_each_entry(it, &ctx->usb_devs, list, struct libusb_device) {
 		struct linux_device_priv *priv = _device_priv(it);
 		if (0 == strcmp (priv->sysfs_dir, parent_sysfs_dir)) {
-			dev->parent_dev = it;
+			dev->parent_dev = libusb_ref_device(it);
 			break;
 		}
 	}
diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 9c0888a..b07d193 100644
--- a/libusb/os/windows_usb.c
+++ b/libusb/os/windows_usb.c
@@ -1110,7 +1110,7 @@ static int init_device(struct libusb_device* dev, struct libusb_device* parent_d
 	dev->port_number = port_number;
 	priv->depth = parent_priv->depth + 1;
 	priv->parent_dev = parent_dev;
-	dev->parent_dev = parent_dev;
+	dev->parent_dev = libusb_ref_device(parent_dev);
 
 	// If the device address is already set, we can stop here
 	if (dev->device_address != 0) {
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index b586028..3940d93 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10690
+#define LIBUSB_NANO 10691