[PATCH 00/13] USB: Make USB devices removable

[PATCH 00/13] USB: Make USB devices removable

[Sascha Hauer]

So far we assumed that USB devices are not disconnected during barebox
lifetime, so we would only ever add to the list of devices. This series
relaxes this assumption. USB storage devices can now be disconnected at
runtime and the disk devices associated with them properly unregister
and filesystems are unmounted. Accesses to files on dead devices are
caught and return an error. Re-plugging USB storage devices works as
well. We still don't detect changes to the USB topology automatically,
but a subsequent "usb" command will bring the barebox topology up to
date with the hardware state.

This has so far only been tested with USB storage devices, USB network
devices and keyboards likely still need some fixes, but as we assumed
USB devices to be non-removable we can still see this series as a new
feature currently limited to USB storage devices.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (13):
      usb: don't report device removal after the device name is gone
      fs: devfs: count an open partition as an open device
      block: propagate errors from blockdevice_unregister()
      fs: add cdev_umount_all()
      block: add blockdevice_unregister_removed()
      usb: storage: tear the disk down properly on disconnect
      usb: hub: cancel pending port scans of a removed device
      usb: reuse the addresses of removed devices
      usb: hub: detect disconnected devices
      usb: don't keep a dangling root device on enumeration failure
      usb: hub: limit the number of ports to USB_MAXCHILDREN
      usb: detect unplugged devices on transfer errors
      usb: storage: stop talking to a device that is gone

 common/block.c                  |  36 +++++++++-
 drivers/usb/core/hub.c          |  91 +++++++++++++++++++++++---
 drivers/usb/core/usb.c          | 141 +++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/usb.h          |   2 +
 drivers/usb/storage/transport.c |   4 ++
 drivers/usb/storage/usb.c       |  31 ++++++++-
 fs/devfs-core.c                 |  14 +++-
 fs/fs.c                         |  31 +++++++++
 include/block.h                 |   1 +
 include/fs.h                    |   1 +
 include/linux/usb/usb.h         |  16 +++++
 11 files changed, 344 insertions(+), 24 deletions(-)
---
base-commit: 39ce98f9cb1cfd4a0601147a4dffa2319d73fc3f
change-id: 20260831-usb-device-lifetime-1c1233592d01

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 01/13] usb: don't report device removal after the device name is gone

[Sascha Hauer]

usb_remove_device() logs the removal after unregister_device() has run.
That is too late: unregister_device() starts with bobject_del(), which
frees the device name, and ends with free_device_res(), which frees
unique_name. dev_printf() then reads both back via dev_name() to build
the message prefix, so both the success and the error message are
use-after-free reads.

Print before unregistering instead. While at it, drop the error branch:
unregister_device() returns 0 unconditionally, so it never ran.

This is not observable today because nothing ever calls
usb_remove_device(), but that is about to change.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/usb.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 3f379197dc..3daa6b1d1d 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -625,10 +625,13 @@ void usb_remove_device(struct usb_device *usbdev)
 	list_del(&usbdev->list);
 	dev_count--;
 
-	if (unregister_device(&usbdev->dev))
-		dev_err(&usbdev->dev, "failed to unregister\n");
-	else
-		dev_info(&usbdev->dev, "removed\n");
+	/*
+	 * unregister_device() frees the device name, so there is nothing
+	 * left to print afterwards.
+	 */
+	dev_info(&usbdev->dev, "removed\n");
+
+	unregister_device(&usbdev->dev);
 
 	usb_free_device(usbdev);
 }

-- 
2.47.3

[PATCH 02/13] fs: devfs: count an open partition as an open device

[Sascha Hauer]

devfs_remove() refuses to remove a cdev that is open, but opening a
partition only increments the open count of the partition itself:
cdev_open() passes the master to the ->open operation, but counts on the
cdev it was given. A disk whose partition is mounted therefore does not
look busy at all.

Removing it then gets half done. The master is unlinked from cdev_list
and its aliases and automount are dropped, then the loop over the
partitions calls cdevfs_del_partition(), which does return -EBUSY for the
mounted partition, but nobody looks at the return value. The partition
stays in cdev_list with its ->master and ->priv pointing at the block
device the caller is about to free, and the next access walks
cdev_get_master() into freed memory.

A device with an open partition is in use, so count the open along the
whole chain of masters instead of only on the cdev that was opened. The
open count then means what it says and devfs_remove() needs no special
case.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 fs/devfs-core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 522d883e1c..c106e65a04 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -249,6 +249,7 @@ static struct cdev *cdev_get_master(struct cdev *cdev)
 int cdev_open(struct cdev *cdev, unsigned long flags)
 {
 	struct cdev *master = cdev_get_master(cdev);
+	struct cdev *c;
 	int ret;
 
 	if (cdev->ops->open) {
@@ -257,7 +258,14 @@ int cdev_open(struct cdev *cdev, unsigned long flags)
 			return ret;
 	}
 
-	cdev->open++;
+	/*
+	 * A device with an open partition is in use itself, so count the
+	 * open along the whole chain up to the device the partition lives
+	 * on. Without that a disk with a mounted partition looks idle and
+	 * could be removed from under the filesystem.
+	 */
+	for (c = cdev; c; c = c->master)
+		c->open++;
 
 	return 0;
 }
@@ -314,6 +322,7 @@ struct cdev *cdev_open_by_path_name(const char *name, unsigned long flags)
 int cdev_close(struct cdev *cdev)
 {
 	struct cdev *master = cdev_get_master(cdev);
+	struct cdev *c;
 
 	if (cdev->ops->close) {
 		int ret = cdev->ops->close(master);
@@ -321,7 +330,8 @@ int cdev_close(struct cdev *cdev)
 			return ret;
 	}
 
-	cdev->open--;
+	for (c = cdev; c; c = c->master)
+		c->open--;
 
 	return 0;
 }

-- 
2.47.3

[PATCH 03/13] block: propagate errors from blockdevice_unregister()

[Sascha Hauer]

blockdevice_unregister() discards the return value of devfs_remove() and
always reports success. A caller therefore has no way to find out that
the block device is still in use and goes on to free the memory the
cdev, and possibly its partitions, still refer to.

Return the error and do so before anything has been torn down, so the
caller can simply try again later. The buffer chunks and the block
device list entry stay untouched in that case.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 common/block.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/block.c b/common/block.c
index 5f509bfb9d..141604eb87 100644
--- a/common/block.c
+++ b/common/block.c
@@ -519,6 +519,15 @@ int blockdevice_register(struct block_device *blk)
 int blockdevice_unregister(struct block_device *blk)
 {
 	struct chunk *chunk, *tmp;
+	int ret;
+
+	/*
+	 * Do this first: once the cdev is gone there is no way for the
+	 * caller to retry, so give up while everything is still intact.
+	 */
+	ret = devfs_remove(&blk->cdev);
+	if (ret)
+		return ret;
 
 	writebuffer_flush(blk);
 
@@ -532,7 +541,6 @@ int blockdevice_unregister(struct block_device *blk)
 		free(chunk);
 	}
 
-	devfs_remove(&blk->cdev);
 	list_del(&blk->list);
 
 	return 0;

-- 
2.47.3

[PATCH 04/13] fs: add cdev_umount_all()

[Sascha Hauer]

umount_by_cdev() unmounts the filesystems mounted from one cdev. For a
partitioned device that is not enough: the filesystems are mounted from
the partitions, not from the disk itself, so unmounting the disk leaves
them behind.

Add a variant that walks the partitions as well. This is what a driver
needs when the medium it handed out is gone and the mounts on it have
become useless.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 fs/fs.c      | 31 +++++++++++++++++++++++++++++++
 include/fs.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index ce41f23f88..f0d3d9fa0a 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1033,6 +1033,37 @@ int umount_by_cdev(struct cdev *cdev)
 }
 EXPORT_SYMBOL(umount_by_cdev);
 
+/**
+ * cdev_umount_all - unmount everything mounted from a cdev
+ * @cdev: cdev to unmount
+ *
+ * Unmounts the filesystems mounted from @cdev and from its partitions.
+ * Useful for a medium that is gone, where the mounts cannot be used
+ * anymore anyway.
+ *
+ * Return: 0 on success or if nothing was mounted, the first error
+ *         otherwise
+ */
+int cdev_umount_all(struct cdev *cdev)
+{
+	struct cdev *partcdev;
+	int first_error = 0;
+	int ret;
+
+	for_each_cdev_partition(partcdev, cdev) {
+		ret = cdev_umount_all(partcdev);
+		if (ret && !first_error)
+			first_error = ret;
+	}
+
+	ret = umount_by_cdev(cdev);
+	if (ret && !first_error)
+		first_error = ret;
+
+	return first_error;
+}
+EXPORT_SYMBOL(cdev_umount_all);
+
 struct readdir_entry {
 	struct dirent d;
 	struct list_head list;
diff --git a/include/fs.h b/include/fs.h
index 9c3c8d9512..0cc58d73c2 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -112,6 +112,7 @@ static inline bool is_tftp_fs(const char *path)
 
 int flush(int fd);
 int umount_by_cdev(struct cdev *cdev);
+int cdev_umount_all(struct cdev *cdev);
 
 /* not-so-standard functions */
 #define ERASE_SIZE_ALL	((loff_t) - 1)

-- 
2.47.3

[PATCH 05/13] block: add blockdevice_unregister_removed()

[Sascha Hauer]

Removing a block device whose medium is gone is not the same as removing
one that is merely no longer wanted. The medium is not coming back, so
the filesystems mounted from it have become useless, and keeping them
around only means the device cannot be removed at all: the open cdevs
make it busy.

Add a variant of blockdevice_unregister() for that case which unmounts
the filesystems on the device and on its partitions first. Removable
media are the ones that need it - a USB stick that has been unplugged, an
SD card that has been ejected - so this is not something the block layer
can decide on its own, the driver knows.

The error case is still reported so a caller that cannot get rid of the
device knows it must not free it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 common/block.c  | 26 ++++++++++++++++++++++++++
 include/block.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/common/block.c b/common/block.c
index 141604eb87..5441c7ca18 100644
--- a/common/block.c
+++ b/common/block.c
@@ -7,6 +7,7 @@
 #include <common.h>
 #include <block.h>
 #include <disks.h>
+#include <fs.h>
 #include <malloc.h>
 #include <linux/err.h>
 #include <linux/list.h>
@@ -546,6 +547,31 @@ int blockdevice_unregister(struct block_device *blk)
 	return 0;
 }
 
+/**
+ * blockdevice_unregister_removed - remove a block device whose medium is gone
+ * @blk: the block device
+ *
+ * For media that can disappear while barebox is running: a USB stick that
+ * has been unplugged, an SD card that has been ejected. The filesystems
+ * mounted from the device or from one of its partitions cannot be used
+ * anymore, and would only keep the device from going away, so drop them.
+ *
+ * Use blockdevice_unregister() instead when the device is still there and
+ * the removal is not forced upon us. That one leaves the mounts alone and
+ * fails when the device is in use.
+ *
+ * Return: 0 when the device has been removed, a negative error code when
+ *         something still holds it open. In the latter case the caller
+ *         must not free the block device: the cdevs are still registered
+ *         and refer to it.
+ */
+int blockdevice_unregister_removed(struct block_device *blk)
+{
+	cdev_umount_all(&blk->cdev);
+
+	return blockdevice_unregister(blk);
+}
+
 int block_read(struct block_device *blk, void *buf, sector_t block, blkcnt_t num_blocks)
 {
 	int ret;
diff --git a/include/block.h b/include/block.h
index 9a0102db15..065ae81358 100644
--- a/include/block.h
+++ b/include/block.h
@@ -97,6 +97,7 @@ extern struct list_head block_device_list;
 
 int blockdevice_register(struct block_device *blk);
 int blockdevice_unregister(struct block_device *blk);
+int blockdevice_unregister_removed(struct block_device *blk);
 
 int block_read(struct block_device *blk, void *buf, sector_t block, blkcnt_t num_blocks);
 int block_write(struct block_device *blk, void *buf, sector_t block, blkcnt_t num_blocks);

-- 
2.47.3

[PATCH 06/13] usb: storage: tear the disk down properly on disconnect

[Sascha Hauer]

usb_stor_disconnect() unregistered the block device and freed it right
away, no matter whether the removal actually worked. With a filesystem
mounted from the stick it does not: the partition cdev is still open, so
the disk cannot go away and the cdevs would be left pointing into freed
memory.

Use blockdevice_unregister_removed(), which drops the filesystems that
were mounted from the stick before removing it. They are stale anyway,
the medium they live on is gone.

Should something else still hold the disk open we now keep it around
instead of freeing it. That leaks the disk and the us_data it refers to,
but a leak is preferable to handing out a cdev that points at freed
memory. barebox has no refcounting on devices, so there is no way to do
better than that here.

While at it, free the cdev name, which nobody did so far.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/storage/usb.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 39c4695b0a..7c19207a8d 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -608,16 +608,34 @@ static void usb_stor_disconnect(struct usb_device *usbdev)
 {
 	struct us_data *us = (struct us_data *)usbdev->drv_data;
 	struct us_blk_dev *bdev, *bdev_tmp;
+	bool busy = false;
+	int ret;
 
 	list_for_each_entry_safe(bdev, bdev_tmp, &us->blk_dev_list, list) {
+		ret = blockdevice_unregister_removed(&bdev->blk);
+		if (ret) {
+			/*
+			 * Something still holds the disk open. Leaking it is
+			 * not nice, but freeing it would leave the cdev that
+			 * is still in use pointing at freed memory.
+			 */
+			dev_err(&usbdev->dev, "%s is still in use, leaking it: %pe\n",
+				bdev->blk.cdev.name, ERR_PTR(ret));
+			busy = true;
+			continue;
+		}
+
 		list_del(&bdev->list);
-		blockdevice_unregister(&bdev->blk);
+		free(bdev->blk.cdev.name);
 		free(bdev);
 	}
 
 	/* release device's private data */
-	usbdev->drv_data = 0;
-	free(us);
+	usbdev->drv_data = NULL;
+
+	/* a leaked disk still refers to us, so that has to stay as well */
+	if (!busy)
+		free(us);
 }
 
 #define USUAL_DEV(use_proto, use_trans, drv_info) \

-- 
2.47.3

[PATCH 07/13] usb: hub: cancel pending port scans of a removed device

[Sascha Hauer]

usb_hub_configure_ports() queues one scan entry per port and leaves the
draining to usb_device_list_scan(). That function is not reentrant: when
a hub is found during a scan, its ports are queued and processed by the
loop that is already running rather than by a nested one.

An entry therefore outlives the call that created it, and it holds
pointers to both the usb_device and its usb_hub_device. Removing a hub
frees the former in usb_free_device() and the latter in
usb_hub_disconnect(), so any entry left in the list would be dereferenced
after the free - hub->query_delay is read on every round.

Drop the entries belonging to a device before it goes away.
usb_remove_device() recurses into the children first, so every device in
a removed subtree cleans up its own entries.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/hub.c | 24 ++++++++++++++++++++++++
 drivers/usb/core/usb.c |  3 +++
 drivers/usb/core/usb.h |  1 +
 3 files changed, 28 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3820b4cc90..b56c658f91 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -260,6 +260,30 @@ static int hub_port_reset(struct usb_device *hub, int port,
 }
 
 
+/**
+ * usb_hub_cancel_scans - drop pending port scans of a hub that goes away
+ * @dev: the USB device that is about to be removed
+ *
+ * The scan list is filled by usb_hub_configure_ports() and drained by
+ * usb_device_list_scan(). As the latter is not reentrant, a hub found
+ * during a scan queues its ports and leaves them for the loop that is
+ * already running. If that hub is removed before its entries have been
+ * processed, they would be left pointing at the freed usb_device and its
+ * freed usb_hub_device.
+ */
+void usb_hub_cancel_scans(struct usb_device *dev)
+{
+	struct usb_device_scan *usb_scan, *tmp;
+
+	list_for_each_entry_safe(usb_scan, tmp, &usb_scan_list, list) {
+		if (usb_scan->dev != dev)
+			continue;
+
+		list_del(&usb_scan->list);
+		free(usb_scan);
+	}
+}
+
 static void usb_hub_port_connect_change(struct usb_device *dev, int port,
 					uint16_t portstatus, uint16_t portchange)
 {
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 3daa6b1d1d..bc80e66fcb 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -620,6 +620,9 @@ void usb_remove_device(struct usb_device *usbdev)
 
 	for (i = 0; i < usbdev->maxchild; i++)
 		usb_remove_device(usbdev->children[i]);
+
+	usb_hub_cancel_scans(usbdev);
+
 	if (usbdev->parent && usbdev->portnr)
 		usbdev->parent->children[usbdev->portnr - 1] = NULL;
 	list_del(&usbdev->list);
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 0d4f80c21d..b3c224d88f 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -6,5 +6,6 @@ struct usb_device *usb_alloc_new_device(void);
 void usb_free_device(struct usb_device *dev);
 int usb_new_device(struct usb_device *dev);
 void usb_remove_device(struct usb_device *dev);
+void usb_hub_cancel_scans(struct usb_device *dev);
 
 #endif /* __CORE_USB_H */

-- 
2.47.3

[PATCH 08/13] usb: reuse the addresses of removed devices

[Sascha Hauer]

dev_index only ever counted up, so every device that showed up got an
address one higher than the previous one. Without device removal that
was fine - there is a limited number of devices on a bus - but once
devices come and go, the counter keeps climbing. USB addresses are 7 bit
wide, so after 127 devices usb_set_address() starts handing out values
that cannot be represented and enumeration breaks until the next reset.

Keep a bitmap of the addresses in use instead and hand back the address
when the device is freed. As a side effect an exhausted address space is
now reported instead of silently producing broken addresses.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/usb.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index bc80e66fcb..ad0d0965b1 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -38,6 +38,7 @@
 #include <init.h>
 #include <dma.h>
 
+#include <linux/bitmap.h>
 #include <linux/usb/usb.h>
 #include <linux/usb/ch9.h>
 
@@ -46,7 +47,33 @@
 #define USB_BUFSIZ	512
 
 static int dev_count;
-static int dev_index;
+
+/*
+ * USB addresses are 7 bit wide and 0 is reserved for the default address,
+ * so the usable range is 1..127. Track them in a bitmap rather than just
+ * counting up, otherwise a board that sees enough plug/unplug cycles
+ * eventually hands out addresses a device cannot have.
+ */
+static DECLARE_BITMAP(usb_addresses, 128);
+
+static int usb_alloc_address(void)
+{
+	int addr;
+
+	addr = find_next_zero_bit(usb_addresses, 128, 1);
+	if (addr >= 128)
+		return -EADDRNOTAVAIL;
+
+	set_bit(addr, usb_addresses);
+
+	return addr;
+}
+
+static void usb_free_address(int addr)
+{
+	if (addr > 0)
+		clear_bit(addr, usb_addresses);
+}
 
 LIST_HEAD(usb_host_list);
 LIST_HEAD(usb_device_list);
@@ -487,7 +514,12 @@ int usb_new_device(struct usb_device *dev)
 
 	usb_setup_descriptor(dev, !host->no_desc_before_addr);
 
-	dev->devnum = ++dev_index;
+	err = usb_alloc_address();
+	if (err < 0) {
+		dev_err(&dev->dev, "out of USB addresses\n");
+		goto err_out;
+	}
+	dev->devnum = err;
 
 	err = usb_set_address(dev); /* set address */
 
@@ -605,6 +637,7 @@ int usb_new_device(struct usb_device *dev)
 
 void usb_free_device(struct usb_device *usbdev)
 {
+	usb_free_address(usbdev->devnum);
 	dma_free(usbdev->descriptor);
 	dma_free(usbdev->setup_packet);
 	free_device_res(&usbdev->dev);

-- 
2.47.3

[PATCH 09/13] usb: hub: detect disconnected devices

[Sascha Hauer]

barebox never noticed that a USB device was unplugged. Running "usb"
again after removing a stick kept reporting it, /dev/disk0 and its
partitions stayed around and reads went to a device that was not there
anymore.

The machinery to remove a device was all there, it was just never
reached. usb_scan_port() bailed out before usb_hub_port_connect_change()
whenever the port reported no connection:

	if (!(portchange & USB_PORT_STAT_C_CONNECTION) ||
	    !(portstatus & USB_PORT_STAT_CONNECTION))

A port whose device has been unplugged has the change bit set - the hub
latches it - and the connection bit clear, which is exactly what the
second half of the condition filters out. The disconnect branch in
usb_hub_port_connect_change() could therefore never run: getting there
required the connection bit to be set, so its test for the very same bit
being clear was always false.

Let a connection change through when we have a device on that port, no
matter what the connection bit says, and decide in
usb_hub_port_connect_change() what it means: the device we knew about is
gone in either case, so remove it and enumerate a new one only if the
port reports a connection again. That also covers a device being swapped
for another one between two scans, which used to end in

	ERROR: register_device: already registered usb1-0

with the new device unusable and the old one orphaned from the port
array, so it could never be removed at all.

The previous attempt to catch that case tested for USB_PORT_STAT_ENABLE
being clear. That cannot work on xHCI: a SuperSpeed root port is enabled
by the controller during link training, so it is already enabled the
first time we look at it. Drop the test.

Ports without a device keep the old behaviour and stay in the scan list
until the connect timeout expires, so slow devices still get their
second before we give up on them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/hub.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b56c658f91..18de8badf5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -292,20 +292,29 @@ static void usb_hub_port_connect_change(struct usb_device *dev, int port,
 	/* Clear the connection change status */
 	usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_CONNECTION);
 
-	/* Disconnect any existing devices under this port */
-	if (dev->children[port] && !(portstatus & USB_PORT_STAT_CONNECTION)) {
+	/*
+	 * The connection changed, so whatever we knew about this port is
+	 * gone. That covers a plain unplug as well as a device that has been
+	 * replaced while we were not looking: the port reports a connection
+	 * again in that case, but not the one we have a device for.
+	 *
+	 * Note we must not test for USB_PORT_STAT_ENABLE here to tell the
+	 * two apart. An xHCI root hub enables a SuperSpeed port on its own
+	 * during link training, so the port is already enabled the first
+	 * time we see it.
+	 */
+	if (dev->children[port]) {
 		dev_dbg(&dev->dev, "port%d: disconnect detected\n", port + 1);
 		usb_remove_device(dev->children[port]);
+		dev->children[port] = NULL;
 
 		if (!dev->parent && dev->host->usbphy)
 			usb_phy_notify_disconnect(dev->host->usbphy, dev->speed);
-
-		return;
 	}
 
-	/* Remove disabled but connected devices */
-	if (dev->children[port] && !(portstatus & USB_PORT_STAT_ENABLE))
-		usb_remove_device(dev->children[port]);
+	/* Nothing connected anymore, we are done */
+	if (!(portstatus & USB_PORT_STAT_CONNECTION))
+		return;
 
 	/* Allocate a new device struct for the port */
 	usb = usb_alloc_new_device();
@@ -380,8 +389,15 @@ static void usb_scan_port(struct usb_device_scan *usb_scan)
 	dev_dbg(&dev->dev, "port%d: Status 0x%04x Change 0x%04x\n",
 			port + 1, portstatus, portchange);
 
+	/*
+	 * A connection change on a port we have a device for has to be
+	 * handled even when the port reports no connection: that is how a
+	 * disconnect looks. An empty port on the other hand keeps its change
+	 * bit set from the power-on, so leave it in the scan list until the
+	 * connect timeout expires to give slow devices time to show up.
+	 */
 	if (!(portchange & USB_PORT_STAT_C_CONNECTION) ||
-	    !(portstatus & USB_PORT_STAT_CONNECTION)) {
+	    (!(portstatus & USB_PORT_STAT_CONNECTION) && !dev->children[port])) {
 		if (get_time_ns() >= hub->connect_timeout) {
 			dev_dbg(&dev->dev, "port%d: timeout\n", port + 1);
 			/* Remove this device from scanning list */
@@ -402,8 +418,7 @@ static void usb_scan_port(struct usb_device_scan *usb_scan)
 		usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_BH_PORT_RESET);
 	}
 
-	/* A new USB device is ready at this point */
-	dev_dbg(&dev->dev, "port%d: USB dev found\n", port + 1);
+	dev_dbg(&dev->dev, "port%d: connection change\n", port + 1);
 
 	usb_hub_port_connect_change(dev, port, portstatus, portchange);
 

-- 
2.47.3

[PATCH 10/13] usb: don't keep a dangling root device on enumeration failure

[Sascha Hauer]

When usb_new_device() fails for the root hub, usb_host_detect() frees it
but leaves host->root_dev pointing at the freed memory. The next "usb"
run finds the pointer non-NULL, skips the re-initialisation and hands the
freed device to device_detect().

Clear the pointer so the next scan starts over.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index ad0d0965b1..55c57e6334 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -704,6 +704,7 @@ int usb_host_detect(struct usb_host *host)
 		ret = usb_new_device(host->root_dev);
 		if (ret) {
 			usb_free_device(host->root_dev);
+			host->root_dev = NULL;
 			return ret;
 		}
 	}

-- 
2.47.3

[PATCH 11/13] usb: hub: limit the number of ports to USB_MAXCHILDREN

[Sascha Hauer]

usb_hub_configure() takes the port count straight from the hub
descriptor:

	dev->maxchild = descriptor->bNbrPorts;

children[] in struct usb_device and overcurrent_count[] in struct
usb_hub_device are both sized USB_MAXCHILDREN though, which is 8. A hub that reports
more ports than that - bNbrPorts is a byte, so up to 255 - makes
usb_hub_configure_ports() queue a scan for every one of them, and
usb_scan_port() and usb_hub_port_connect_change() then index both arrays
out of bounds.

The port count is device supplied, so cap it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/hub.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 18de8badf5..f897740cbf 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -550,7 +550,17 @@ static int usb_hub_configure(struct usb_device *dev)
 	for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++)
 		hub->desc.u.hs.PortPwrCtrlMask[i] = descriptor->u.hs.PortPwrCtrlMask[i];
 
+	/*
+	 * bNbrPorts comes from the device, while children[] and
+	 * overcurrent_count[] are sized after USB_MAXCHILDREN. Don't let a
+	 * hub that reports more ports than that write past their ends.
+	 */
 	dev->maxchild = descriptor->bNbrPorts;
+	if (dev->maxchild > USB_MAXCHILDREN) {
+		dev_warn(&dev->dev, "hub reports %d ports, only using %d\n",
+			 dev->maxchild, USB_MAXCHILDREN);
+		dev->maxchild = USB_MAXCHILDREN;
+	}
 	dev_dbg(&dev->dev, "%d ports detected\n", dev->maxchild);
 
 	switch (hub->desc.wHubCharacteristics & HUB_CHAR_LPSM) {

-- 
2.47.3

[PATCH 12/13] usb: detect unplugged devices on transfer errors

[Sascha Hauer]

Removing a device is only done when somebody runs "usb". Until then I/O
to a device that has been unplugged in the meantime keeps failing, and
neither the USB core nor the drivers realise why. Reading from a
filesystem that was mounted off a stick that is gone results in an
endless stream of

	usb-storage usb1-0-1: Resetting EP 1...
	ERROR: xHCI xHCI0: halted endpoint, not queueing URB.

as the storage driver keeps resetting endpoints of a device that is not
there and retries the command.

When a transfer fails, ask the hub the device is attached to whether the
port still reports a connection. That is cheap - the hub is still there -
and it answers the question the transfer error cannot: is this a flaky
device or no device at all. Remember the answer, so the bus is asked once
and every further transfer fails with -ENODEV right away.

Only a hub that positively reports an empty port counts. If the hub
cannot be asked we have learned nothing, and marking a device gone
because some unrelated transfer failed would be worse than leaving it
alone. A device behind a hub that turns out to be gone is marked as well,
along with everything below it.

Removal itself is left to the next scan. Tearing the device down from
inside a transfer would pull the block device and the cdevs out from
under the filesystem that is asking for the data.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/core/hub.c  | 22 ++++++++++++
 drivers/usb/core/usb.c  | 89 +++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/core/usb.h  |  1 +
 include/linux/usb/usb.h | 16 +++++++++
 4 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f897740cbf..6520cd43d5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -260,6 +260,28 @@ static int hub_port_reset(struct usb_device *hub, int port,
 }
 
 
+/**
+ * usb_hub_port_connected - ask a hub whether a port still has a device
+ * @hub: the hub to ask
+ * @port: 1-based port number
+ *
+ * Return: 1 if the port reports a device, 0 if it doesn't and a negative
+ * error code when the hub could not be asked. Do not treat the latter as
+ * "no device": a hub that doesn't answer tells us nothing about what is
+ * plugged into it.
+ */
+int usb_hub_port_connected(struct usb_device *hub, int port)
+{
+	struct usb_port_status portsts;
+	int ret;
+
+	ret = usb_get_port_status(hub, port, &portsts);
+	if (ret < 0)
+		return ret;
+
+	return !!(le16_to_cpu(portsts.wPortStatus) & USB_PORT_STAT_CONNECTION);
+}
+
 /**
  * usb_hub_cancel_scans - drop pending port scans of a hub that goes away
  * @dev: the USB device that is about to be removed
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 55c57e6334..71b72c3efb 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -751,6 +751,73 @@ int usb_rescan(void)
  *
  */
 
+/**
+ * usb_mark_disconnected - remember that a device and its children are gone
+ * @dev: the device that has been unplugged
+ */
+static void usb_mark_disconnected(struct usb_device *dev)
+{
+	int i;
+
+	if (usb_device_disconnected(dev))
+		return;
+
+	dev->disconnected = true;
+	dev_info(&dev->dev, "disconnected\n");
+
+	/* everything behind an unplugged hub is gone as well */
+	for (i = 0; i < dev->maxchild; i++)
+		if (dev->children[i])
+			usb_mark_disconnected(dev->children[i]);
+}
+
+/**
+ * usb_device_check_gone - find out whether a failed transfer means "unplugged"
+ * @dev: the device a transfer just failed on
+ *
+ * A transfer can fail for all kinds of reasons, so ask the hub the device
+ * is attached to whether the port still sees a device. Without this the
+ * only thing telling a stale device from a flaky one is a scan, and until
+ * somebody runs one we keep resetting endpoints of a device that is not
+ * there anymore.
+ *
+ * The result is remembered, so this asks the bus once and all further
+ * transfers fail right away.
+ */
+static bool usb_device_check_gone(struct usb_device *dev)
+{
+	struct usb_device *parent = dev->parent;
+
+	if (usb_device_disconnected(dev))
+		return true;
+
+	/*
+	 * A root hub has no hub to ask. It goes away with its controller,
+	 * which is not something we can detect here.
+	 */
+	if (!parent)
+		return false;
+
+	/* If the hub itself is gone, so is everything below it */
+	if (usb_device_check_gone(parent)) {
+		usb_mark_disconnected(dev);
+		return true;
+	}
+
+	/*
+	 * Only believe a hub that positively says the port is empty. If it
+	 * doesn't answer we have learned nothing, and declaring the device
+	 * gone on a transfer that failed for some other reason would be
+	 * worse than leaving it alone.
+	 */
+	if (usb_hub_port_connected(parent, dev->portnr) != 0)
+		return false;
+
+	usb_mark_disconnected(dev);
+
+	return true;
+}
+
 /*
  * submits an Interrupt Message
  */
@@ -760,6 +827,9 @@ int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 	struct usb_host *host = dev->host;
 	int ret;
 
+	if (usb_device_disconnected(dev))
+		return -ENODEV;
+
 	ret = usb_host_acquire(host);
 	if (ret)
 		return ret;
@@ -768,6 +838,9 @@ int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe,
 
 	usb_host_release(host);
 
+	if (ret && usb_device_check_gone(dev))
+		return -ENODEV;
+
 	return ret;
 }
 
@@ -788,6 +861,9 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 	int ret;
 	struct devrequest *setup_packet = dev->setup_packet;
 
+	if (usb_device_disconnected(dev))
+		return -ENODEV;
+
 	ret = usb_host_acquire(host);
 	if (ret)
 		return ret;
@@ -808,8 +884,11 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 
 	usb_host_release(host);
 
-	if (ret)
+	if (ret) {
+		if (usb_device_check_gone(dev))
+			return -ENODEV;
 		return ret;
+	}
 
 	return dev->act_len;
 }
@@ -828,6 +907,9 @@ int usb_bulk_msg(struct usb_device *dev, unsigned int pipe,
 	if (len < 0)
 		return -1;
 
+	if (usb_device_disconnected(dev))
+		return -ENODEV;
+
 	ret = usb_host_acquire(host);
 	if (ret)
 		return ret;
@@ -837,8 +919,11 @@ int usb_bulk_msg(struct usb_device *dev, unsigned int pipe,
 
 	usb_host_release(host);
 
-	if (ret)
+	if (ret) {
+		if (usb_device_check_gone(dev))
+			return -ENODEV;
 		return ret;
+	}
 
 	*actual_length = dev->act_len;
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index b3c224d88f..2529bcfec1 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -7,5 +7,6 @@ void usb_free_device(struct usb_device *dev);
 int usb_new_device(struct usb_device *dev);
 void usb_remove_device(struct usb_device *dev);
 void usb_hub_cancel_scans(struct usb_device *dev);
+int usb_hub_port_connected(struct usb_device *hub, int port);
 
 #endif /* __CORE_USB_H */
diff --git a/include/linux/usb/usb.h b/include/linux/usb/usb.h
index c25f3d73c3..784786dad5 100644
--- a/include/linux/usb/usb.h
+++ b/include/linux/usb/usb.h
@@ -126,8 +126,24 @@ struct usb_device {
 
 	/* slot_id - for xHCI enabled devices */
 	unsigned int slot_id;
+
+	/* device is physically gone, don't talk to it anymore */
+	bool disconnected;
 };
 
+/**
+ * usb_device_disconnected - has the device been unplugged?
+ * @dev: the USB device
+ *
+ * Set once a transfer to the device failed and the hub it is attached to
+ * confirmed that the port has no connection anymore. Transfers to such a
+ * device fail with -ENODEV without touching the bus.
+ */
+static inline bool usb_device_disconnected(struct usb_device *dev)
+{
+	return dev->disconnected;
+}
+
 struct usb_device_id;
 
 struct usb_driver {

-- 
2.47.3

[PATCH 13/13] usb: storage: stop talking to a device that is gone

[Sascha Hauer]

usb_stor_transport() retries a failed command ten times, and every
attempt runs usb_stor_Bulk_reset(), which issues three more control
transfers to the device. For a device that has been unplugged all of
these run into their timeouts, so a single read takes minutes and fills
the console with endpoint reset messages.

Now that the USB core notices that a device has been unplugged, check
for it: skip the reset, which cannot work without a device, and give up
instead of retrying. A read of an unplugged stick fails after the first
timeout and every further one fails immediately.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/usb/storage/transport.c | 4 ++++
 drivers/usb/storage/usb.c       | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index be3b18dc66..bda8f93c7a 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -220,6 +220,10 @@ int usb_stor_Bulk_reset(struct us_data *us)
 
 	dev_dbg(dev, "%s called\n", __func__);
 
+	/* Nothing to reset if the device isn't there anymore */
+	if (usb_device_disconnected(us->pusb_dev))
+		return -ENODEV;
+
 	/* issue the command */
 	result = usb_control_msg(us->pusb_dev,
 	                         usb_sndctrlpipe(us->pusb_dev, 0),
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 7c19207a8d..55facf8a60 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -86,6 +86,13 @@ static int usb_stor_transport(struct us_blk_dev *usb_blkdev,
 		if (ret == USB_STOR_TRANSPORT_GOOD)
 			return 0;
 
+		/* Retrying is pointless once the device has been unplugged */
+		if (usb_device_disconnected(us->pusb_dev)) {
+			dev_dbg(dev, "%s: device is gone\n",
+				usb_stor_opcode_name(cmd[0]));
+			return -ENODEV;
+		}
+
 		if (request_sense_delay_ms == USB_STOR_NO_REQUEST_SENSE)
 			continue;
 

-- 
2.47.3