Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Vinayak Katoch" <vinayak.katoch@oss.qualcomm.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4 3/3] dt-bindings: misc: qcom,fastrpc: deprecate qcom,nsessions
Date: Wed, 26 Aug 2026 13:40:33 +0000	[thread overview]
Message-ID: <20260826134033.A55A41F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260826-dup-sessions-v4-3-35555d2bfed4@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Child context bank devices are never attached to their IOMMU domains, breaking DMA mapping and causing SMMU faults.

Pre-existing issues:
- [High] Unconditional `kfree(data)` on the probe error path causes a Use-After-Free if userspace opened the device.
--

commit e928c95aa5ca7ef57a0d4943351ae2891fcf2b1c
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 over child
device tree nodes to initialize qcom,fastrpc-compute-cb devices synchronously.
It aims to ensure all context banks are fully initialized before
fastrpc_rpmsg_probe() returns by directly calling fastrpc_cb_init().

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d4fac2caca867..a153107d00853 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2363,7 +2363,7 @@ static const struct file_operations fastrpc_fops = {
>  	.compat_ioctl = fastrpc_device_ioctl,
>  };
>  
> -static int fastrpc_cb_probe(struct platform_device *pdev)
> +static int fastrpc_cb_init(struct platform_device *pdev)
>  {
>  	struct fastrpc_channel_ctx *cctx;
>  	struct fastrpc_session_ctx *sess;

[ ... ]

> @@ -2642,12 +2610,29 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	data->rpdev = rpdev;
>  	dev_set_drvdata(&rpdev->dev, data);
>  
> -	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
> -	if (err)
> -		goto err_deregister_fdev;
> +	of_node_set_flag(rdev->of_node, OF_POPULATED_BUS);
> +
> +	for_each_available_child_of_node_scoped(rdev->of_node, np) {
> +		struct platform_device *pdev;
> +
> +		if (!of_device_is_compatible(np, "qcom,fastrpc-compute-cb"))
> +			continue;
> +
> +		pdev = of_platform_device_create(np, NULL, rdev);
> +		if (!pdev) {
> +			err = -EINVAL;
> +			goto err_depopulate;
> +		}
> +
> +		err = fastrpc_cb_init(pdev);

[Severity: High]
By replacing driver core matching with manual of_platform_device_create()
followed by fastrpc_cb_init() in fastrpc_rpmsg_probe(), does this child
device skip the driver core's platform probe path?

It appears this manual initialization skips IOMMU and DMA configuration
logic, such as of_dma_configure(), which normally runs during probe.

Since these context bank devices are no longer bound to a platform driver,
they are never attached to their SMMU domains. Could this result in SMMU
translation faults and a failure of DSP context bank DMA operations when a
DSP invocation attempts DMA?

> +		if (err)
> +			goto err_depopulate;
> +	}
>  
>  	return 0;
>  
> +err_depopulate:
> +	of_platform_depopulate(rdev);
>  err_deregister_fdev:
>  	if (data->fdevice)
>  		misc_deregister(&data->fdevice->miscdev);

[Severity: High]
This is a pre-existing issue, but if fastrpc_cb_init() fails, this error path
jumps to err_depopulate and eventually reaches err_free_data in
fastrpc_rpmsg_probe():

err_free_data:
	kfree(data);
	return err;

Since fastrpc_device_register() makes the misc device available to userspace
earlier in the probe, what happens if userspace has already called open()
and incremented the data reference count via fastrpc_channel_ctx_get()?

Does this unconditional kfree(data) ignore active references held by open
file descriptors, leading to a use-after-free condition if userspace later
performs an ioctl() on the still-open file descriptor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-dup-sessions-v4-0-35555d2bfed4@oss.qualcomm.com?part=3

      reply	other threads:[~2026-08-26 13:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26 13:25 [PATCH v4 0/3] misc: fastrpc: fix ADSP duplicate session creation Vinayak Katoch
2026-08-26 13:25 ` [PATCH v4 1/3] misc: fastrpc: iterate CB nodes manually instead of of_platform_populate Vinayak Katoch
2026-08-26 13:45   ` sashiko-bot
2026-08-26 13:25 ` [PATCH v4 2/3] misc: fastrpc: move ADSP duplicate session creation to the driver Vinayak Katoch
2026-08-26 13:40   ` sashiko-bot
2026-08-26 13:25 ` [PATCH v4 3/3] dt-bindings: misc: qcom,fastrpc: deprecate qcom,nsessions Vinayak Katoch
2026-08-26 13:40   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260826134033.A55A41F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vinayak.katoch@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox