Re: [PATCH V5 05/14] i3c: Make dev->desc locking assumptions explicit

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

Pre-existing issues:
- [High] Use-After-Free and potential information leak in `i3c_device_uevent()` due to missing locks during concurrent device descriptor replacement.
--

--- Patch [5]: [PATCH V5 05/14] i3c: Make dev->desc locking assumptions explicit ---
commit 278282a9eadd6c355b60a12ba9af4c6b5982e144
Author: Adrian Hunter <[email protected]>

i3c: Make dev->desc locking assumptions explicit

This commit removes an unnecessary NULL pointer check in
i3c_device_get_info() since the descriptor pointer is guaranteed
valid under the bus normal-use lock.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index a3778282e84c2..5e6df6de02834 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -101,8 +101,7 @@ void i3c_device_get_info(const struct i3c_device *dev,
>  		return;
>  
>  	i3c_bus_normaluse_lock(dev->bus);
> -	if (dev->desc)
> -		*info = dev->desc->info;
> +	*info = dev->desc->info;
>  	i3c_bus_normaluse_unlock(dev->bus);

[Severity: High]
This is a pre-existing issue, but does the same locking rule need to be
applied to i3c_device_uevent() in drivers/i3c/master.c?

It appears that i3c_device_uevent() currently reads i3cdev->desc without
holding the normal-use bus lock:

drivers/i3c/master.c:i3c_device_uevent() {
        struct i3c_device_info devinfo;
        ...
        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;
        ...
}

Could this sequence cause a use-after-free if a concurrent device discovery
or hot-join occurs? For example, if __i3c_master_add_i3c_dev_locked() 
replaces and frees the descriptor while the device is still accessible 
via sysfs uevents:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
        ...
        i3c_master_detach_i3c_dev(olddev);
        i3c_master_free_i3c_dev(olddev);
        ...
}

Also, if i3cdev->desc evaluates to NULL during the uevent, could this
leave the devinfo stack variable uninitialized and expose kernel stack
memory to user space via the modalias string?

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

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c