Re: [PATCH v7 04/12] i3c: master: Add support for devices using SETAASA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Bypassing PID validation for SETAASA devices permits identical dummy PIDs, causing sysfs name collisions during registration and triggering a Use-After-Free.
- [High] Sending SETAASA and SETHID CCCs will unconditionally fail bus initialization on most hardware master controllers.
- [High] SETAASA devices become permanently unreachable after system hibernation/restore because the SETAASA broadcast is not re-sent.
--

--- Patch [4]: [PATCH v7 04/12] i3c: master: Add support for devices using SETAASA ---
commit 61911732853b10649effe5164454ca8a4a87e970
Author: Akhil R <akhilrajeev@nvidia.com>
Subject: i3c: master: Add support for devices using SETAASA

This patch adds support for discovering and configuring devices using the
SETAASA method, which statically assigns addresses without using ENTDAA.
It adheres to the MIPI Discovery and Configuration Specification.

Link: https://www.mipi.org/mipi-disco-for-i3c-download

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 623c6b2247d9f..87749a739e62d 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -1878,6 +1926,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
>  	if (ret)
>  		goto err_free_dev;
>  
> +	/*
> +	 * For devices using SETAASA instead of ENTDAA, the address is statically
> +	 * assigned. Update the dynamic address to the provided static address.
> +	 * Reattach the I3C device after updating the dynamic address with the same
> +	 * static address. It is not mandatory for such devices to implement CCC
> +	 * commands like GETPID, GETDCR etc. Hence, we can return after reattaching.
> +	 */
> +	if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
> +		ret = i3c_master_reattach_i3c_dev_locked(i3cdev, 0);
> +		if (ret)
> +			goto err_detach_dev;
> +
> +		return 0;
> +	}

[Severity: High]
Does this early return prevent the device from acquiring a unique PID?

Since PID validation is bypassed for SETAASA devices in
i3c_master_add_i3c_boardinfo(), multiple devices can share the same default
PID (like 0). Because this skips i3c_master_retrieve_dev_info(), the dummy
PID is never replaced by a real one.

When i3c_master_register_new_i3c_devs() attempts to register these devices,
the sysfs registration fails due to duplicate names. The error handling calls
put_device() but leaves a dangling pointer in desc->dev.

Could this dangling pointer lead to a use-after-free when
i3c_master_unregister_i3c_devs() later accesses and frees it again?

[ ... ]
> @@ -2272,6 +2346,19 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  			i3c_master_early_i3c_dev_add(master, i3cboardinfo);
>  	}
>  
> +	/*
> +	 * SETAASA is a broadcast CCC. Issue it after SETDASA so that devices
> +	 * configured for SETDASA (or supporting both methods) are assigned
> +	 * first, matching MIPI DISCO guidance to prefer SETDASA when both are
> +	 * available. Targets that already have a dynamic address ignore the
> +	 * later SETAASA broadcast.
> +	 */
> +	if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		ret = i3c_master_setaasa_locked(master);
> +		if (ret)
> +			goto err_rstdaa;
> +	}

[Severity: High]
What happens here if the underlying master controller driver rejects the
SETAASA and SETHID CCC commands?

Most mainline I3C master drivers, like Cadence and DesignWare, implement a
strict allowlist in their supports_ccc_cmd callback. If they reject these
new commands, i3c_master_send_ccc_cmd_locked() returns -EOPNOTSUPP.

Will this cause the entire I3C bus initialization to abort and detach all
devices, effectively breaking all I3C communications for that controller?

[Severity: High]
How are SETAASA devices handled during system resume?

During resume, i3c_master_do_daa_ext() issues an RSTDAA broadcast to reset
dynamic addresses and then calls ENTDAA. Since SETAASA devices do not
participate in ENTDAA, and SETAASA is not re-broadcasted there, will these
devices lose their dynamic address on the physical bus?

If the kernel still retains the i3c_dev_desc with the dynamic address
uncleared, won't all subsequent I/O to these targets fail?

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

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