Re: [PATCH v5 06/12] i3c: master: match I3C device through DT and ACPI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Suppressing the native I3C modalias breaks module autoloading for standard (non-SETAASA) I3C devices that possess an OF or ACPI node.

Pre-existing issues:
- [Critical] Kernel stack memory disclosure in `i3c_device_uevent`, `modalias_show`, and `i3c_device_match_id` when `i3cdev->desc` is NULL.
- [High] Lockless access to `i3cdev->desc` in `i3c_device_match()` can lead to a Use-After-Free during concurrent DAA or Hot-Join operations.
--

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

i3c: master: match I3C device through DT and ACPI

This patch allows SETAASA-based I3C devices to match through Device Tree
or ACPI when they cannot be identified by PID or DCR. It emits OF and ACPI
modaliases so firmware-matched devices can autoload their drivers.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -320,6 +321,15 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
>  	const struct i3c_device *i3cdev = dev_to_i3cdev(dev);
>  	struct i3c_device_info devinfo;
>  	u16 manuf, part, ext;
> +	int rc;
> +
> +	rc = of_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;

[Severity: High]
Does returning early here break module autoloading for standard I3C devices
that have an OF or ACPI node?

If a standard non-SETAASA device has an OF node, it will only emit the
of: modalias and skip generating the standard i3c: modalias.

If the device's driver only provides an I3C id_table, udev might fail to
find a matching module. Furthermore, since i3c_device_match() restricts
of_driver_match_device() strictly to devices with the
I3C_ADDR_METHOD_SETAASA flag, standard I3C devices wouldn't be able to match
via Device Tree either.

> +
> +	rc = acpi_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;
>  
>  	if (i3cdev->desc)
>  		devinfo = i3cdev->desc->info;

[Severity: Critical]
This is a pre-existing issue, but can this leak uninitialized stack memory
to userspace?

When a device is unregistered, i3c_master_unregister_i3c_devs() sets
i3cdev->dev->desc = NULL before calling device_unregister(). This triggers
a uevent that calls i3c_device_uevent().

If i3cdev->desc is NULL, the devinfo stack variable is never initialized,
but its contents (like devinfo.pid and devinfo.dcr) are still used to
construct the modalias string right below this block.

Could this expose kernel stack memory via netlink messages, or via sysfs
reads in modalias_show()? A similar uninitialized read also occurs in
i3c_device_match_id() when i3c_device_get_info() is called with a NULL
descriptor.

[ ... ]
> @@ -345,15 +355,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;

[Severity: High]
This is a pre-existing issue, but is it safe to access i3cdev->desc here
without holding the bus read lock?

During dynamic address assignment or hot-join operations,
__i3c_master_add_i3c_dev_locked() may find a duplicate device, reassign
a new descriptor, and free the old descriptor.

Since i3c_device_match() is called by the driver core without acquiring
the bus lock, might a concurrent hot-join thread free i3cdev->desc while
it is being dereferenced here, leading to a use-after-free? This same race
could happen when i3cdev->desc->info is accessed by i3c_device_get_info()
during matching.

> +
> +	/*
> +	 * SETAASA-based devices 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))
>  		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;
> +	}
> +
>  	return 0;
>  }

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