Re: [PATCH v3 07/13] i3c: master: match I3C device through DT and ACPI

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] I3C devices matched via DT or ACPI will not automatically load their corresponding kernel modules because the I3C subsystem's `uevent` handler fails to generate OF or ACPI modaliases.
- [High] Lockless access to `i3cdev->desc` in `i3c_device_match()` introduces a TOCTOU race and a Use-After-Free / NULL Pointer Dereference.
- [High] Uninitialized memory read in `i3c_device_match_id()` when `i3cdev->desc` is NULL.
- [Medium] Architectural restriction of firmware matching to SETAASA devices.
--

commit b086c2a6b40084b4476548d93f6440348d3a2ee5
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: match I3C device through DT and ACPI

This patch allows SETAASA-based I3C devices to be matched through Device
Tree or ACPI, as they might not implement deviceinfo CCC commands and lack
standard ID matching.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index fbe8710c4c56f8..0be83bfdb563f5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -343,15 +344,32 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>  {
>  	struct i3c_device *i3cdev;
>  	const struct i3c_driver *i3cdrv;
> +	u8 static_addr_method = 0;
>  
>  	if (dev->type != &i3c_device_type)
>  		return 0;
>  
>  	i3cdev = dev_to_i3cdev(dev);
>  	i3cdrv = drv_to_i3cdrv(drv);
> -	if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
> +
> +	if (i3cdev->desc && i3cdev->desc->boardinfo)
> +		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;

Is it safe to access i3cdev->desc and boardinfo here without holding the I3C
bus lock?

During a Dynamic Address Assignment or Hot-Join event,
i3c_master_add_i3c_dev_locked() can replace the descriptor and free the old
one. Because this match function runs without locks, it might see a non-NULL
descriptor, get preempted while the descriptor is freed, and then dereference
freed memory.

Additionally, the compiler might fetch i3cdev->desc twice here, which could
result in a NULL pointer dereference if it gets set to NULL concurrently
during device unregistration.

> +
> +	/*
> +	 * SETAASA based device need not always have a matching ID since
> +	 * it is not mandatory for such devices to implement deviceinfo
> +	 * CCC commands. Allow them to register through DT or ACPI.
> +	 */
> +	if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))

If i3cdev->desc is NULL, does i3c_device_match_id() operate on uninitialized
memory?

The check above indicates i3cdev->desc can be NULL, but i3c_device_get_info()
will return early without initializing the devinfo structure when the
descriptor is missing, leaving i3c_device_match_id() to extract PID
components from uninitialized stack memory.

>  		return 1;
>  
> +	if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		if (of_driver_match_device(dev, drv))
> +			return 1;
> +		if (acpi_driver_match_device(dev, drv))
> +			return 1;
> +	}

Is there a reason to restrict DT and ACPI matching to only SETAASA devices?

Generic firmware matching is generally available to any device with a valid
firmware node. Since of_driver_match_device() and acpi_driver_match_device()
safely return 0 if no firmware match exists, gating this behind a specific
address assignment type might unnecessarily restrict firmware-based matching
for standard I3C devices.

Also, does i3c_device_uevent() need an update to generate OF and ACPI
modaliases?

If drivers omit an id_table and rely solely on DT or ACPI matching, they
might not be automatically loaded by userspace during device discovery, since
the uevent handler only emits standard I3C modaliases.

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=7