Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration

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

New issues:
- [High] Dropping the bus maintenance lock around device_register() exposes the I3C device descriptor to concurrent Use-After-Free.

Pre-existing issues:
- [High] Unbalanced fwnode reference count decrement in i3c_device_release() leads to Use-After-Free.
--

--- Patch [1]: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration ---
commit b31a8482aa2ea710ccc853d0c74b77edebb131ff
Author: Adrian Hunter <[email protected]>
Date:   Thu Aug 6 13:18:44 2026 +0000

    i3c: master: Fix recursive locking during device registration

This commit addresses a recursive locking lockdep warning by decoupling
device creation from registration. It collects newly populated devices into a
temporary list under the maintenance lock, then releases the lock prior to
invoking device_register() to prevent probe deadlocks. Finally, the lock is
re-acquired to clean up any devices that failed registration.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)

[ ... ]

> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);

[Severity: High]
Does dropping the bus maintenance lock around device_register() expose the
I3C device descriptor to a concurrent use-after-free?

During registration, the driver core can call subsystem functions like
i3c_device_match(), which read i3cdev->desc locklessly:

drivers/i3c/master.c:i3c_device_match() {
    ...
    if (i3cdev->desc && i3cdev->desc->boardinfo)
        static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
    ...
}

Concurrently, an I3C Hot-Join or sysfs-triggered Dynamic Address Assignment
(DAA) could execute i3c_master_do_daa(), take the i3c_bus_maintenance_lock,
and call __i3c_master_add_i3c_dev_locked(). If DAA detects the device as a
duplicate, it replaces the descriptor and frees the old one:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    i3c_master_detach_i3c_dev(olddev);
    i3c_master_free_i3c_dev(olddev);
    ...
}

If the registering thread is actively evaluating i3cdev->desc when this
happens, will it lead to memory corruption when reading fields like
desc->boardinfo?

> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);

[Severity: High]
This is a pre-existing issue, but does this error path cause an unbalanced
fwnode reference count decrement leading to a use-after-free?

During device initialization, device_set_node() assigns the fwnode without
incrementing its reference count:

drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
    ...
    if (desc->boardinfo)
        device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
    ...
}

However, when put_device() is called here on registration failure,
the release function unconditionally drops a reference:

drivers/i3c/master.c:i3c_device_release() {
    ...
    fwnode_handle_put(dev->fwnode);
    ...
}

Since the device object never acquired a reference to the boardinfo's fwnode,
will repeated registration failures or unbind operations drop the refcount
below zero, eventually prematurely freeing the fwnode?

> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1