Re: [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode

Adrian Hunter <[email protected]>
Newsgroups org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel
Organization Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki
Message-ID <[email protected]>
On 04/08/2026 13:12, Akhil R wrote:
> i3c_master_add_i3c_boardinfo() takes a reference on the firmware node of
> every i3c child described for the bus and never drops it, while
> i3c_device_release() drops one that the device never took, as
> i3c_master_register_new_i3c_devs() assigns the node of the boardinfo as
> is.
> 
> The two mistakes cancel out for a device that is registered exactly once,
> which is why the imbalance goes unnoticed. Register it again, as happens

I think it is (potentially repeated) registration failure path

> when the bus is re-initialised, and the count underflows, so the node may
> be freed while the boardinfo still refers to it. A child that never joins
> the bus leaks its reference instead.
> 
> Take a reference where the node is handed to the device, so that the one
> the release callback drops is balanced, and drop the reference of the
> boardinfo with a device managed action, as it is done for the i2c
> boardinfo.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Reported-by: Adrian Hunter <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <[email protected]>

Cosmetic suggestion below, nevertheless:

Reviewed-by: Adrian Hunter <[email protected]>

> ---
>  drivers/i3c/master.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 08dc10f172aa..568788e4cdb5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2102,7 +2102,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  				     desc->info.static_addr);
>  
>  		if (desc->boardinfo)
> -			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
> +			device_set_node(&desc->dev->dev,
> +					fwnode_handle_get(desc->boardinfo->fwnode));
>  
>  		ret = device_register(&desc->dev->dev);
>  		if (ret) {
> @@ -2994,7 +2995,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
>  	boardinfo->fwnode = fwnode_handle_get(fwnode);
>  	list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
>  
> -	return 0;
> +	return devm_add_action_or_reset(dev, i3c_master_put_boardinfo_fwnode,
> +					fwnode);

I'd suggest wrapping at 100 cols rather than 80.

>  }
>  
>  static int i3c_master_add_of_dev(struct i3c_master_controller *master,


-- 
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.