Re: [PATCH 1/3] i3c: master: Update dev_nack_retry_count under maintenance lock

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

On 16/06/2026 14:51, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Preexisting Use-After-Free (UAF) due to devres freeing the I3C master controller while its embedded `struct device` is still referenced.
> --
> 
> commit d8d2b455dc4bd3d672311fc14258b4a9d0b83e8b
> Author: Adrian Hunter <adrian.hunter@intel.com>
> Subject: i3c: master: Update dev_nack_retry_count under maintenance lock
> 
> This commit protects master->dev_nack_retry_count against concurrent sysfs
> updates by updating it while holding the bus maintenance lock. It also
> combines adjacent return statements and updates the read path to hold the bus
> normaluse lock.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index 372d911ecbadc..05ea9e3c4d46f 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -744,7 +744,14 @@ static DEVICE_ATTR_RW(hotjoin);
>>  static ssize_t dev_nack_retry_count_show(struct device *dev,
>>  					 struct device_attribute *attr, char *buf)
>>  {
>> -	return sysfs_emit(buf, "%u\n", dev_to_i3cmaster(dev)->dev_nack_retry_count);
>> +	struct i3c_bus *i3cbus = dev_to_i3cbus(dev);
>> +	ssize_t ret;
>> +
>> +	i3c_bus_normaluse_lock(i3cbus);
>> +	ret = sysfs_emit(buf, "%u\n", dev_to_i3cmaster(dev)->dev_nack_retry_count);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but since we are modifying sysfs
> attributes here, is there a pre-existing use-after-free risk related to how
> the i3c_master_controller is allocated and freed?
> 
> Many I3C master drivers (e.g., dw-i3c-master.c) allocate their driver context
> (which embeds struct i3c_master_controller) using devm_kzalloc(). During
> unbind/removal, device_unregister(&master->dev) is called, which drops the
> initial reference. If a user-space process holds a sysfs file open (like this
> dev_nack_retry_count attribute), or if an attacker blocks in a sysfs read or
> write using userfaultfd, the kobject reference count remains elevated.
> 
> The driver's remove function then returns, and the devres framework
> automatically frees the driver context memory. Later, when the final reference
> is dropped, the driver core calls i3c_masterdev_release():
> 
> drivers/i3c/master.c:i3c_masterdev_release() {
>         struct i3c_master_controller *master = dev_to_i3cmaster(dev);
>         struct i3c_bus *bus = dev_to_i3cbus(dev);
> 
>         if (master->wq)
>                 destroy_workqueue(master->wq);
>         ...
> }
> 
> Will this result in a use-after-free since the release function accesses the
> master struct after its memory has already been freed by devres?

No.  Open sysfs files do not block device release.  On the other hand,
driver remove is blocked if a driver sysfs callback is in progress.


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c