From: sashiko-bot@kernel.org
To: "Vinayak Katoch" <vinayak.katoch@oss.qualcomm.com>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v7 2/3] misc: fastrpc: move ADSP duplicate session creation to the driver
Date: Mon, 02 Nov 2026 09:06:06 +0000 [thread overview]
Message-ID: <sashiko-outbox-193153@kernel.org> (raw)
In-Reply-To: <20261102-dup-sessions-v7-2-03f06c014e78@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] misc: fastrpc: session leak in fastrpc_rpmsg_probe() on race with open
--
commit 51e0d53d78d8245f1572af01da3a96d0f550fc6f
Author: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
misc: fastrpc: move ADSP duplicate session creation to the driver
This commit removes the qcom,nsessions device tree property read and per-CB
session duplication from fastrpc_cb_init(), and instead appends
FASTRPC_DUP_SESSIONS copies of the last initialized session in
fastrpc_rpmsg_probe() for the ADSP domain.
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7441e9b8940d8..b4be0ee2bc181 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
[ ... ]
> @@ -2369,16 +2372,6 @@ static int fastrpc_cb_init(struct platform_device *pdev)
> if (cctx->domain_id == CDSP_DOMAIN_ID)
> dma_bits = cctx->soc_data->dma_addr_bits_cdsp;
>
> - if (sessions > 0) {
> - struct fastrpc_session_ctx *dup_sess;
> -
> - for (i = 1; i < sessions; i++) {
> - if (cctx->sesscount >= FASTRPC_MAX_SESSIONS)
> - break;
> - dup_sess = &cctx->session[cctx->sesscount++];
> - memcpy(dup_sess, sess, sizeof(*dup_sess));
> - }
> - }
> spin_unlock_irqrestore(&cctx->lock, flags);
> rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
> if (rc) {
> @@ -2681,6 +2674,22 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> if (err)
> goto err_deregister_fdev;
>
> + if (data->domain_id == ADSP_DOMAIN_ID && data->sesscount > 0) {
> + struct fastrpc_session_ctx *last_sess;
> + struct fastrpc_session_ctx *dup_sess;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data->lock, flags);
> + last_sess = &data->session[data->sesscount - 1];
> + for (i = 0; i < FASTRPC_DUP_SESSIONS; i++) {
> + if (data->sesscount >= FASTRPC_MAX_SESSIONS)
> + break;
> + dup_sess = &data->session[data->sesscount++];
> + memcpy(dup_sess, last_sess, sizeof(*dup_sess));
> + }
> + spin_unlock_irqrestore(&data->lock, flags);
> + }
[Severity: High]
As reported by Sashiko on v2 [1], could a concurrent open() claim last_sess
between fastrpc_cb_init() dropping cctx->lock and fastrpc_rpmsg_probe()
duplicating the session, permanently leaking all 4 dup_sess entries?
In fastrpc_rpmsg_probe(), fastrpc_device_register() calls misc_register()
before fastrpc_cb_devices_create() and before the ADSP duplicate session
block:
drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
...
switch (domain_id) {
case ADSP_DOMAIN_ID:
...
err = fastrpc_device_register(rdev, data, secure_dsp, domain);
...
err = fastrpc_cb_devices_create(rpdev);
}
Previously, fastrpc_cb_init() duplicated the sessions inside the same
cctx->lock critical section that initialized sess->used = false. With this
change, fastrpc_cb_init() sets sess->valid = true, increments
cctx->sesscount, and drops cctx->lock before fastrpc_rpmsg_probe() re-acquires
data->lock to copy last_sess.
If userspace daemons open the character device node during this window,
fastrpc_device_open() -> fastrpc_session_alloc() can claim last_sess and set
last_sess->used = true (or fail with -EBUSY if all raw CB sessions are
consumed before the duplicate sessions are appended):
drivers/misc/fastrpc.c:fastrpc_session_alloc() {
...
for (i = 0; i < cctx->sesscount; i++) {
if (!cctx->session[i].used && cctx->session[i].valid) {
cctx->session[i].used = true;
session = &cctx->session[i];
...
break;
}
}
}
When fastrpc_rpmsg_probe() then runs memcpy(dup_sess, last_sess,
sizeof(*dup_sess)), used = true is copied into all 4 dup_sess slots without
resetting dup_sess->used = false. Because no fastrpc_user instance owns those
duplicated slots, fastrpc_session_free() will never be called on them,
leaving all 4 extra ADSP sessions permanently unavailable. Also,
data->sesscount is checked outside data->lock before entering the block.
[1] https://lore.kernel.org/r/20260708041312.ACF681F000E9@smtp.kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-dup-sessions-v7-0-03f06c014e78@oss.qualcomm.com?part=2
next prev parent reply other threads:[~2026-11-02 9:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-02 8:31 [PATCH v7 0/3] misc: fastrpc: fix ADSP duplicate session creation Vinayak Katoch
2026-11-02 8:31 ` [PATCH v7 1/3] misc: fastrpc: iterate CB nodes manually instead of of_platform_populate Vinayak Katoch
2026-11-02 8:54 ` sashiko-bot
2026-11-02 8:31 ` [PATCH v7 2/3] misc: fastrpc: move ADSP duplicate session creation to the driver Vinayak Katoch
2026-11-02 9:06 ` sashiko-bot [this message]
2026-11-02 8:31 ` [PATCH v7 3/3] dt-bindings: misc: qcom,fastrpc: deprecate qcom,nsessions Vinayak Katoch
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=sashiko-outbox-193153@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.