[PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[James Clark]

On a BeagleY-AI with BeagleBoard's device tree, kernels since commit
f918e3804498 ("arm64: enable HAVE_SHARED_GPIOS for all ARM64 platforms")
no longer boot to a usable system. The log repeats "hub 1-1:1.0: USB hub
found" for the on-board TUSB8041 hub every few milliseconds, with no USB
disconnect in between. The flood of uevents stalls udev and the boot ends
in emergency mode. Reverting that commit makes the board boot normally
again.

The underlying problem is in onboard_usb_dev. Both peer hub nodes in this
device tree carry the same reset-gpios, so the GPIO is now a shared one,
and with CONFIG_GPIO_SHARED_PROXY=m (the default) the hub's platform
probe is legitimately deferred until udev has loaded that module. The
bootloader leaves the hub running, so the USB core enumerates it long
before, and the USB device probe finds the platform device without
driver data.

At that point, onboard_dev_usbdev_probe() returns -EPROBE_DEFER. This is
what causes the problem. The driver has generic_subclass set, so
usb_probe_device() has already run the generic probe, which configured
the device and bound the interface drivers. Each binding is a
deferred-probe trigger, and the driver core re-triggers when a trigger
happened during a probe that then deferred. So the device is re-probed
straight away, reconfigured, its interfaces bound again, and so on for
as long as the platform device remains unbound. This is not specific to
shared GPIOs: it happens whenever the platform probe is deferred, and
briefly whenever onboard_usb_dev is a module.

Bind to the USB device anyway, with NULL drvdata. When the platform
probe finishes, it already schedules attach_usb_driver_work to
re-attach the USB driver; extend it to also walk the USB devices and
attach the ones that are bound to this driver but have no drvdata. This
ensures the USB device always has a driver.

It doesn't work to return -ENXIO: the driver core takes this as the
driver declining the device, but the device is then left without a
driver, and a device without a driver can't be resumed once it has
autosuspended, which a hub with empty ports does right away. Nor does
it work to return -ENODEV: the USB core then binds the generic driver,
and driver_attach() doesn't rebind a device that already has one.

With this change the BeagleY-AI boots normally with that commit in
place.

Fixes: 8bc063641ceb ("usb: misc: Add onboard_usb_hub driver")
Cc: stable@vger.kernel.org
Signed-off-by: James Clark <jjc@jclark.com>
---
I found this by booting commit 85b4bc79dde7 (net/main during the 7.4
merge window) on a BeagleY-AI with BeagleBoard's device tree. A coding
agent identified the problem, explained it to me and proposed an initial
fix, which was revised multiple times on the basis of local Sashiko
reviews. I verified that the patch fixed the problem on top of commit
d89e1d186ec9.

On this board the platform probe asserts the hub's reset line, so the
hubs re-enumerate and are probed with the platform device already
bound. The path where attach_usb_driver_work adopts a USB device that
was bound with NULL drvdata has therefore not been exercised. It needs a
board whose platform probe does not reset the hub.

#regzbot introduced: f918e3804498

 drivers/usb/misc/onboard_usb_dev.c | 76 ++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index c60406240416..aac4e29c8233 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -43,6 +43,7 @@
 #define USB5744_CREG_BYPASS_UDC_SUSPEND		BIT(3)
 
 static void onboard_dev_attach_usb_driver(struct work_struct *work);
+static struct onboard_dev *_find_onboard_dev(struct device *dev);
 
 static struct usb_device_driver onboard_dev_usbdev_driver;
 static DECLARE_WORK(attach_usb_driver_work, onboard_dev_attach_usb_driver);
@@ -301,6 +302,47 @@ static const struct attribute_group onboard_dev_group = {
 __ATTRIBUTE_GROUPS(onboard_dev);
 
 
+/*
+ * Attach a USB device that was bound by onboard_dev_usbdev_probe() while its
+ * platform device was still unbound. The device lock serialises this with
+ * the probe and with an unbind. This walks the USB bus, not the driver's
+ * device list: unbinding a device holds its lock while it waits for all
+ * references to its node on the driver's list to be dropped, so an iterator
+ * over that list that takes the device lock would deadlock with it.
+ */
+static int onboard_dev_attach_usbdev(struct usb_device *udev, void *data)
+{
+	struct device *dev = &udev->dev;
+	struct onboard_dev *onboard_dev;
+	int err;
+
+	/* Unlocked pre-check, so as not to wait for every USB device's lock. */
+	if (READ_ONCE(dev->driver) != &onboard_dev_usbdev_driver.driver)
+		return 0;
+
+	device_lock(dev);
+
+	if (dev->driver != &onboard_dev_usbdev_driver.driver ||
+	    dev_get_drvdata(dev))
+		goto out;
+
+	onboard_dev = _find_onboard_dev(dev);
+	if (IS_ERR_OR_NULL(onboard_dev))
+		goto out;
+
+	err = onboard_dev_add_usbdev(onboard_dev, udev);
+	if (err)
+		dev_err(dev, "failed to attach to %s: %pe\n",
+			dev_name(onboard_dev->dev), ERR_PTR(err));
+	else
+		dev_set_drvdata(dev, onboard_dev);
+
+out:
+	device_unlock(dev);
+
+	return 0;
+}
+
 static void onboard_dev_attach_usb_driver(struct work_struct *work)
 {
 	int err;
@@ -308,6 +350,8 @@ static void onboard_dev_attach_usb_driver(struct work_struct *work)
 	err = driver_attach(&onboard_dev_usbdev_driver.driver);
 	if (err)
 		pr_err("Failed to attach USB driver: %pe\n", ERR_PTR(err));
+
+	usb_for_each_dev(NULL, onboard_dev_attach_usbdev);
 }
 
 #if IS_ENABLED(CONFIG_USB_ONBOARD_DEV_USB5744)
@@ -497,7 +541,10 @@ static int onboard_dev_probe(struct platform_device *pdev)
 	/*
 	 * The USB driver might have been detached from the USB devices by
 	 * onboard_dev_remove() (e.g. through an 'unbind' by userspace),
-	 * make sure to re-attach it if needed.
+	 * make sure to re-attach it if needed. The work also attaches USB
+	 * devices that were probed while this platform device was still
+	 * unbound (e.g. because this probe was deferred), see
+	 * onboard_dev_usbdev_probe().
 	 *
 	 * This needs to be done deferred to avoid self-deadlocks on systems
 	 * with nested onboard hubs.
@@ -606,14 +653,9 @@ static struct onboard_dev *_find_onboard_dev(struct device *dev)
 
 	/*
 	 * The presence of drvdata indicates that the platform driver finished
-	 * probing. This handles the case where (conceivably) we could be
-	 * running at the exact same time as the platform driver's probe. If
-	 * we detect the race we request probe deferral and we'll come back and
-	 * try again.
+	 * probing. Without it the USB device can't be attached to the platform
+	 * device yet; return NULL and let the caller decide what to do.
 	 */
-	if (!onboard_dev)
-		return ERR_PTR(-EPROBE_DEFER);
-
 	return onboard_dev;
 }
 
@@ -635,6 +677,21 @@ static int onboard_dev_usbdev_probe(struct usb_device *udev)
 
 	dev_set_drvdata(dev, onboard_dev);
 
+	/*
+	 * The platform device may not be bound yet (e.g. because its probe
+	 * was deferred). Bind to the USB device anyway, with NULL drvdata,
+	 * and let the platform driver attach it once it has probed (see
+	 * attach_usb_driver_work). Requesting probe deferral here wouldn't
+	 * work: the generic probe has already bound the interface drivers,
+	 * which triggers deferred probing, so the device would be re-probed
+	 * in a loop until the platform device is bound. Returning -ENXIO
+	 * doesn't work either: the device is then left without a driver and
+	 * can't be resumed once it has autosuspended. Returning -ENODEV
+	 * hands it to the generic driver for good.
+	 */
+	if (!onboard_dev)
+		return 0;
+
 	err = onboard_dev_add_usbdev(onboard_dev, udev);
 	if (err)
 		return err;
@@ -646,7 +703,8 @@ static void onboard_dev_usbdev_disconnect(struct usb_device *udev)
 {
 	struct onboard_dev *onboard_dev = dev_get_drvdata(&udev->dev);
 
-	onboard_dev_remove_usbdev(onboard_dev, udev);
+	if (onboard_dev)
+		onboard_dev_remove_usbdev(onboard_dev, udev);
 }
 
 static const struct usb_device_id onboard_dev_id_table[] = {

Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: misc: onboard_usb_dev: UAF race in onboard_dev_attach_usbdev()
--

commit f6a7eed8d40c04892ba2d262827780a7d1796445
Author: James Clark <jjc@jclark.com>

usb: misc: onboard_usb_dev: don't defer the USB device probe

This commit avoids returning -EPROBE_DEFER in onboard_dev_usbdev_probe() when
the corresponding platform device has not finished probing, preventing an
infinite re-probe loop caused by generic USB interface driver bindings.
Instead, it binds the USB device with NULL drvdata and extends
attach_usb_driver_work to attach such USB devices once the platform device
probes.

> diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
> index c604062404162..aac4e29c82335 100644
> --- a/drivers/usb/misc/onboard_usb_dev.c
> +++ b/drivers/usb/misc/onboard_usb_dev.c
[ ... ]
> @@ -301,6 +302,47 @@ static const struct attribute_group onboard_dev_group = {
>  __ATTRIBUTE_GROUPS(onboard_dev);
>  
>  
> +/*
> + * Attach a USB device that was bound by onboard_dev_usbdev_probe() while its
> + * platform device was still unbound. The device lock serialises this with
> + * the probe and with an unbind. This walks the USB bus, not the driver's
> + * device list: unbinding a device holds its lock while it waits for all
> + * references to its node on the driver's list to be dropped, so an iterator
> + * over that list that takes the device lock would deadlock with it.
> + */
> +static int onboard_dev_attach_usbdev(struct usb_device *udev, void *data)
> +{
> +	struct device *dev = &udev->dev;
> +	struct onboard_dev *onboard_dev;
> +	int err;
> +
> +	/* Unlocked pre-check, so as not to wait for every USB device's lock. */
> +	if (READ_ONCE(dev->driver) != &onboard_dev_usbdev_driver.driver)
> +		return 0;
> +
> +	device_lock(dev);
> +
> +	if (dev->driver != &onboard_dev_usbdev_driver.driver ||
> +	    dev_get_drvdata(dev))
> +		goto out;
> +
> +	onboard_dev = _find_onboard_dev(dev);
> +	if (IS_ERR_OR_NULL(onboard_dev))
> +		goto out;
> +
> +	err = onboard_dev_add_usbdev(onboard_dev, udev);
> +	if (err)
> +		dev_err(dev, "failed to attach to %s: %pe\n",
> +			dev_name(onboard_dev->dev), ERR_PTR(err));
> +	else
> +		dev_set_drvdata(dev, onboard_dev);

[Severity: High]
Can onboard_dev_attach_usbdev() race with platform device unbind and cause a
use-after-free of struct onboard_dev (and potentially struct platform_device)?

In _find_onboard_dev(), the platform device is looked up, its drvdata is read
without holding device_lock(&pdev->dev), and the reference on &pdev->dev is
immediately dropped before returning the devm-allocated onboard_dev pointer:

drivers/usb/misc/onboard_usb_dev.c:_find_onboard_dev() {
	...
	onboard_dev = dev_get_drvdata(&pdev->dev);
	put_device(&pdev->dev);

	return onboard_dev;
}

When the platform device is unbound (for example via sysfs unbind, parent hub
disconnect calling onboard_dev_destroy_pdevs(), or
CONFIG_DEBUG_TEST_DRIVER_REMOVE), __device_release_driver(&pdev->dev) calls
onboard_dev_remove() followed by device_unbind_cleanup():

drivers/usb/misc/onboard_usb_dev.c:onboard_dev_remove() {
	struct onboard_dev *onboard_dev = dev_get_drvdata(&pdev->dev);
	...
	onboard_dev->going_away = true;

	mutex_lock(&onboard_dev->lock);

	/* unbind the USB devices to avoid dangling references to this device */
	while (!list_empty(&onboard_dev->udev_list)) {
		...
	}
}

Because a USB device bound with NULL drvdata is not yet in
onboard_dev->udev_list, onboard_dev_remove() does not unbind it or take
device_lock(&udev->dev), nor does it cancel attach_usb_driver_work or clear
the platform device's drvdata. In device_unbind_cleanup(),
devres_release_all() frees the devm-allocated struct onboard_dev before
dev_set_drvdata(&pdev->dev, NULL) is called.

If onboard_dev_attach_usbdev() (or onboard_dev_usbdev_probe()) runs
concurrently with platform device unbind, _find_onboard_dev() can return the
non-NULL onboard_dev pointer just before devres_release_all() frees it,
allowing onboard_dev_add_usbdev() to lock and access freed memory.

Furthermore, even if onboard_dev_add_usbdev() sees onboard_dev->going_away set
under onboard_dev->lock and returns -EINVAL before devres_release_all() runs,
the error path in onboard_dev_attach_usbdev() then evaluates
dev_name(onboard_dev->dev) outside onboard_dev->lock, dereferencing
onboard_dev->dev after onboard_dev (and potentially pdev, since put_device()
was already called) has been freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030114705.3159093-1-jjc@jclark.com?part=1

Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[Arnd Bergmann]

On Fri, Oct 30, 2026, at 12:47, James Clark wrote:
> On a BeagleY-AI with BeagleBoard's device tree, kernels since commit
> f918e3804498 ("arm64: enable HAVE_SHARED_GPIOS for all ARM64 platforms")
> no longer boot to a usable system. The log repeats "hub 1-1:1.0: USB hub
> found" for the on-board TUSB8041 hub every few milliseconds, with no USB
> disconnect in between. The flood of uevents stalls udev and the boot ends
> in emergency mode. Reverting that commit makes the board boot normally
> again.
>
> The underlying problem is in onboard_usb_dev. Both peer hub nodes in this
> device tree carry the same reset-gpios, so the GPIO is now a shared one,
> and with CONFIG_GPIO_SHARED_PROXY=m (the default) the hub's platform
> probe is legitimately deferred until udev has loaded that module. The
> bootloader leaves the hub running, so the USB core enumerates it long
> before, and the USB device probe finds the platform device without
> driver data.

> At that point, onboard_dev_usbdev_probe() returns -EPROBE_DEFER. This is
> what causes the problem. The driver has generic_subclass set, so
> usb_probe_device() has already run the generic probe, which configured
> the device and bound the interface drivers. Each binding is a
> deferred-probe trigger, and the driver core re-triggers when a trigger
> happened during a probe that then deferred. So the device is re-probed
> straight away, reconfigured, its interfaces bound again, and so on for
> as long as the platform device remains unbound. This is not specific to
> shared GPIOs: it happens whenever the platform probe is deferred, and
> briefly whenever onboard_usb_dev is a module.

It sounds like the underlying problem should be fixed elsewhere,
possibly by preventing the deferred probe to be retried each time.
 
> +/*
> + * Attach a USB device that was bound by onboard_dev_usbdev_probe() 
> while its
> + * platform device was still unbound. The device lock serialises this 
> with
> + * the probe and with an unbind. This walks the USB bus, not the 
> driver's
> + * device list: unbinding a device holds its lock while it waits for 
> all
> + * references to its node on the driver's list to be dropped, so an 
> iterator
> + * over that list that takes the device lock would deadlock with it.
> + */
> +static int onboard_dev_attach_usbdev(struct usb_device *udev, void 
> *data)
> +{

This conceptually looks like a layering violation: no driver should
have to look through the list of devices like this.

> +	struct device *dev = &udev->dev;
> +	struct onboard_dev *onboard_dev;
> +	int err;
> +
> +	/* Unlocked pre-check, so as not to wait for every USB device's lock. 
> */
> +	if (READ_ONCE(dev->driver) != &onboard_dev_usbdev_driver.driver)
> +		return 0;
> +
> +	device_lock(dev);
> +
> +	if (dev->driver != &onboard_dev_usbdev_driver.driver ||
> +	    dev_get_drvdata(dev))
> +		goto out;
> +
> +	onboard_dev = _find_onboard_dev(dev);
> +	if (IS_ERR_OR_NULL(onboard_dev))
> +		goto out;

IS_ERR_OR_NULL() is almost always a bug, make sure you understan
the interface and what this can return. If the function can
actually return both error codes or NULL pointers, fix that
first.

     Arnd

Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[James Clark]

On Fri, 30 Oct 2026 13:24:49 +0100, Arnd Bergmann wrote:
> It sounds like the underlying problem should be fixed elsewhere,
> possibly by preventing the deferred probe to be retried each time.

Hi Arnd,

Thanks for the feedback. It looks to me like it is probably possible to
fix this better in the USB core. I will attempt a v2 fix that does this.

James

Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[Johan Hovold]

On Fri, Oct 30, 2026 at 06:47:05PM +0700, James Clark wrote:

> At that point, onboard_dev_usbdev_probe() returns -EPROBE_DEFER. This is
> what causes the problem. The driver has generic_subclass set, so
> usb_probe_device() has already run the generic probe, which configured
> the device and bound the interface drivers. Each binding is a
> deferred-probe trigger, and the driver core re-triggers when a trigger
> happened during a probe that then deferred. So the device is re-probed
> straight away, reconfigured, its interfaces bound again, and so on for
> as long as the platform device remains unbound. This is not specific to
> shared GPIOs: it happens whenever the platform probe is deferred, and
> briefly whenever onboard_usb_dev is a module.

This is indeed a bug in the onboard_usb_dev driver as a driver is not
allowed to probe defer after it has registered devices for precisely the
reason that it will trigger a probe loop (see commit fbc35b45f9f6 ("Add
documentation on meaning of -EPROBE_DEFER")).

Any USB driver using generic_subclass that may probe defer is currently
broken.

I guess you can look into replacing generic_subclass with having the
onboard_usb_dev driver call usb_generic_driver_probe() itself or
possibly adding a pre-probe callback to usb_device_driver.

Johan

Re: [PATCH] usb: misc: onboard_usb_dev: don't defer the USB device probe

[James Clark]

On Fri, 30 Oct 2026 14:34:59 +0100, Johan Hovold wrote:
> I guess you can look into replacing generic_subclass with having the
> onboard_usb_dev driver call usb_generic_driver_probe() itself or
> possibly adding a pre-probe callback to usb_device_driver.

Thanks for the suggestions. It looks to me like your pre-probe callback
idea will give the cleanest fix. I will try that for v2.

James