Re: [PATCH v5 3/3] dt-bindings: misc: qcom,fastrpc: deprecate qcom,nsessions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] of: use-after-free and double put on device tree node during iteration
- [Medium] device: memory leak due to missing put_device() after of_find_device_by_node()
--

commit 0cd61e434e0b0defac9c431333bfa40bb39df800
Author: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>

misc: fastrpc: iterate CB nodes manually instead of of_platform_populate

This commit replaces of_platform_populate() with manual iteration of device
tree child nodes to ensure context banks are fully initialised before
fastrpc_rpmsg_probe() returns. It introduces fastrpc_cb_devices_create()
and fastrpc_cb_devices_destroy() to handle this synchronous setup and
teardown.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index af18ff1992eed..f20d3e5ecc81f 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
[ ... ]
> @@ -2389,37 +2389,63 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static void fastrpc_cb_remove(struct platform_device *pdev)
> +static void fastrpc_cb_devices_destroy(struct rpmsg_device *rpdev)
>  {
> -	struct fastrpc_channel_ctx *cctx = dev_get_drvdata(pdev->dev.parent);
> -	struct fastrpc_session_ctx *sess = dev_get_drvdata(&pdev->dev);
> +	struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev);
> +	struct device *rdev = &rpdev->dev;
> +	struct platform_device *pdev;
> +	struct device_node *np;
>  	unsigned long flags;
>  	int i;
>  
>  	spin_lock_irqsave(&cctx->lock, flags);
> -	for (i = 0; i < FASTRPC_MAX_SESSIONS; i++) {
> -		if (cctx->session[i].sid == sess->sid) {
> -			cctx->session[i].valid = false;
> -			cctx->sesscount--;
> +	for (i = 0; i < cctx->sesscount; i++)
> +		cctx->session[i].valid = false;
> +	spin_unlock_irqrestore(&cctx->lock, flags);
> +
> +	for_each_available_child_of_node(rdev->of_node, np) {
> +		if (!of_device_is_compatible(np, "qcom,fastrpc-compute-cb")) {
> +			of_node_put(np);
> +			continue;
>  		}

[Severity: High]
Can this result in a double-put and use-after-free on the device tree node?

The for_each_available_child_of_node() macro automatically handles dropping
the reference on the current child node at the end of each iteration via
of_get_next_available_child(). Calling of_node_put(np) manually before
continue causes the iterator to dereference np->sibling on the already-freed
node, followed by a second of_node_put(np) that underflows the reference
count.

> +
> +		pdev = of_find_device_by_node(np);
> +		of_node_put(np);

[Severity: High]
Will this manual of_node_put() lead to the same use-after-free issue?

Calling of_node_put(np) manually at the end of the loop body causes the same
issue with the loop macro underflowing the reference count and accessing the
freed node during iteration.

> +		if (pdev)
> +			of_platform_device_destroy(&pdev->dev, NULL);

[Severity: Medium]
Could this leak the pdev device reference?

of_find_device_by_node() acquires a device reference by incrementing its
refcount. This reference doesn't appear to be released via put_device(&pdev->dev)
or platform_device_put(pdev) after calling of_platform_device_destroy().

>  	}
> -	spin_unlock_irqrestore(&cctx->lock, flags);
>  }
>  
> -static const struct of_device_id fastrpc_match_table[] = {
> -	{ .compatible = "qcom,fastrpc-compute-cb" },
> -	{ }
> -};
> +static int fastrpc_cb_devices_create(struct rpmsg_device *rpdev)
> +{
> +	struct device *rdev = &rpdev->dev;
> +	struct platform_device *pdev;
> +	struct device_node *np;
> +	int err;
>  
> -static struct platform_driver fastrpc_cb_driver = {
> -	.probe = fastrpc_cb_probe,
> -	.remove = fastrpc_cb_remove,
> -	.driver = {
> -		.name = "qcom,fastrpc-cb",
> -		.of_match_table = fastrpc_match_table,
> -		.suppress_bind_attrs = true,
> -	},
> -};
> +	for_each_available_child_of_node(rdev->of_node, np) {
> +		if (!of_device_is_compatible(np, "qcom,fastrpc-compute-cb")) {
> +			of_node_put(np);
> +			continue;
> +		}

[Severity: High]
Does this early of_node_put() cause a similar double-put and use-after-free
during the loop iteration?

Similar to fastrpc_cb_devices_destroy(), calling of_node_put(np) before
continue will double-put the node and use-after-free np->sibling during
the loop iteration.

> +
> +		pdev = of_platform_device_create(np, NULL, rdev);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-dup-sessions-v5-0-e953133a1827@oss.qualcomm.com?part=3