Re: [PATCH v10 1/9] i3c: master: rename i3c_master_reattach_i3c_dev() to *_locked

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The return value of `i3c_master_reattach_i3c_dev_locked()` is ignored in `i3c_master_add_i3c_dev_locked()`, which can lead to an I3C address collision and bus address slot corruption on failure.
--

commit c8a363e4e146cbd0fba01ffdf2e47defa9bd5b84
Author: Lakshay Piplani <lakshay.piplani@nxp.com>

i3c: master: rename i3c_master_reattach_i3c_dev() to *_locked

This commit renames i3c_master_reattach_i3c_dev() to *_locked() to make the
locking requirement explicit and consistent with other I3C core helpers
that require the bus lock to be held by the caller.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 6b8df8089a357..c45b450af319d 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -2358,7 +2358,7 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
>  		if (!ret) {
>  			old_dyn_addr = newdev->info.dyn_addr;
>  			newdev->info.dyn_addr = expected_dyn_addr;
> -			i3c_master_reattach_i3c_dev(newdev, old_dyn_addr);
> +			i3c_master_reattach_i3c_dev_locked(newdev, old_dyn_addr);

[Severity: High]
This is a pre-existing issue, but does this code ignore the return value
of the reattach function on failure?

If i3c_master_reattach_i3c_dev_locked() fails (for example, if the
controller's reattach_i3c_dev callback returns an error), it internally
calls i3c_master_put_i3c_addrs(). This frees the dynamic and static address
slots by marking them as I3C_ADDR_SLOT_FREE in the master's bus address map.

Because i3c_master_add_i3c_dev_locked() completely ignores the return value
and proceeds to return 0 (success), the device remains attached and active
on the bus tracking the newly assigned dynamic address, while the bus state
considers the address slot empty.

Could this structural inconsistency lead to the master assigning the exact
same dynamic address to a new hot-joining device or during subsequent
dynamic address assignment, resulting in an address collision and bus
communication corruption?

>  		} else {
>  			dev_err(&master->dev,
>  				"Failed to assign reserved/old address to device %d%llx",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525064209.2263045-1-lakshay.piplani@nxp.com?part=1