[PATCH] driver core: take parent lock on deferred probe

[PATCH] driver core: take parent lock on deferred probe

[Johan Hovold]

USB interfaces should be probed with the parent USB device lock held,
but this is not honoured when probe is deferred.

Add a new bus_probe_device_deferred() helper to handle deferred probe
where the parent device lock is not held.

Fixes: d1c3414c2a9d ("drivercore: Add driver probe deferral mechanism")
Reported-by: Mario Peter <mario.peter@leica-geosystems.com>
Link: https://lore.kernel.org/20261007131514.1260696-1-mario.peter@leica-geosystems.com
Cc: stable@vger.kernel.org	# 3.4
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/base/base.h |  1 +
 drivers/base/bus.c  | 28 +++++++++++++++++++++-------
 drivers/base/dd.c   |  7 ++++---
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index a5b7abc10ff0..3c36a2128f88 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -154,6 +154,7 @@ struct kobject *virtual_device_parent(void);
 
 int bus_add_device(struct device *dev);
 void bus_probe_device(struct device *dev);
+void bus_probe_device_deferred(struct device *dev);
 void bus_remove_device(struct device *dev);
 void bus_notify(struct device *dev, enum bus_notifier_event value);
 bool bus_is_registered(const struct bus_type *bus);
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index c51ad96d4de4..fd62594eb6f9 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -606,13 +606,7 @@ int bus_add_device(struct device *dev)
 	return error;
 }
 
-/**
- * bus_probe_device - probe drivers for a new device
- * @dev: device to probe
- *
- * - Automatically probe for a driver if the bus allows it.
- */
-void bus_probe_device(struct device *dev)
+static void __bus_probe_device(struct device *dev)
 {
 	struct subsys_private *sp = bus_to_subsys(dev->bus);
 	struct subsys_interface *sif;
@@ -630,6 +624,26 @@ void bus_probe_device(struct device *dev)
 	subsys_put(sp);
 }
 
+/**
+ * bus_probe_device - probe drivers for a new device
+ * @dev: device to probe
+ *
+ * - Automatically probe for a driver if the bus allows it.
+ */
+void bus_probe_device(struct device *dev)
+{
+	__bus_probe_device(dev);
+}
+
+void bus_probe_device_deferred(struct device *dev)
+{
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_lock(dev->parent);
+	__bus_probe_device(dev);
+	if (dev->parent && dev->bus->need_parent_lock)
+		device_unlock(dev->parent);
+}
+
 /**
  * bus_remove_device - remove device from bus
  * @dev: device to be removed
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9dc7cce9bd8d..87962f4e914c 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -86,8 +86,9 @@ static void deferred_probe_work_func(struct work_struct *work)
 	/*
 	 * This block processes every device in the deferred 'active' list.
 	 * Each device is removed from the active list and passed to
-	 * bus_probe_device() to re-attempt the probe.  The loop continues
-	 * until every device in the active list is removed and retried.
+	 * bus_probe_device_deferred() to re-attempt the probe.  The loop
+	 * continues until every device in the active list is removed and
+	 * retried.
 	 *
 	 * Note: Once the device is removed from the list and the mutex is
 	 * released, it is possible for the device get freed by another thread
@@ -121,7 +122,7 @@ static void deferred_probe_work_func(struct work_struct *work)
 		device_pm_move_to_tail(dev);
 
 		dev_dbg(dev, "Retrying from deferred list\n");
-		bus_probe_device(dev);
+		bus_probe_device_deferred(dev);
 		mutex_lock(&deferred_probe_mutex);
 
 		put_device(dev);
-- 
2.55.0

Re: [PATCH] driver core: take parent lock on deferred probe

[Danilo Krummrich]

On Tue Oct 13, 2026 at 4:06 PM CEST, Johan Hovold wrote:
> +void bus_probe_device_deferred(struct device *dev)
> +{
> +	if (dev->parent && dev->bus->need_parent_lock)
> +		device_lock(dev->parent);

This looks like it has the same potential UAF as the other patch.  It's quite
unfortunate we need the whole need_parent_lock thing just for USB.  IIUC the
main reason USB needs this is because interface devices share resources of USB
devices and we need to keep the locking order to avoid inversions, right?  Is
there more to this from the USB side of things?

I think we can probably do something like this

	struct device *parent = NULL;

	scoped_guard(device, dev) {
		if (dev->p->dead)
			return;
	
		if (dev->bus->need_parent_lock)
			parent = get_device(dev->parent);
	}

to avoid the problem, but it's still a bit ugly.

> +	__bus_probe_device(dev);
> +	if (dev->parent && dev->bus->need_parent_lock)
> +		device_unlock(dev->parent);
> +}

Re: [PATCH] driver core: take parent lock on deferred probe

[Alan Stern]

On Tue, Oct 13, 2026 at 07:43:08PM +0200, Danilo Krummrich wrote:
> On Tue Oct 13, 2026 at 4:06 PM CEST, Johan Hovold wrote:
> > +void bus_probe_device_deferred(struct device *dev)
> > +{
> > +	if (dev->parent && dev->bus->need_parent_lock)
> > +		device_lock(dev->parent);
> 
> This looks like it has the same potential UAF as the other patch.  It's quite
> unfortunate we need the whole need_parent_lock thing just for USB.  IIUC the
> main reason USB needs this is because interface devices share resources of USB
> devices and we need to keep the locking order to avoid inversions, right?  Is
> there more to this from the USB side of things?

That's pretty much it.

> I think we can probably do something like this
> 
> 	struct device *parent = NULL;
> 
> 	scoped_guard(device, dev) {
> 		if (dev->p->dead)
> 			return;
> 	
> 		if (dev->bus->need_parent_lock)
> 			parent = get_device(dev->parent);
> 	}
> 
> to avoid the problem, but it's still a bit ugly.
> 
> > +	__bus_probe_device(dev);
> > +	if (dev->parent && dev->bus->need_parent_lock)
> > +		device_unlock(dev->parent);
> > +}

Wouldn't it be simpler just to make each USB interface take a reference 
to its parent device?  And likewise for USB devices, if needed?

Alan Stern

Re: [PATCH] driver core: take parent lock on deferred probe

[Danilo Krummrich]

On Tue Oct 13, 2026 at 9:11 PM CEST, Alan Stern wrote:
> On Tue, Oct 13, 2026 at 07:43:08PM +0200, Danilo Krummrich wrote:
>> On Tue Oct 13, 2026 at 4:06 PM CEST, Johan Hovold wrote:
>> > +void bus_probe_device_deferred(struct device *dev)
>> > +{
>> > +	if (dev->parent && dev->bus->need_parent_lock)
>> > +		device_lock(dev->parent);
>> 
>> This looks like it has the same potential UAF as the other patch.  It's quite
>> unfortunate we need the whole need_parent_lock thing just for USB.  IIUC the
>> main reason USB needs this is because interface devices share resources of USB
>> devices and we need to keep the locking order to avoid inversions, right?  Is
>> there more to this from the USB side of things?
>
> That's pretty much it.
>
>> I think we can probably do something like this
>> 
>> 	struct device *parent = NULL;
>> 
>> 	scoped_guard(device, dev) {
>> 		if (dev->p->dead)
>> 			return;
>> 	
>> 		if (dev->bus->need_parent_lock)
>> 			parent = get_device(dev->parent);
>> 	}
>> 
>> to avoid the problem, but it's still a bit ugly.
>> 
>> > +	__bus_probe_device(dev);
>> > +	if (dev->parent && dev->bus->need_parent_lock)
>> > +		device_unlock(dev->parent);
>> > +}
>
> Wouldn't it be simpler just to make each USB interface take a reference 
> to its parent device?  And likewise for USB devices, if needed?

The driver core already holds a reference count of the parent between
device_add() and device_del(), which is where a "parent" is technically defined.
But the deferred retry can extend beyond device_del().

USB interfaces already take an additional reference to their USB device in
usb_set_configuration() and drop it in usb_release_interface(), so they should
already be protected against this particular UAF.

The issue concerns a USB device and its parent hub's USB device.  (This makes me
wonder whether probing a USB device actually requires the parent USB HUB
device's lock to be held, or whether this is just a consequence of sharing the
same usb_bus_type?)

In theory a USB device could also keep a reference count to its parent, but then
the driver core code would rely on an implementation detail of its users; I'd
prefer not to make driver core's access to dev->parent depend on references
maintained by individual subsystems.

I'd even go a step further any say that if subsystems try to keep this pointer
valid beyond what's guaranteed by the driver core, they should do this with a
separate bus specific pointer.  The reason is that otherwise the lifetime and
ownership definition of this pointer does not depend on API contracts, but on
individual implementation details, which is a very common source for all kinds
of bugs.