[PATCH v7 0/3] misc: fastrpc: fix ADSP duplicate session creation

[PATCH v7 0/3] misc: fastrpc: fix ADSP duplicate session creation

[Vinayak Katoch]

For ADSP, only a limited number of FastRPC context banks (CBs) are
available. Each CB supports a single session, which means only a few
processes can run on ADSP simultaneously. If all sessions are consumed
by fastrpc daemons, no session remains available when a user application
starts, causing the application to fail.

To work around this, some DT set:
  qcom,nsessions = <5>;
which duplicated sessions inline during context bank initialisation.
Upstream feedback indicated that this policy does not belong in DT and
should be handled at the driver level instead.

This series iterates over CB child nodes directly and synchronously,
replacing of_platform_populate() and of_platform_depopulate(), and
configures DMA/IOMMU for each CB itself since no driver binds to them
any more. It also moves the ADSP session duplication to the driver.
The qcom,nsessions binding is deprecated in the same series.

Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
Changes in v7:
- Call of_dma_configure() for each CB device, as no driver binds to
  them any more
- Skip and destroy a CB whose DMA/IOMMU setup fails; defer on
  -EPROBE_DEFER
- Drop extra of_node_put() in fastrpc_cb_devices_create()
- Link to v6: https://lore.kernel.org/r/20261021-dup-sessions-v6-0-578ab52ba66c@oss.qualcomm.com

Changes in v6:
- Drop unused dev_set_drvdata() from fastrpc_cb_init()
- Use device_for_each_child_reverse() to destroy CB devices
- Return -ENODEV when of_platform_device_create() fails
- Add a comment for FASTRPC_DUP_SESSIONS
- Link to v5: https://lore.kernel.org/r/20260923-dup-sessions-v5-0-e953133a1827@oss.qualcomm.com

Changes in v5:
- Introduce fastrpc_cb_devices_create() and fastrpc_cb_devices_destroy()
  for symmetric CB device lifecycle management in probe and remove.
- Replace module_init/exit pair with module_rpmsg_driver().
- Link to v4: https://lore.kernel.org/r/20260826-dup-sessions-v4-0-35555d2bfed4@oss.qualcomm.com

Changes in v4:
- Rebased on latest linux-next (a8406e6c0b79, 2026-08-25)
- Link to v3: https://lore.kernel.org/r/20260824-dup-sessions-v3-0-4b019def4e0b@oss.qualcomm.com

Changes in v3:
- Add patch to fix async probe race with of_platform_populate().
- Rename fastrpc_cb_probe() to fastrpc_cb_init().
- Reorder series: sync fix, nsessions, binding.
- Collect Reviewed-by tags.
- Link to v2: https://lore.kernel.org/r/20260708-dup-sessions-v2-0-da40f9c98a2b@oss.qualcomm.com

Changes in v2:
- Added patch to deprecate the qcom,nsessions dt-binding.
- Kept the logic unchanged; only split into two patches.
- Link to v1: https://lore.kernel.org/r/20260609-dup-sessions-v1-1-26934abb9fa3@oss.qualcomm.com

---
Vinayak Katoch (3):
      misc: fastrpc: iterate CB nodes manually instead of of_platform_populate
      misc: fastrpc: move ADSP duplicate session creation to the driver
      dt-bindings: misc: qcom,fastrpc: deprecate qcom,nsessions

 .../devicetree/bindings/misc/qcom,fastrpc.yaml     |   2 +
 drivers/misc/fastrpc.c                             | 153 ++++++++++++---------
 2 files changed, 89 insertions(+), 66 deletions(-)
---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20260609-dup-sessions-ea2acaac1994

Best regards,
-- 
Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>

[PATCH v7 1/3] misc: fastrpc: iterate CB nodes manually instead of of_platform_populate

[Vinayak Katoch]

of_platform_populate() only guarantees that child devices are registered,
not that their probes have completed before it returns. This creates a
window where fastrpc_cb_init() may not have run for all context bank
nodes, leaving the channel context partially initialised.

Introduce fastrpc_cb_devices_create() to iterate over child DT nodes
directly and call fastrpc_cb_init() synchronously for each
qcom,fastrpc-compute-cb node. This ensures all context banks are fully
initialised before fastrpc_rpmsg_probe() returns.

Since no driver binds to the CB devices any more, the driver core no
longer runs dma_configure() for them. Call of_dma_configure() for each
CB so that dma-coherent, dma-ranges and the IOMMU group are set up
before the session is used. If a CB cannot be configured (or has
"iommus" but no IOMMU group), log it, destroy that CB device and carry
on with the remaining ones, rather than leaving a half-configured
device behind for a later IOMMU registration to trip over, or failing
the whole channel probe. -EPROBE_DEFER still fails the probe so that it
is retried once the SMMU is available.

Introduce fastrpc_cb_devices_destroy() as the symmetric counterpart.
Before destroying the CB platform devices, invalidate all sessions under
the channel lock so that any fastrpc_user still holding a reference to
the channel context cannot acquire a new session backed by a destroyed
device.

Since fastrpc_cb_driver is no longer needed as an independent platform
driver, remove it along with its match table and remove callback. Use
module_rpmsg_driver() now that only a single driver registration remains.

Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 120 +++++++++++++++++++++++++++----------------------
 1 file changed, 66 insertions(+), 54 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index af18ff1992ee..7441e9b8940d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/sort.h>
 #include <linux/of_platform.h>
@@ -2333,7 +2334,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;
@@ -2355,7 +2356,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 	spin_lock_irqsave(&cctx->lock, flags);
 	if (cctx->sesscount >= FASTRPC_MAX_SESSIONS) {
 		spin_unlock_irqrestore(&cctx->lock, flags);
-		dev_err(&pdev->dev, "too many sessions\n");
+		dev_err(dev, "too many sessions\n");
 		return -ENOSPC;
 	}
 	dma_bits = cctx->soc_data->dma_addr_bits_default;
@@ -2363,7 +2364,6 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 	sess->used = false;
 	sess->valid = true;
 	sess->dev = dev;
-	dev_set_drvdata(dev, sess);
 	sess->sid = sid;
 
 	if (cctx->domain_id == CDSP_DOMAIN_ID)
@@ -2389,37 +2389,75 @@ 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;
 	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);
+
+	device_for_each_child_reverse(rdev, NULL, of_platform_device_destroy);
 }
 
-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"))
+			continue;
+
+		pdev = of_platform_device_create(np, NULL, rdev);
+		if (!pdev) {
+			of_node_put(np);
+			fastrpc_cb_devices_destroy(rpdev);
+			return -ENODEV;
+		}
+
+		/*
+		 * No driver binds to the CB devices, so the driver core never
+		 * runs dma_configure for them. Do it here so that dma-coherent,
+		 * dma-ranges and the IOMMU link are set up before the session
+		 * is used. A CB whose IOMMU setup failed is destroyed now,
+		 * instead of being left for the next IOMMU registration to
+		 * retry (and fail on), but does not fail the whole channel:
+		 * failing the probe makes the DSP reopen the channel in a loop.
+		 */
+		err = of_dma_configure(&pdev->dev, np, true);
+		if (!err && of_property_present(np, "iommus") &&
+		    !device_iommu_mapped(&pdev->dev))
+			err = -ENODEV;
+		if (err) {
+			if (err == -EPROBE_DEFER) {
+				of_node_put(np);
+				fastrpc_cb_devices_destroy(rpdev);
+				return err;
+			}
+
+			dev_err(rdev, "skipping %pOF: failed to configure DMA/IOMMU: %d\n",
+				np, err);
+			of_platform_device_destroy(&pdev->dev, NULL);
+			continue;
+		}
+
+		err = fastrpc_cb_init(pdev);
+		if (err) {
+			of_node_put(np);
+			fastrpc_cb_devices_destroy(rpdev);
+			return err;
+		}
+	}
+
+	return 0;
+}
 
 static int fastrpc_device_register(struct device *dev, struct fastrpc_channel_ctx *cctx,
 				   bool is_secured, const char *domain)
@@ -2639,7 +2677,7 @@ 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);
+	err = fastrpc_cb_devices_create(rpdev);
 	if (err)
 		goto err_deregister_fdev;
 
@@ -2711,7 +2749,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 				&cctx->remote_heap_addr, cctx->remote_heap_size, err);
 	}
 
-	of_platform_depopulate(&rpdev->dev);
+	fastrpc_cb_devices_destroy(rpdev);
 
 	fastrpc_channel_ctx_put(cctx);
 }
@@ -2798,33 +2836,7 @@ static struct rpmsg_driver fastrpc_driver = {
 	},
 };
 
-static int fastrpc_init(void)
-{
-	int ret;
-
-	ret = platform_driver_register(&fastrpc_cb_driver);
-	if (ret < 0) {
-		pr_err("fastrpc: failed to register cb driver\n");
-		return ret;
-	}
-
-	ret = register_rpmsg_driver(&fastrpc_driver);
-	if (ret < 0) {
-		pr_err("fastrpc: failed to register rpmsg driver\n");
-		platform_driver_unregister(&fastrpc_cb_driver);
-		return ret;
-	}
-
-	return 0;
-}
-module_init(fastrpc_init);
-
-static void fastrpc_exit(void)
-{
-	platform_driver_unregister(&fastrpc_cb_driver);
-	unregister_rpmsg_driver(&fastrpc_driver);
-}
-module_exit(fastrpc_exit);
+module_rpmsg_driver(fastrpc_driver);
 
 MODULE_DESCRIPTION("Qualcomm FastRPC");
 MODULE_LICENSE("GPL v2");

-- 
2.34.1

Re: [PATCH v7 1/3] misc: fastrpc: iterate CB nodes manually instead of of_platform_populate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] misc: fastrpc: missing SCM memory and pointer cleanup in fastrpc_rpmsg_probe()
- [High] misc: fastrpc: premature fastrpc_device_register() in fastrpc_rpmsg_probe()
--

commit 3d1d4146f1c73d49d10eb5163573a4b2ed34670c
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() and the separate
fastrpc_cb_driver platform driver with manual iteration over child DT nodes
in fastrpc_cb_devices_create() so that context banks are synchronously
configured and initialized before fastrpc_rpmsg_probe() returns. It also
adds fastrpc_cb_devices_destroy() to invalidate sessions and destroy the
child platform devices on teardown.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index af18ff1992eed..7441e9b8940d8 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c

[ ... ]

> +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;
>  
> +	for_each_available_child_of_node(rdev->of_node, np) {
> +		if (!of_device_is_compatible(np, "qcom,fastrpc-compute-cb"))
> +			continue;
> +
> +		pdev = of_platform_device_create(np, NULL, rdev);
> +		if (!pdev) {
> +			of_node_put(np);
> +			fastrpc_cb_devices_destroy(rpdev);
> +			return -ENODEV;
> +		}
> +
> +		/*
> +		 * No driver binds to the CB devices, so the driver core never
> +		 * runs dma_configure for them. Do it here so that dma-coherent,
> +		 * dma-ranges and the IOMMU link are set up before the session
> +		 * is used. A CB whose IOMMU setup failed is destroyed now,
> +		 * instead of being left for the next IOMMU registration to
> +		 * retry (and fail on), but does not fail the whole channel:
> +		 * failing the probe makes the DSP reopen the channel in a loop.
> +		 */
> +		err = of_dma_configure(&pdev->dev, np, true);
> +		if (!err && of_property_present(np, "iommus") &&
> +		    !device_iommu_mapped(&pdev->dev))
> +			err = -ENODEV;
> +		if (err) {
> +			if (err == -EPROBE_DEFER) {
> +				of_node_put(np);
> +				fastrpc_cb_devices_destroy(rpdev);
> +				return err;
> +			}
> +
> +			dev_err(rdev, "skipping %pOF: failed to configure DMA/IOMMU: %d\n",
> +				np, err);
> +			of_platform_device_destroy(&pdev->dev, NULL);
> +			continue;
> +		}
> +
> +		err = fastrpc_cb_init(pdev);
> +		if (err) {
> +			of_node_put(np);
> +			fastrpc_cb_devices_destroy(rpdev);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
>  
>  static int fastrpc_device_register(struct device *dev, struct fastrpc_channel_ctx *cctx,
>  				   bool is_secured, const char *domain)

[Severity: High]
As reported by Sashiko on v2 [1], fastrpc_rpmsg_probe() calls
fastrpc_device_register() -> misc_register() before initializing the
channel context fields and before calling fastrpc_cb_devices_create():

fastrpc_rpmsg_probe()
    fastrpc_device_register()
        misc_register(&fdev->miscdev);
    ...
    kref_init(&data->refcount);
    ...
    INIT_LIST_HEAD(&data->users);
    spin_lock_init(&data->lock);
    idr_init(&data->ctx_idr);
    data->domain_id = domain_id;
    data->rpdev = rpdev;
    dev_set_drvdata(&rpdev->dev, data);

    err = fastrpc_cb_devices_create(rpdev);
    if (err)
        goto err_deregister_fdev;

If userspace opens /dev/fastrpc-* immediately after misc_register(),
fastrpc_device_open() runs on an uninitialized spinlock and kref, can
dereference a null cctx->rpdev in dev_err_ratelimited(&cctx->rpdev->dev, ...),
and still sees zero or only partially populated sessions while
fastrpc_cb_devices_create() iterates over the child nodes (additionally,
fastrpc_cb_init() sets sess->valid = true under cctx->lock before checking
dma_set_mask()).

Furthermore, with this patch, if fastrpc_cb_devices_create() initializes
the first context bank and a concurrent fastrpc_device_open() acquires that
session, and then a subsequent child node returns -EPROBE_DEFER from
of_dma_configure() or fails in of_platform_device_create() or
fastrpc_cb_init(), fastrpc_cb_devices_create() calls
fastrpc_cb_devices_destroy(rpdev) and fastrpc_rpmsg_probe() jumps to
err_deregister_fdev:

err_deregister_fdev:
    if (data->fdevice)
        misc_deregister(&data->fdevice->miscdev);
    if (data->secure_fdevice)
        misc_deregister(&data->secure_fdevice->miscdev);

err_free_data:
    kfree(data);

Can this destroy the child platform device backing fl->sctx->dev and
unconditionally kfree(data) while the open file descriptor still holds
active references to fl->cctx and fl->sctx?

> @@ -2639,7 +2677,7 @@ 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);
> +	err = fastrpc_cb_devices_create(rpdev);
>  	if (err)
>  		goto err_deregister_fdev;

[Severity: High]
Previously, of_platform_populate() only registered child devices and returned
0 even when child context bank probes deferred waiting for the SMMU. Now
fastrpc_cb_devices_create() synchronously calls of_dma_configure() and
fastrpc_cb_init() and returns -EPROBE_DEFER (or another error) to
fastrpc_rpmsg_probe(), jumping to err_deregister_fdev.

Earlier in fastrpc_rpmsg_probe(), fastrpc_init_reserved_mem() transfers
ownership of the reserved memory region for ADSP_DOMAIN_ID and SDSP_DOMAIN_ID
away from HLOS to the remote DSP VMIDs:

fastrpc_init_reserved_mem()
    if (domain_id == ADSP_DOMAIN_ID) {
        cctx->remote_heap_addr = res.start;
        cctx->remote_heap_size = resource_size(&res);
    }
    if (!cctx->vmcount)
        return 0;
    src_perms = BIT(QCOM_SCM_VMID_HLOS);
    return qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
                               cctx->vmperms, cctx->vmcount);

And before calling fastrpc_cb_devices_create(), fastrpc_rpmsg_probe() also
sets up:

    rdev->dma_mask = &data->dma_mask;
    ...
    idr_init(&data->ctx_idr);
    ...
    dev_set_drvdata(&rpdev->dev, data);

When fastrpc_cb_devices_create() fails or returns -EPROBE_DEFER,
err_deregister_fdev frees data without reassigning the reserved memory back
to QCOM_SCM_VMID_HLOS, without destroying data->ctx_idr, and without clearing
rpdev->dev.driver_data or rdev->dma_mask.

When the deferred probe is retried later, won't fastrpc_init_reserved_mem()
fail permanently in qcom_scm_assign_mem() because QCOM_SCM_VMID_HLOS no
longer owns the memory region, while also leaving dangling rdev->dma_mask and
drvdata pointers into freed memory (where an incoming rpmsg callback before
rpmsg_destroy_ept() in rpmsg_dev_probe() could dereference cctx in
fastrpc_rpmsg_callback())?

[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=1

[PATCH v7 2/3] misc: fastrpc: move ADSP duplicate session creation to the driver

[Vinayak Katoch]

For ADSP, only a limited number of FastRPC context banks (CBs) are
available. Each CB supports a single session, which means only a few
processes can run on ADSP simultaneously. If all sessions are consumed
by fastrpc daemons, no session remains available when a user application
starts, causing the application to fail.

To work around this, qcom,nsessions = <5> was set in DT to duplicate
sessions inline during fastrpc_cb_init(). This policy does not belong
in DT and should be handled at the driver level instead.

Remove the qcom,nsessions DT property read and the per-CB duplication
logic from fastrpc_cb_init(). After all context banks have been
initialised in fastrpc_rpmsg_probe(), append FASTRPC_DUP_SESSIONS (4)
copies of the last successfully initialised session for the ADSP
domain.

Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7441e9b8940d..b4be0ee2bc18 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -35,6 +35,11 @@
 #define CDSP_DOMAIN_ID (3)
 #define GDSP_DOMAIN_ID (4)
 #define FASTRPC_MAX_SESSIONS	14
+/*
+ * Extra ADSP sessions duplicated from the last CB, as each CB supports
+ * only one session.
+ */
+#define FASTRPC_DUP_SESSIONS	4
 #define FASTRPC_MAX_VMIDS	16
 #define FASTRPC_ALIGN		128
 #define FASTRPC_MAX_FDLIST	16
@@ -2339,7 +2344,6 @@ static int fastrpc_cb_init(struct platform_device *pdev)
 	struct fastrpc_channel_ctx *cctx;
 	struct fastrpc_session_ctx *sess;
 	struct device *dev = &pdev->dev;
-	int i, sessions = 0;
 	unsigned long flags;
 	u32 dma_bits;
 	u32 sid = 0;
@@ -2349,7 +2353,6 @@ static int fastrpc_cb_init(struct platform_device *pdev)
 	if (!cctx)
 		return -EINVAL;
 
-	of_property_read_u32(dev->of_node, "qcom,nsessions", &sessions);
 	if (of_property_read_u32(dev->of_node, "reg", &sid))
 		dev_info(dev, "FastRPC Session ID not specified in DT\n");
 
@@ -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);
+	}
+
 	return 0;
 
 err_deregister_fdev:

-- 
2.34.1

Re: [PATCH v7 2/3] misc: fastrpc: move ADSP duplicate session creation to the driver

[sashiko-bot]

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

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

[Vinayak Katoch]

The qcom,nsessions property was used to duplicate FastRPC sessions
inline during context bank initialisation. Session duplication is now
handled at the driver level, making this DT property redundant. Mark
it deprecated.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
index 49d2a5e8ba39..e945c8ba75e3 100644
--- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
+++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
@@ -101,8 +101,10 @@ patternProperties:
       qcom,nsessions:
         $ref: /schemas/types.yaml#/definitions/uint32
         default: 1
+        deprecated: true
         description: >
           A value indicating how many sessions can share this context bank.
+          Session duplication is now handled by the driver.
 
     required:
       - compatible

-- 
2.34.1