Re: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[Adrian Hunter <adrian.hunter@intel.com>]

On 04/08/2026 18:09, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [Critical] The patch transforms a predictable NULL pointer dereference into an exploitable Use-After-Free of `master->this` when an I3C device outlives its master controller's unregistration.

No. 'desc' is never NULL however it could be being changed concurrently
prior to being freed.  UAF of 'desc' is no different to UAF of master->this
(which is anyway being addressed by a different patch
"i3c: master: Fix use-after-free of master->this"),
so nothing "transforms" here.

> 
> Pre-existing issues:
> - [High] Uninitialized stack memory (`devinfo`) is used for driver matching when `i3cdev->desc` is NULL.
> - [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists when accessing `i3cdev->desc->boardinfo`, leading to a Use-After-Free or NULL pointer dereference.
> --
> 
> --- Patch [2]: [PATCH V3 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode() ---
> commit 97cfccca08da732124561cbc1aa49b571a84dd8c
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()
> 
> This patch modifies i3c_device_get_supported_xfer_mode() to use dev->bus
> instead of dev->desc to obtain the master controller. This avoids a potential
> unlocked dereference of dev->desc.
> 
>> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
>> index 101eaa77de686..a3778282e84c2 100644
>> --- a/drivers/i3c/device.c
>> +++ b/drivers/i3c/device.c
>> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
>>   */
>>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>>  {
>> -	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
>> +	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
> 
> [Severity: Critical]
> Does this change introduce a use-after-free if the device outlives the master
> controller?
> 
> When a master is unregistered, i3c_master_unregister() calls
> i3c_master_unregister_i3c_devs() to unregister child devices. If a child
> device has an elevated refcount (e.g., from an open file descriptor), its
> struct i3c_device outlives the unregistration.
> 
> i3c_master_bus_cleanup() then calls i3c_master_detach_free_devs(), which
> explicitly frees master->this.
> 
> If the child device later calls i3c_device_get_supported_xfer_mode(), it
> would dereference the explicitly freed master->this pointer. The previous
> code safely faulted on a NULL dev->desc, but this new path appears to access
> freed memory.
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but does i3c_device_match() have a TOCTOU race
> condition leading to a use-after-free?
> 
> drivers/i3c/master.c:i3c_device_match() {
>         if (i3cdev->desc && i3cdev->desc->boardinfo)
>                 static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
> }
> 
> If i3c_device_match() accesses i3cdev->desc without holding the device lock
> during concurrent master unregistration, a context switch could occur right
> after i3cdev->desc is checked.
> 
> If the unregistration thread then frees the descriptor via kfree() in
> i3c_master_bus_cleanup(), the subsequent evaluation of
> i3cdev->desc->boardinfo will dereference a freed pointer.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can i3c_device_match_id()
> evaluate uninitialized stack memory?
> 
> During concurrent driver registration and master unregistration, if
> i3c_master_unregister_i3c_devs() sets i3cdev->dev->desc = NULL,
> i3c_device_match() might still proceed to call i3c_device_match_id().
> 
> drivers/i3c/device.c:i3c_device_match_id() {
>         struct i3c_device_info devinfo;
>         ...
>         i3c_device_get_info(i3cdev, &devinfo);
>         
>         manuf = I3C_PID_MANUF_ID(devinfo.pid);
> }
> 
> If i3cdev->desc is NULL, i3c_device_get_info() bails out safely without
> populating devinfo, leaving the stack memory completely uninitialized when
> it is subsequently read for driver matching.
>