Re: [PATCH v3 03/13] i3c: master: Use unified device property interface

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Critical] Dangling fwnode pointer in boardinfo leading to Use-After-Free on device hot-plug.
- [High] Master registration fails on ACPI systems due to mandatory OF-specific property checks.
--

commit bc7c2a6c59d2e2a9a674fb4e8a851cbc8852b1e2
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Use unified device property interface

This commit replaces OF-specific functions with unified device property
functions to serve as a prerequisite for supporting both ACPI and device
tree.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5cd4e5da22333..d0677061faab4 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -993,7 +995,7 @@ static void i3c_device_release(struct device *dev)
>  
>  	WARN_ON(i3cdev->desc);
>  
> -	of_node_put(i3cdev->dev.of_node);
> +	fwnode_handle_put(dev->fwnode);
>  	kfree(i3cdev);
>  }
>  
[ ... ]
> @@ -1789,7 +1791,7 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  			     desc->info.pid);
>  
>  		if (desc->boardinfo)
> -			desc->dev->dev.of_node = desc->boardinfo->of_node;
> +			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);

Can this result in a use-after-free if a device is hot-unplugged and later
re-registered?

Since device_set_node() does not increment the fwnode reference count, the
device shares the reference owned by boardinfo.

During device unregistration, i3c_device_release() unconditionally calls
fwnode_handle_put(dev->fwnode), dropping that single reference.

If the device is subsequently hot-plugged, will the cached boardinfo reuse
the now-freed fwnode pointer? Would it be safer to acquire a new reference
when setting the device node, like fwnode_handle_get(desc->boardinfo->fwnode)?

[ ... ]
> @@ -2499,7 +2505,7 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
>  	if (!master)
>  		return -EINVAL;
>  
> -	ret = of_property_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
> +	ret = fwnode_property_read_u32_array(fwnode, "reg", reg, ARRAY_SIZE(reg));
>  	if (ret)
>  		return ret;

Does this cause master registration to fail on ACPI systems?

Previously, of_populate_i3c_bus() safely ignored ACPI systems because
dev->of_node was NULL. Now, fwnode_populate_i3c_bus() retrieves the ACPI
fwnode and iterates through its child devices.

For each child, this unconditionally attempts to read the OF-specific reg
property. Since ACPI defines resources via _CRS and _ADR rather than a
reg array, this read will fail and return an error.

This error then propagates up and aborts the entire i3c_master_register()
process. I noticed this regression is fixed later in the patch series by
commit e979566482d52 (i3c: master: Support ACPI enumeration of child
devices), but I am pointing it out here as it temporarily breaks
bisectability for ACPI platforms.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=3