[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] driver core: unbalanced parent lock in deferred probe due to TOCTOU on dev->parent
- [High] driver core: Use-After-Free of dev->parent in deferred probe
--

commit fa3fc53326d3b088884e2e7e5dc50b5162e40379
Author: Johan Hovold <johan@kernel.org>

driver core: take parent lock on deferred probe

This commit adds a new helper bus_probe_device_deferred() to take
the parent device lock during deferred probe. It modifies the deferred
probe worker to use this helper instead of bus_probe_device().

Link: https://lore.kernel.org/20261007131514.1260696-1-mario.peter@leica-geosystems.com

> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 18f12c6f910a0..ebd069d98d255 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
[ ... ]
> @@ -630,6 +624,26 @@ void bus_probe_device(struct device *dev)
[ ... ]
> +void bus_probe_device_deferred(struct device *dev)
> +{
> +	if (dev->parent && dev->bus->need_parent_lock)
> +		device_lock(dev->parent);

[Severity: High]
Does this code risk a use-after-free of the parent device?

When called from deferred_probe_work_func(), the worker thread has only
taken a reference via get_device(dev) and dropped the deferred_probe_mutex.

If device_del(dev) runs concurrently, it can drop the device's reference to
its parent via put_device(parent). If that was the last reference, the parent
object could be freed immediately.

Because we only pinned the device itself and not the parent, could this read
of dev->parent and the subsequent device_lock() access freed memory?

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

[Severity: High]
Is it possible for dev->parent to change during the execution of
__bus_probe_device(dev)?

If a concurrent thread executes device_move() on the device while we are
probing, dev->parent could be updated. This happens because device_move()
updates dev->parent while only holding device_pm_lock(), which is not held
here.

This would result in evaluating dev->parent a second time and potentially
calling device_unlock() on the new parent instead of the one that was
originally locked, causing a lock imbalance.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013140615.1226992-1-johan@kernel.org?part=1