Re: [PATCH RFC 09/12] remoteproc: search for a fwnode during vdev registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] remoteproc: fwnode handle leak in rproc_handle_vdev()
--

commit dd5d3ce5995561638c5b17c0bfbbfdf4c09d1e31
Author: Francesco Valla <francesco@valla.it>

remoteproc: search for a fwnode during vdev registration

The virtio subsystem already supports a fwnode describing a vdev; search
for a specific node during vdev registration and supply it to the
platform device that is being registered.

> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index b7d9957534bf9..e3ecc61cc1def 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -440,6 +440,29 @@ void rproc_remove_rvdev(struct rproc_vdev *rvdev)
>  	if (rvdev)
>  		list_del(&rvdev->node);
>  }
> +
> +static struct fwnode_handle *rproc_get_vdev_fwnode(struct device *dev, int idx)
> +{
> +	struct fwnode_handle *group, *child;
> +	u32 reg;
> +	int ret;
> +
> +	group = device_get_named_child_node(dev, "virtio");
> +
> +	fwnode_for_each_child_node(group, child) {
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			continue;
> +
> +		if (idx == reg)
> +			break;
> +	}
> +
> +	fwnode_handle_put(group);
> +
> +	return child;
> +}
> +

[ ... ]

> @@ -504,8 +528,16 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
>  	 * as device id, then we get duplication in sysfs, so need to use
>  	 * PLATFORM_DEVID_AUTO to auto select device id.
>  	 */
> -	pdev = platform_device_register_data(dev, "rproc-virtio", PLATFORM_DEVID_AUTO, &rvdev_data,
> -					     sizeof(rvdev_data));
> +	memset(&pdev_info, 0, sizeof(pdev_info));
> +	pdev_info.parent = dev;
> +	pdev_info.fwnode = rproc_get_vdev_fwnode(dev->parent, rvdev_data.index);
> +	pdev_info.of_node_reused = true;
> +	pdev_info.name = "rproc-virtio";
> +	pdev_info.id = PLATFORM_DEVID_AUTO;
> +	pdev_info.data = &rvdev_data;
> +	pdev_info.size_data = sizeof(rvdev_data);
> +
> +	pdev = platform_device_register_full(&pdev_info);
>  	if (IS_ERR(pdev)) {
>  		dev_err(dev, "failed to create rproc-virtio device\n");
>  		return PTR_ERR(pdev);
>  	}
>  
>  	return 0;
>  }

[Severity: Medium]
Does this code leak the fwnode handle?

rproc_get_vdev_fwnode() returns a fwnode handle with an incremented reference
count due to breaking out of the fwnode_for_each_child_node() loop.

When this handle is assigned to pdev_info.fwnode,
platform_device_register_full() takes its own independent reference to it.

Should rproc_handle_vdev() call fwnode_handle_put(pdev_info.fwnode) before
returning on both the success and error paths to drop the local reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-remoteproc_virtio_map-v1-0-dac8c5eb4aa9@valla.it?part=9