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

On Tue, 28 Jul 2026 07:22:01 +0000, Sashiko AI review <[email protected]> wrote:
> commit 61911732853b10649effe5164454ca8a4a87e970
> Author: Akhil R <[email protected]>
>
> 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.
>
> [ ... ]
>
> > @@ -1878,6 +1926,22 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
> > +	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?

As clarified in v6, devices without a PID are named using their static address
in the next patch of this series, and two devices cannot share the same static
address. The dangling desc->dev on device_register() failure is pre-existing
and will have to be fixed separately.

> [ ... ]
>
> > @@ -2272,6 +2346,19 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> > +	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?

This is clarified in v6. It's an opt-in property and existing drivers are
unaffected.

> [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?

i3c_master_do_daa_ext() is only reached through the mipi-i3c-hci driver when
HCI_QUIRK_RPM_ALLOWED is set and not through dw-i3c-master. So this issue does
not have any impact today since HCI_QUIRK_RPM_ALLOWED is set only by
intel-lpss-i3c platform.

The same issue also applies to other statically configured devices like SETDASA
devices that do not implement ENTDAA. It will be better to fix this separately.

Best Regards,
Akhil

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c