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
prev parent 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