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

[sashiko-bot@kernel.org]

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
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;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920151248.46936-1-adrian.hunter@intel.com?part=16

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