Re: [PATCH v7 05/12] i3c: master: Add support for devices without PID

On Tue, 28 Jul 2026 07:20:40 +0000, Sashiko AI review <[email protected]> wrote:
> commit 978db33da88aedc2a14abd69fdc128b5e3961e92
> Author: Akhil R <[email protected]>
>
> i3c: master: Add support for devices without PID
>
> Devices using SETAASA for address assignment are not required to have
> a 48-bit PID according to the I3C specification. Allow such devices to
> register and use the static address where PID was required.
>
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 87749a739e62d..488177d95e484 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
>
> [ ... ]
>
> [Severity: High]
> This is a pre-existing issue, but does device_register() failure in this
> function leave a dangling pointer that causes a use-after-free later?
>
> When put_device() frees desc->dev->dev, desc->dev is not set to NULL.
> Later, if the master is unregistered, i3c_master_unregister_i3c_devs()
> iterates through the list and dereferences this dangling pointer.
>
> Could desc->dev be set to NULL after put_device() to prevent this?
> [ ... ]
>
> > @@ -2401,8 +2410,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
> >  	list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> > -		if (i3cdev->info.pid != i3cboardinfo->pid)
> > -			continue;
> > +		if (i3cboardinfo->pid) {
> > +			if (i3cdev->info.pid != i3cboardinfo->pid)
> > +				continue;
> > +		} else {
> > +			if (!i3cboardinfo->static_addr ||
> > +			    i3cdev->info.static_addr != i3cboardinfo->static_addr)
> > +				continue;
> > +		}
>
> [Severity: High]
> Will this fallback matching logic fail for SETAASA devices discovered
> dynamically?
>
> If a SETAASA device without a PID in the Device Tree loses its dynamic
> address and asserts a Hot-Join, the master will execute Dynamic Address
> Assignment (DAA).
>
> During DAA, __i3c_master_add_i3c_dev_locked() allocates a new device
> descriptor and initializes newdev's static_addr to 0, because the static
> address cannot be retrieved via CCCs.
>
> When i3c_master_attach_boardinfo(newdev) is then called,
> i3cdev->info.static_addr will be 0. Would this cause the condition
> i3cdev->info.static_addr != i3cboardinfo->static_addr to always evaluate
> to true, skipping the boardinfo assignment and preventing the driver from
> binding?

A device without a PID cannot be discovered through DAA, so this should not
happen. Per the MIPI I3C specification, a Target that supports ENTDAA shall
have a 48-bit Provisioned ID (Section 5.1.4.1.1), which it transmits during
the procedure, and Hot-Join is only compatible with ENTDAA - "Targets that
do not support ENTDAA shall not use Hot-Join" (Section 5.1.5). A device
that supports both SETAASA and ENTDAA does have a PID and is expected
to describe it in DT/ACPI, in which case the PID match applies.

> >  		i3cdev->boardinfo = i3cboardinfo;
> >  		i3cdev->info.static_addr = i3cboardinfo->static_addr;
> > @@ -2416,8 +2435,12 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> > +	if (!refdev->info.pid)
> > +		return NULL;
> > +
> >  	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
> > -		if (i3cdev != refdev && i3cdev->info.pid == refdev->info.pid)
> > +		if (i3cdev != refdev && i3cdev->info.pid &&
> > +		    i3cdev->info.pid == refdev->info.pid)
> >  			return i3cdev;
> >  	}
>
> [Severity: High]
> Does this logic lead to duplicate device creation for SETAASA devices that
> participate in Dynamic Address Assignment?
>
> If a SETAASA device is defined in the DT without a PID, it is initially
> added to the master's bus list with pid = 0. If the device later asserts a
> Hot-Join, the master executes DAA, discovers the device, and retrieves its
> true physical PID.
>
> Since the existing device has pid = 0, the check here for i3cdev->info.pid
> and i3cdev->info.pid == refdev->info.pid will fail to match the newly
> discovered PID to the existing device. Will this cause the master to add
> the newly discovered device instead of detaching and updating the existing
> one, resulting in duplicate devices on the bus?

This also needs a device without a PID to take part in ENTDAA, which the
specification does not allow, as above. A device that already has a Dynamic
Address assigned through SETAASA does not participate in ENTDAA either
(Section 5.1.4.2).

The !pid checks are intentional in this patch. Devices without a PID have
pid == 0 and would otherwise match each other, and one of them would be
freed as a duplicate. Devices that do have a PID are matched as before.

Best Regards,
Akhil

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