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

[sashiko-bot@kernel.org]

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