Re: [PATCH V3 16/17] i3c: Restrict HDR modes to those supported by the bus and target

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

On 20/09/2026 18:33, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] i3c: missing NULL check for dev->desc in i3c_device_get_supported_xfer_mode
> --
> 
> commit 62529bd655d4c18d69da34f93d179ce363aba85c
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: Restrict HDR modes to those supported by the bus and target
> 
> This commit refactors i3c_device_get_supported_xfer_mode() to consider HDR
> modes allowed by the controller, bus configuration, and target. It
> introduces a helper i3c_dev_supported_xfer_modes_locked() and adds bus
> locking in normal use mode to access the device descriptor safely.
> 
>> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
>> index f1ba363b22a17..4df4682af1504 100644
>> --- a/drivers/i3c/device.c
>> +++ b/drivers/i3c/device.c
> [ ... ]
>> @@ -301,16 +301,23 @@ i3c_device_match_id(struct i3c_device *i3cdev,
>>  EXPORT_SYMBOL_GPL(i3c_device_match_id);
>>  
>>  /**
>> - * i3c_device_get_supported_xfer_mode - Returns the supported transfer mode by
>> - *					connected master controller.
>> + * i3c_device_get_supported_xfer_mode - Returns the transfer modes supported by
>> + *					the connected master controller and @dev,
>> + *					and allowed by the bus configuration.
>>   * @dev: I3C device
>>   *
>>   * Return: a bit mask, which supported transfer mode, bit position is defined at
>> - *	   enum i3c_hdr_mode
>> + *	   enum i3c_xfer_mode
>>   */
>>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>>  {
>> -	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
>> +	u32 modes;
>> +
>> +	i3c_bus_normaluse_lock(dev->bus);
>> +	modes = i3c_dev_supported_xfer_modes_locked(dev->desc);
> 
> [Severity: High]
> Does this code risk a NULL pointer dereference during device unregistration?
> 
> If a device is being concurrently unregistered,
> i3c_master_unregister_i3c_devs() asynchronously sets dev->desc = NULL
> before driver unbinding completes. Other APIs in the I3C subsystem (such

FWIW: no it doesn't - that was fixed already, refer:

commit d2c743efd2d1ee64e94324664808f623dd865872
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Thu Jul 23 10:57:47 2026 +0300

    i3c: master: Fix info leak and UAF in device unregister path


> as i3c_device_get_info() and i3c_device_disable_ibi()) appear to guard
> against this teardown race condition by explicitly checking if (dev->desc)
> inside the bus lock.
> 
> Without a prior NULL check here, dev->desc is passed into
> i3c_dev_supported_xfer_modes_locked(), which unconditionally dereferences
> it:
> 
> include/linux/i3c/master.h:i3c_dev_get_master() {
>     return dev->common.master;
> }
> 
> drivers/i3c/master.c:i3c_dev_supported_xfer_modes_locked() {
>     struct i3c_master_controller *master = i3c_dev_get_master(dev);
>     ...
> }
> 
> Could a check for dev->desc be added inside the lock before calling the
> helper?
> 
>> +	i3c_bus_normaluse_unlock(dev->bus);
>> +
>> +	return modes;
>>  }
> 


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