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

[email protected]
Newsgroups org.infradead.lists.linux-i3c,org.kernel.vger.linux-pci
Message-ID <[email protected]>
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

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.