Re: [PATCH v10 02/10] drivers: core: Use fw_devlink_set_device()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless read of `fwnode->dev` can lead to Use-After-Free and NULL pointer dereferences.
--

--- Patch [2]: [PATCH v10 02/10] drivers: core: Use fw_devlink_set_device() ---
commit 80a2fb2740f34981de6ee4d63b13800473b38e85
Author: Herve Codina <[email protected]>

drivers: core: Use fw_devlink_set_device()

This patch updates the driver core to use the dedicated
fw_devlink_set_device() helper instead of assigning the fwnode->dev field
directly.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4d026682944f2..c205125344cf3 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
[ ... ]
> @@ -3925,7 +3925,7 @@ void device_del(struct device *dev)
>  	device_unlock(dev);
>  
>  	if (dev->fwnode && dev->fwnode->dev == dev)
> -		dev->fwnode->dev = NULL;
> +		fw_devlink_set_device(dev->fwnode, NULL);
>  
>  	/* Notify clients of device removal.  This call must come
>  	 * before dpm_sysfs_remove().

[Severity: High]
This is a pre-existing issue, but does this lockless assignment expose a race
condition with concurrent readers?

In device_del(), fwnode->dev is cleared locklessly via
fw_devlink_set_device(). Concurrently, other operations holding
fwnode_link_lock (which doesn't serialize with device_del()) read fwnode->dev.

For example, if get_dev_from_fwnode() is called concurrently:

drivers/base/core.c:get_dev_from_fwnode() {
    ...
    return get_device((fwnode)->dev);
}

If the thread is preempted between reading fwnode->dev and dereferencing it
in get_device(), and device_del() finishes allowing the device to be freed,
could this result in a use-after-free?

Similarly, in __fw_devlink_pickup_dangling_consumers():

drivers/base/core.c:__fw_devlink_pickup_dangling_consumers() {
    ...
    if (fwnode->dev && fwnode->dev->bus)
    ...
}

Could the compiler reload fwnode->dev between the NULL check and the
dereference, leading to a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2