Re: [PATCH 3/5] driver core: async device shutdown infrastructure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprotected lockless modification of `devices_kset->list` leading to list corruption.
--

commit 3c22e8d639c6f39d1755272000b5af82fc85ebc8
Author: David Jeffery <djeffery@redhat.com>

driver core: async device shutdown infrastructure

This commit introduces an asynchronous device shutdown infrastructure. Devices
marked for async shutdown can be processed in parallel, waiting only for
their dependencies. This helps minimize false dependencies on unrelated
synchronous devices.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3b3d983b17470..751fe2e13b3aa 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c

[ ... ]

> +static void start_async_shutdown_devices(void)
> +{
> +	struct device *dev, *next, *ndev, *needs_put = NULL;
> +	bool clear_async = false;
> +
> +	if (!async_shutdown)
> +		return;
> +
> +	spin_lock(&devices_kset->list_lock);
> +restart:
> +	list_for_each_entry_safe_reverse(dev, next, &devices_kset->list,
> +					 kobj.entry) {
> +		if (wants_async_shutdown(dev)) {
> +			if (clear_async) {
> +				dev_clear_async_shutdown(dev);
> +				continue;
> +			}
> +			get_device(dev);
> +
> +			if (!list_entry_is_head(next, &devices_kset->list,
> +						kobj.entry))
> +				ndev = get_device(next);
> +			else
> +				ndev = NULL;
> +			spin_unlock(&devices_kset->list_lock);
> +
> +			if (shutdown_device_async(dev)) {
> +				list_del_init(&dev->kobj.entry);

[Severity: High]
Could this lead to list corruption?

In start_async_shutdown_devices(), the devices_kset->list_lock is released
before calling shutdown_device_async(). If the async schedule succeeds,
list_del_init() is called to remove the device from devices_kset->list
without reacquiring the lock.

Since this is a global doubly-linked list and concurrent device hotplug or
unplug events can still occur during system shutdown, doesn't modifying
this list without the lock risk corrupting the list pointers and causing
memory corruption or a panic?

> +			} else {
> +				/*
> +				 * async failed, clean up extra reference
> +				 * and run shutdown from the sync shutdown loop
> +				 */
> +				clear_async = true;
> +				put_device(dev);
> +			}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616152219.6268-1-djeffery@redhat.com?part=3