mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes
@ 2026-06-02  7:17 Jianping Li
  2026-06-02  7:17 ` [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool Jianping Li
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Jianping Li, linux-arm-msm, dri-devel, linux-kernel,
	ekansh.gupta, quic_chennak

Add missing bug fixes in memory areas. This patch series fixes multiple memory
handling issues in the FastRPC driver, primarily around the Audio PD remote heap.

The Audio PD uses a reserved memory-region that is shared between HLOS
and the DSP. Allocating and freeing this memory from userspace is unsafe,
as the kernel cannot reliably determine when the DSP has finished using
the buffers.

To address this, the entire reserved memory-region for the Audio PD is
now fully assigned to the DSP during remoteproc boot-up, and its lifetime
is tied to the rpmsg channel.

Patch [v6]: https://lore.kernel.org/all/20260526111124.515-2-jianping.li@oss.qualcomm.com/

Change in v7:
  - Removed duplicate remote heap validation check; keep it only at
    the beginning of fastrpc_init_create_static_process()

Change in v6:
  - Separate the handling of err_copy
  - Place the check for remote_heap at the beginning of the function

Change in v5:
  - Split reserved-memory handling into separate patches

Change in v4:
  - Fail Audio PD static process creation when no reserved memory-region
    is present, instead of silently proceeding

Change in v3:
  - Adjusted the order of the series, placing NULL check changes that are not bug fixes at the end
  - Modified the commit message to describe the bug background in detail
  - Switch buf->list_lock back to fl->lock
  - Add locking to the operation of audio_init_mem

Changes in v2:
  - Remove the if check outside fastrpc_buf_free
  - Store the spinlock pointer in the struct fastrpc_buf instead
  - Allocate entire reserved memory to audio PD through remote heap

Ekansh Gupta (3):
  misc: fastrpc: Fix initial memory allocation for Audio PD memory pool
  misc: fastrpc: Remove buffer from list prior to unmap operation
  misc: fastrpc: Allow fastrpc_buf_free() to accept NULL

Jianping Li (2):
  misc: fastrpc: Fail Audio PD init when reserved memory is missing
  misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

 drivers/misc/fastrpc.c | 128 +++++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 62 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool
  2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
@ 2026-06-02  7:17 ` Jianping Li
  2026-06-08  6:06   ` Dmitry Baryshkov
  2026-06-02  7:17 ` [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation Jianping Li
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Ekansh Gupta, linux-arm-msm, dri-devel, linux-kernel,
	quic_chennak, stable, Jianping Li

From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

The initial buffer allocated for the Audio PD memory pool is never added
to the pool because pageslen is set to 0. As a result, the buffer is not
registered with Audio PD and is never used, causing a memory leak. Audio
PD immediately falls back to allocating memory from the remote heap since
the pool starts out empty.

Fix this by setting pageslen to 1 so that the initially allocated buffer
is correctly registered and becomes part of the Audio PD memory pool.

Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
Cc: stable@kernel.org
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 5524354f3ccc..80a636962357 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1373,7 +1373,9 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 		err = PTR_ERR(name);
 		goto err;
 	}
-
+	inbuf.client_id = fl->client_id;
+	inbuf.namelen = init.namelen;
+	inbuf.pageslen = 0;
 	if (!fl->cctx->remote_heap) {
 		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
 						&fl->cctx->remote_heap);
@@ -1396,12 +1398,10 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 				goto err_map;
 			}
 			scm_done = true;
+			inbuf.pageslen = 1;
 		}
 	}
 
-	inbuf.client_id = fl->client_id;
-	inbuf.namelen = init.namelen;
-	inbuf.pageslen = 0;
 	fl->pd = USER_PD;
 
 	args[0].ptr = (u64)(uintptr_t)&inbuf;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation
  2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
  2026-06-02  7:17 ` [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool Jianping Li
@ 2026-06-02  7:17 ` Jianping Li
  2026-06-08  6:06   ` Dmitry Baryshkov
  2026-06-02  7:17 ` [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing Jianping Li
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Ekansh Gupta, linux-arm-msm, dri-devel, linux-kernel,
	quic_chennak, stable, Jianping Li

From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

fastrpc_req_munmap_impl() is called to unmap any buffer. The buffer is
getting removed from the list after it is unmapped from DSP. This can
create potential race conditions if multiple threads invoke unmap
concurrently, where one thread may remove the entry from the list while
another thread's unmap operation is still ongoing.

Fix this by removing the buffer entry from the list before calling the
unmap operation. If the unmap fails, the entry is re-added to the list
so that userspace can retry the unmap, or alternatively, the buffer
will be cleaned up during device release when the DSP process is torn
down and all DSP-side mappings are freed along with remaining buffers
in the list.

Fixes: 2419e55e532de ("misc: fastrpc: add mmap/unmap support")
Cc: stable@kernel.org
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 80a636962357..a8a58f889d07 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1892,9 +1892,6 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
 				      &args[0]);
 	if (!err) {
 		dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr);
-		spin_lock(&fl->lock);
-		list_del(&buf->node);
-		spin_unlock(&fl->lock);
 		fastrpc_buf_free(buf);
 	} else {
 		dev_err(dev, "unmmap\tpt 0x%09lx ERROR\n", buf->raddr);
@@ -1908,6 +1905,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	struct fastrpc_buf *buf = NULL, *iter, *b;
 	struct fastrpc_req_munmap req;
 	struct device *dev = fl->sctx->dev;
+	int err;
 
 	if (copy_from_user(&req, argp, sizeof(req)))
 		return -EFAULT;
@@ -1915,6 +1913,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	spin_lock(&fl->lock);
 	list_for_each_entry_safe(iter, b, &fl->mmaps, node) {
 		if ((iter->raddr == req.vaddrout) && (iter->size == req.size)) {
+			list_del(&iter->node);
 			buf = iter;
 			break;
 		}
@@ -1927,7 +1926,14 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 		return -EINVAL;
 	}
 
-	return fastrpc_req_munmap_impl(fl, buf);
+	err = fastrpc_req_munmap_impl(fl, buf);
+	if (err) {
+		spin_lock(&fl->lock);
+		list_add_tail(&buf->node, &fl->mmaps);
+		spin_unlock(&fl->lock);
+	}
+
+	return err;
 }
 
 static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
  2026-06-02  7:17 ` [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool Jianping Li
  2026-06-02  7:17 ` [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation Jianping Li
@ 2026-06-02  7:17 ` Jianping Li
  2026-06-02  9:25   ` Jie Gan
  2026-06-08  6:11   ` Dmitry Baryshkov
  2026-06-02  7:17 ` [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
  2026-06-02  7:17 ` [PATCH v7 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL Jianping Li
  4 siblings, 2 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Jianping Li, linux-arm-msm, dri-devel, linux-kernel,
	ekansh.gupta, quic_chennak, stable

Audio PD static process creation assumes that a reserved-memory
region is defined in DT and exposed via cctx->remote_heap.

If reserved-memory is missing or incomplete, the driver may pass
invalid address/size information to the DSP, leading to undefined
behavior or crashes.

Add explicit validation for remote_heap presence and size before
sending the memory to DSP, and fail early if the configuration is
invalid.

Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
Cc: stable@kernel.org
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a8a58f889d07..f46a8f53970d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	} inbuf;
 	u32 sc;
 
+	if (!fl->cctx->remote_heap ||
+	    !fl->cctx->remote_heap->dma_addr ||
+	    !fl->cctx->remote_heap->size) {
+		err = -ENOMEM;
+		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");
+		return err;
+	}
 	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
 	if (!args)
 		return -ENOMEM;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
  2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
                   ` (2 preceding siblings ...)
  2026-06-02  7:17 ` [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing Jianping Li
@ 2026-06-02  7:17 ` Jianping Li
  2026-06-02  9:41   ` Jie Gan
  2026-06-02  7:17 ` [PATCH v7 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL Jianping Li
  4 siblings, 1 reply; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Jianping Li, linux-arm-msm, dri-devel, linux-kernel,
	ekansh.gupta, quic_chennak, stable

Allocating and freeing Audio PD memory from userspace is unsafe because
the kernel cannot reliably determine when the DSP has finished using the
memory. Userspace may free buffers while they are still in use by the DSP,
and remote free requests cannot be safely trusted.

Additionally, the current implementation allows userspace to repeatedly
grow the Audio PD heap, but does not support shrinking it. This can lead
to unbounded memory usage over time, effectively causing a memory leak.

Fix this by allocating the entire Audio PD reserved-memory region during
rpmsg probe and tying its lifetime to the rpmsg channel. This removes
userspace-controlled alloc/free and ensures that memory is reclaimed only
when the DSP process is torn down.

Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
Cc: stable@kernel.org
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 96 +++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 53 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index f46a8f53970d..33be8bed6a0b 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -276,6 +276,8 @@ struct fastrpc_channel_ctx {
 	struct kref refcount;
 	/* Flag if dsp attributes are cached */
 	bool valid_attributes;
+	/* Flag if audio PD init mem was allocated */
+	bool audio_init_mem;
 	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
 	struct fastrpc_device *secure_fdevice;
 	struct fastrpc_device *fdevice;
@@ -1344,15 +1346,16 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	struct fastrpc_init_create_static init;
 	struct fastrpc_invoke_args *args;
 	struct fastrpc_phy_page pages[1];
+	struct fastrpc_channel_ctx *cctx = fl->cctx;
 	char *name;
 	int err;
-	bool scm_done = false;
 	struct {
 		int client_id;
 		u32 namelen;
 		u32 pageslen;
 	} inbuf;
 	u32 sc;
+	unsigned long flags;
 
 	if (!fl->cctx->remote_heap ||
 	    !fl->cctx->remote_heap->dma_addr ||
@@ -1383,31 +1386,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	inbuf.client_id = fl->client_id;
 	inbuf.namelen = init.namelen;
 	inbuf.pageslen = 0;
-	if (!fl->cctx->remote_heap) {
-		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
-						&fl->cctx->remote_heap);
-		if (err)
-			goto err_name;
-
-		/* Map if we have any heap VMIDs associated with this ADSP Static Process. */
-		if (fl->cctx->vmcount) {
-			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
-
-			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
-							(u64)fl->cctx->remote_heap->size,
-							&src_perms,
-							fl->cctx->vmperms, fl->cctx->vmcount);
-			if (err) {
-				dev_err(fl->sctx->dev,
-					"Failed to assign memory with dma_addr %pad size 0x%llx err %d\n",
-					&fl->cctx->remote_heap->dma_addr,
-					fl->cctx->remote_heap->size, err);
-				goto err_map;
-			}
-			scm_done = true;
-			inbuf.pageslen = 1;
-		}
-	}
 
 	fl->pd = USER_PD;
 
@@ -1419,8 +1397,17 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	args[1].length = inbuf.namelen;
 	args[1].fd = -1;
 
-	pages[0].addr = fl->cctx->remote_heap->dma_addr;
-	pages[0].size = fl->cctx->remote_heap->size;
+	spin_lock_irqsave(&cctx->lock, flags);
+	if (!fl->cctx->audio_init_mem) {
+		pages[0].addr = fl->cctx->remote_heap->dma_addr;
+		pages[0].size = fl->cctx->remote_heap->size;
+		fl->cctx->audio_init_mem = true;
+		inbuf.pageslen = 1;
+	} else {
+		pages[0].addr = 0;
+		pages[0].size = 0;
+	}
+	spin_unlock_irqrestore(&cctx->lock, flags);
 
 	args[2].ptr = (u64)(uintptr_t) pages;
 	args[2].length = sizeof(*pages);
@@ -1438,27 +1425,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 
 	return 0;
 err_invoke:
-	if (fl->cctx->vmcount && scm_done) {
-		u64 src_perms = 0;
-		struct qcom_scm_vmperm dst_perms;
-		u32 i;
-
-		for (i = 0; i < fl->cctx->vmcount; i++)
-			src_perms |= BIT(fl->cctx->vmperms[i].vmid);
-
-		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
-		dst_perms.perm = QCOM_SCM_PERM_RWX;
-		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
-						(u64)fl->cctx->remote_heap->size,
-						&src_perms, &dst_perms, 1);
-		if (err)
-			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
-				&fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
-	}
-err_map:
-	fastrpc_buf_free(fl->cctx->remote_heap);
-	fl->cctx->remote_heap = NULL;
-err_name:
+	fl->cctx->audio_init_mem = false;
 	kfree(name);
 err:
 	kfree(args);
@@ -2425,12 +2392,21 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		}
 	}
 
-	if (domain_id == SDSP_DOMAIN_ID) {
+	if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
 		struct resource res;
 		u64 src_perms;
 
 		err = of_reserved_mem_region_to_resource(rdev->of_node, 0, &res);
 		if (!err) {
+			if (domain_id == ADSP_DOMAIN_ID) {
+				data->remote_heap =
+					kzalloc_obj(*data->remote_heap, GFP_KERNEL);
+				if (!data->remote_heap)
+					return -ENOMEM;
+
+				data->remote_heap->dma_addr = res.start;
+				data->remote_heap->size = resource_size(&res);
+			}
 			src_perms = BIT(QCOM_SCM_VMID_HLOS);
 
 			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
@@ -2438,7 +2414,6 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 			if (err)
 				goto err_free_data;
 		}
-
 	}
 
 	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
@@ -2519,6 +2494,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	struct fastrpc_buf *buf, *b;
 	struct fastrpc_user *user;
 	unsigned long flags;
+	int err;
 
 	/* No invocations past this point */
 	spin_lock_irqsave(&cctx->lock, flags);
@@ -2536,8 +2512,22 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
 		list_del(&buf->node);
 
-	if (cctx->remote_heap)
-		fastrpc_buf_free(cctx->remote_heap);
+	if (cctx->remote_heap && cctx->vmcount) {
+		u64 src_perms = 0;
+		struct qcom_scm_vmperm dst_perms;
+
+		for (u32 i = 0; i < cctx->vmcount; i++)
+			src_perms |= BIT(cctx->vmperms[i].vmid);
+
+		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
+		dst_perms.perm = QCOM_SCM_PERM_RWX;
+
+		err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
+					  cctx->remote_heap->size, &src_perms,
+					  &dst_perms, 1);
+		if (!err)
+			kfree(cctx->remote_heap);
+	}
 
 	of_platform_depopulate(&rpdev->dev);
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL
  2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
                   ` (3 preceding siblings ...)
  2026-06-02  7:17 ` [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
@ 2026-06-02  7:17 ` Jianping Li
  4 siblings, 0 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-02  7:17 UTC (permalink / raw)
  To: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: Ekansh Gupta, linux-arm-msm, dri-devel, linux-kernel,
	quic_chennak, Dmitry Baryshkov, Jianping Li

From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

Make fastrpc_buf_free() a no-op when passed a NULL pointer, allowing
callers to avoid open-coded NULL checks.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 33be8bed6a0b..370b4f9ca135 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -418,6 +418,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
 
 static void fastrpc_buf_free(struct fastrpc_buf *buf)
 {
+	if (!buf)
+		return;
+
 	dma_free_coherent(buf->dev, buf->size, buf->virt,
 			  fastrpc_ipa_to_dma_addr(buf->fl->cctx, buf->dma_addr));
 	kfree(buf);
@@ -510,8 +513,7 @@ static void fastrpc_user_free(struct kref *ref)
 	struct fastrpc_map *map, *m;
 	struct fastrpc_buf *buf, *b;
 
-	if (fl->init_mem)
-		fastrpc_buf_free(fl->init_mem);
+	fastrpc_buf_free(fl->init_mem);
 
 	list_for_each_entry_safe(ctx, n, &fl->pending, node) {
 		list_del(&ctx->node);
@@ -556,8 +558,7 @@ static void fastrpc_context_free(struct kref *ref)
 	for (i = 0; i < ctx->nbufs; i++)
 		fastrpc_map_put(ctx->maps[i]);
 
-	if (ctx->buf)
-		fastrpc_buf_free(ctx->buf);
+	fastrpc_buf_free(ctx->buf);
 
 	spin_lock_irqsave(&cctx->lock, flags);
 	idr_remove(&cctx->ctx_idr, ctx->ctxid >> 4);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-02  7:17 ` [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing Jianping Li
@ 2026-06-02  9:25   ` Jie Gan
  2026-06-08  4:32     ` Jianping Li
  2026-06-08  6:11   ` Dmitry Baryshkov
  1 sibling, 1 reply; 16+ messages in thread
From: Jie Gan @ 2026-06-02  9:25 UTC (permalink / raw)
  To: Jianping Li, srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: linux-arm-msm, dri-devel, linux-kernel, ekansh.gupta,
	quic_chennak, stable



On 6/2/2026 3:17 PM, Jianping Li wrote:
> Audio PD static process creation assumes that a reserved-memory
> region is defined in DT and exposed via cctx->remote_heap.
> 
> If reserved-memory is missing or incomplete, the driver may pass
> invalid address/size information to the DSP, leading to undefined
> behavior or crashes.
> 
> Add explicit validation for remote_heap presence and size before
> sending the memory to DSP, and fail early if the configuration is
> invalid.
> 
> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
> Cc: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>   drivers/misc/fastrpc.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index a8a58f889d07..f46a8f53970d 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>   	} inbuf;
>   	u32 sc;
>   
> +	if (!fl->cctx->remote_heap ||
> +	    !fl->cctx->remote_heap->dma_addr ||
> +	    !fl->cctx->remote_heap->size) {
> +		err = -ENOMEM;
> +		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");

should be an error instead of dbg info.

Thanks,
Jie

> +		return err;
> +	}
>   	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
>   	if (!args)
>   		return -ENOMEM;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
  2026-06-02  7:17 ` [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
@ 2026-06-02  9:41   ` Jie Gan
  2026-06-08  4:35     ` Jianping Li
  0 siblings, 1 reply; 16+ messages in thread
From: Jie Gan @ 2026-06-02  9:41 UTC (permalink / raw)
  To: Jianping Li, srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez
  Cc: linux-arm-msm, dri-devel, linux-kernel, ekansh.gupta,
	quic_chennak, stable



On 6/2/2026 3:17 PM, Jianping Li wrote:
> Allocating and freeing Audio PD memory from userspace is unsafe because
> the kernel cannot reliably determine when the DSP has finished using the
> memory. Userspace may free buffers while they are still in use by the DSP,
> and remote free requests cannot be safely trusted.
> 
> Additionally, the current implementation allows userspace to repeatedly
> grow the Audio PD heap, but does not support shrinking it. This can lead
> to unbounded memory usage over time, effectively causing a memory leak.
> 
> Fix this by allocating the entire Audio PD reserved-memory region during
> rpmsg probe and tying its lifetime to the rpmsg channel. This removes
> userspace-controlled alloc/free and ensures that memory is reclaimed only
> when the DSP process is torn down.
> 
> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
> Cc: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>   drivers/misc/fastrpc.c | 96 +++++++++++++++++++-----------------------
>   1 file changed, 43 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index f46a8f53970d..33be8bed6a0b 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -276,6 +276,8 @@ struct fastrpc_channel_ctx {
>   	struct kref refcount;
>   	/* Flag if dsp attributes are cached */
>   	bool valid_attributes;
> +	/* Flag if audio PD init mem was allocated */
> +	bool audio_init_mem;
>   	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
>   	struct fastrpc_device *secure_fdevice;
>   	struct fastrpc_device *fdevice;
> @@ -1344,15 +1346,16 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>   	struct fastrpc_init_create_static init;
>   	struct fastrpc_invoke_args *args;
>   	struct fastrpc_phy_page pages[1];
> +	struct fastrpc_channel_ctx *cctx = fl->cctx;
>   	char *name;
>   	int err;
> -	bool scm_done = false;
>   	struct {
>   		int client_id;
>   		u32 namelen;
>   		u32 pageslen;
>   	} inbuf;
>   	u32 sc;
> +	unsigned long flags;
>   
>   	if (!fl->cctx->remote_heap ||
>   	    !fl->cctx->remote_heap->dma_addr ||
> @@ -1383,31 +1386,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>   	inbuf.client_id = fl->client_id;
>   	inbuf.namelen = init.namelen;
>   	inbuf.pageslen = 0;
> -	if (!fl->cctx->remote_heap) {
> -		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
> -						&fl->cctx->remote_heap);
> -		if (err)
> -			goto err_name;
> -
> -		/* Map if we have any heap VMIDs associated with this ADSP Static Process. */
> -		if (fl->cctx->vmcount) {
> -			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> -
> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> -							(u64)fl->cctx->remote_heap->size,
> -							&src_perms,
> -							fl->cctx->vmperms, fl->cctx->vmcount);
> -			if (err) {
> -				dev_err(fl->sctx->dev,
> -					"Failed to assign memory with dma_addr %pad size 0x%llx err %d\n",
> -					&fl->cctx->remote_heap->dma_addr,
> -					fl->cctx->remote_heap->size, err);
> -				goto err_map;
> -			}
> -			scm_done = true;
> -			inbuf.pageslen = 1;
> -		}
> -	}
>   
>   	fl->pd = USER_PD;
>   
> @@ -1419,8 +1397,17 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>   	args[1].length = inbuf.namelen;
>   	args[1].fd = -1;
>   
> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
> -	pages[0].size = fl->cctx->remote_heap->size;
> +	spin_lock_irqsave(&cctx->lock, flags);
> +	if (!fl->cctx->audio_init_mem) {
> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
> +		pages[0].size = fl->cctx->remote_heap->size;
> +		fl->cctx->audio_init_mem = true;
> +		inbuf.pageslen = 1;
> +	} else {
> +		pages[0].addr = 0;
> +		pages[0].size = 0;
> +	}
> +	spin_unlock_irqrestore(&cctx->lock, flags);
>   
>   	args[2].ptr = (u64)(uintptr_t) pages;
>   	args[2].length = sizeof(*pages);
> @@ -1438,27 +1425,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>   
>   	return 0;
>   err_invoke:
> -	if (fl->cctx->vmcount && scm_done) {
> -		u64 src_perms = 0;
> -		struct qcom_scm_vmperm dst_perms;
> -		u32 i;
> -
> -		for (i = 0; i < fl->cctx->vmcount; i++)
> -			src_perms |= BIT(fl->cctx->vmperms[i].vmid);
> -
> -		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> -		dst_perms.perm = QCOM_SCM_PERM_RWX;
> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> -						(u64)fl->cctx->remote_heap->size,
> -						&src_perms, &dst_perms, 1);
> -		if (err)
> -			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
> -				&fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> -	}
> -err_map:
> -	fastrpc_buf_free(fl->cctx->remote_heap);
> -	fl->cctx->remote_heap = NULL;
> -err_name:
> +	fl->cctx->audio_init_mem = false;
>   	kfree(name);
>   err:
>   	kfree(args);
> @@ -2425,12 +2392,21 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>   		}
>   	}
>   
> -	if (domain_id == SDSP_DOMAIN_ID) {
> +	if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
>   		struct resource res;
>   		u64 src_perms;
>   
>   		err = of_reserved_mem_region_to_resource(rdev->of_node, 0, &res);
>   		if (!err) {
> +			if (domain_id == ADSP_DOMAIN_ID) {
> +				data->remote_heap =
> +					kzalloc_obj(*data->remote_heap, GFP_KERNEL);
> +				if (!data->remote_heap)
> +					return -ENOMEM;

allocated data never free with directly return.

goto err_free_data;

Beside, we also need free data->remote_heap in err_free_data path as you 
added new memory allocation.

> +
> +				data->remote_heap->dma_addr = res.start;
> +				data->remote_heap->size = resource_size(&res);
> +			}
>   			src_perms = BIT(QCOM_SCM_VMID_HLOS);
>   
>   			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> @@ -2438,7 +2414,6 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>   			if (err)
>   				goto err_free_data;
>   		}
> -
>   	}
>   
>   	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
> @@ -2519,6 +2494,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>   	struct fastrpc_buf *buf, *b;
>   	struct fastrpc_user *user;
>   	unsigned long flags;
> +	int err;
>   
>   	/* No invocations past this point */
>   	spin_lock_irqsave(&cctx->lock, flags);
> @@ -2536,8 +2512,22 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>   	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
>   		list_del(&buf->node);
>   
> -	if (cctx->remote_heap)
> -		fastrpc_buf_free(cctx->remote_heap);

after removed the code, the cctx->remote_heap is not freed:
1. cctx->vmcount == 0
2. if (!err) is false

we should free the cctx->remote_heap unconditionally if it exists.

Thanks,
Jie

> +	if (cctx->remote_heap && cctx->vmcount) {
> +		u64 src_perms = 0;
> +		struct qcom_scm_vmperm dst_perms;
> +
> +		for (u32 i = 0; i < cctx->vmcount; i++)
> +			src_perms |= BIT(cctx->vmperms[i].vmid);
> +
> +		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> +		dst_perms.perm = QCOM_SCM_PERM_RWX;
> +
> +		err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
> +					  cctx->remote_heap->size, &src_perms,
> +					  &dst_perms, 1);
> +		if (!err)
> +			kfree(cctx->remote_heap);
> +	}
>   
>   	of_platform_depopulate(&rpdev->dev);
>   


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-02  9:25   ` Jie Gan
@ 2026-06-08  4:32     ` Jianping Li
  0 siblings, 0 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-08  4:32 UTC (permalink / raw)
  To: Jie Gan
  Cc: srini, amahesh, arnd, Greg KH, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, Ekansh Gupta,
	quic_chennak, stable


On 6/2/2026 5:25 PM, Jie Gan wrote:
>
>
> On 6/2/2026 3:17 PM, Jianping Li wrote:
>> Audio PD static process creation assumes that a reserved-memory
>> region is defined in DT and exposed via cctx->remote_heap.
>>
>> If reserved-memory is missing or incomplete, the driver may pass
>> invalid address/size information to the DSP, leading to undefined
>> behavior or crashes.
>>
>> Add explicit validation for remote_heap presence and size before
>> sending the memory to DSP, and fail early if the configuration is
>> invalid.
>>
>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a8a58f889d07..f46a8f53970d 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1354,6 +1354,13 @@ static int 
>> fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>       } inbuf;
>>       u32 sc;
>>   +    if (!fl->cctx->remote_heap ||
>> +        !fl->cctx->remote_heap->dma_addr ||
>> +        !fl->cctx->remote_heap->size) {
>> +        err = -ENOMEM;
>> +        dev_dbg(fl->sctx->dev, "remote heap memory region is not 
>> added\n");
>
> should be an error instead of dbg info.
>
> Thanks,
> Jie

I will use dev_err to print this log.

Thanks,

Jianping.

>
>> +        return err;
>> +    }
>>       args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
>>       if (!args)
>>           return -ENOMEM;
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
  2026-06-02  9:41   ` Jie Gan
@ 2026-06-08  4:35     ` Jianping Li
  0 siblings, 0 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-08  4:35 UTC (permalink / raw)
  To: Jie Gan
  Cc: srini, amahesh, arnd, Greg KH, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, Ekansh Gupta,
	quic_chennak, stable


On 6/2/2026 5:41 PM, Jie Gan wrote:
>
>
> On 6/2/2026 3:17 PM, Jianping Li wrote:
>> Allocating and freeing Audio PD memory from userspace is unsafe because
>> the kernel cannot reliably determine when the DSP has finished using the
>> memory. Userspace may free buffers while they are still in use by the 
>> DSP,
>> and remote free requests cannot be safely trusted.
>>
>> Additionally, the current implementation allows userspace to repeatedly
>> grow the Audio PD heap, but does not support shrinking it. This can lead
>> to unbounded memory usage over time, effectively causing a memory leak.
>>
>> Fix this by allocating the entire Audio PD reserved-memory region during
>> rpmsg probe and tying its lifetime to the rpmsg channel. This removes
>> userspace-controlled alloc/free and ensures that memory is reclaimed 
>> only
>> when the DSP process is torn down.
>>
>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 96 +++++++++++++++++++-----------------------
>>   1 file changed, 43 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index f46a8f53970d..33be8bed6a0b 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -276,6 +276,8 @@ struct fastrpc_channel_ctx {
>>       struct kref refcount;
>>       /* Flag if dsp attributes are cached */
>>       bool valid_attributes;
>> +    /* Flag if audio PD init mem was allocated */
>> +    bool audio_init_mem;
>>       u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
>>       struct fastrpc_device *secure_fdevice;
>>       struct fastrpc_device *fdevice;
>> @@ -1344,15 +1346,16 @@ static int 
>> fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>       struct fastrpc_init_create_static init;
>>       struct fastrpc_invoke_args *args;
>>       struct fastrpc_phy_page pages[1];
>> +    struct fastrpc_channel_ctx *cctx = fl->cctx;
>>       char *name;
>>       int err;
>> -    bool scm_done = false;
>>       struct {
>>           int client_id;
>>           u32 namelen;
>>           u32 pageslen;
>>       } inbuf;
>>       u32 sc;
>> +    unsigned long flags;
>>         if (!fl->cctx->remote_heap ||
>>           !fl->cctx->remote_heap->dma_addr ||
>> @@ -1383,31 +1386,6 @@ static int 
>> fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>       inbuf.client_id = fl->client_id;
>>       inbuf.namelen = init.namelen;
>>       inbuf.pageslen = 0;
>> -    if (!fl->cctx->remote_heap) {
>> -        err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
>> -                        &fl->cctx->remote_heap);
>> -        if (err)
>> -            goto err_name;
>> -
>> -        /* Map if we have any heap VMIDs associated with this ADSP 
>> Static Process. */
>> -        if (fl->cctx->vmcount) {
>> -            u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>> -
>> -            err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> - (u64)fl->cctx->remote_heap->size,
>> -                            &src_perms,
>> -                            fl->cctx->vmperms, fl->cctx->vmcount);
>> -            if (err) {
>> -                dev_err(fl->sctx->dev,
>> -                    "Failed to assign memory with dma_addr %pad size 
>> 0x%llx err %d\n",
>> - &fl->cctx->remote_heap->dma_addr,
>> -                    fl->cctx->remote_heap->size, err);
>> -                goto err_map;
>> -            }
>> -            scm_done = true;
>> -            inbuf.pageslen = 1;
>> -        }
>> -    }
>>         fl->pd = USER_PD;
>>   @@ -1419,8 +1397,17 @@ static int 
>> fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>       args[1].length = inbuf.namelen;
>>       args[1].fd = -1;
>>   -    pages[0].addr = fl->cctx->remote_heap->dma_addr;
>> -    pages[0].size = fl->cctx->remote_heap->size;
>> +    spin_lock_irqsave(&cctx->lock, flags);
>> +    if (!fl->cctx->audio_init_mem) {
>> +        pages[0].addr = fl->cctx->remote_heap->dma_addr;
>> +        pages[0].size = fl->cctx->remote_heap->size;
>> +        fl->cctx->audio_init_mem = true;
>> +        inbuf.pageslen = 1;
>> +    } else {
>> +        pages[0].addr = 0;
>> +        pages[0].size = 0;
>> +    }
>> +    spin_unlock_irqrestore(&cctx->lock, flags);
>>         args[2].ptr = (u64)(uintptr_t) pages;
>>       args[2].length = sizeof(*pages);
>> @@ -1438,27 +1425,7 @@ static int 
>> fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>         return 0;
>>   err_invoke:
>> -    if (fl->cctx->vmcount && scm_done) {
>> -        u64 src_perms = 0;
>> -        struct qcom_scm_vmperm dst_perms;
>> -        u32 i;
>> -
>> -        for (i = 0; i < fl->cctx->vmcount; i++)
>> -            src_perms |= BIT(fl->cctx->vmperms[i].vmid);
>> -
>> -        dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>> -        dst_perms.perm = QCOM_SCM_PERM_RWX;
>> -        err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> - (u64)fl->cctx->remote_heap->size,
>> -                        &src_perms, &dst_perms, 1);
>> -        if (err)
>> -            dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 
>> %pad size 0x%llx err %d\n",
>> -                &fl->cctx->remote_heap->dma_addr, 
>> fl->cctx->remote_heap->size, err);
>> -    }
>> -err_map:
>> -    fastrpc_buf_free(fl->cctx->remote_heap);
>> -    fl->cctx->remote_heap = NULL;
>> -err_name:
>> +    fl->cctx->audio_init_mem = false;
>>       kfree(name);
>>   err:
>>       kfree(args);
>> @@ -2425,12 +2392,21 @@ static int fastrpc_rpmsg_probe(struct 
>> rpmsg_device *rpdev)
>>           }
>>       }
>>   -    if (domain_id == SDSP_DOMAIN_ID) {
>> +    if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
>>           struct resource res;
>>           u64 src_perms;
>>             err = of_reserved_mem_region_to_resource(rdev->of_node, 
>> 0, &res);
>>           if (!err) {
>> +            if (domain_id == ADSP_DOMAIN_ID) {
>> +                data->remote_heap =
>> +                    kzalloc_obj(*data->remote_heap, GFP_KERNEL);
>> +                if (!data->remote_heap)
>> +                    return -ENOMEM;
>
> allocated data never free with directly return.
>
> goto err_free_data;
>
> Beside, we also need free data->remote_heap in err_free_data path as 
> you added new memory allocation.

I will adjust goto err_free_data, and add free(data->remote_heap) in 
err_free_data.

err_free_data:
     kfree(data->remote_heap);
     kfree(data);

>
>> +
>> +                data->remote_heap->dma_addr = res.start;
>> +                data->remote_heap->size = resource_size(&res);
>> +            }
>>               src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>                 err = qcom_scm_assign_mem(res.start, 
>> resource_size(&res), &src_perms,
>> @@ -2438,7 +2414,6 @@ static int fastrpc_rpmsg_probe(struct 
>> rpmsg_device *rpdev)
>>               if (err)
>>                   goto err_free_data;
>>           }
>> -
>>       }
>>         secure_dsp = !(of_property_read_bool(rdev->of_node, 
>> "qcom,non-secure-domain"));
>> @@ -2519,6 +2494,7 @@ static void fastrpc_rpmsg_remove(struct 
>> rpmsg_device *rpdev)
>>       struct fastrpc_buf *buf, *b;
>>       struct fastrpc_user *user;
>>       unsigned long flags;
>> +    int err;
>>         /* No invocations past this point */
>>       spin_lock_irqsave(&cctx->lock, flags);
>> @@ -2536,8 +2512,22 @@ static void fastrpc_rpmsg_remove(struct 
>> rpmsg_device *rpdev)
>>       list_for_each_entry_safe(buf, b, 
>> &cctx->invoke_interrupted_mmaps, node)
>>           list_del(&buf->node);
>>   -    if (cctx->remote_heap)
>> -        fastrpc_buf_free(cctx->remote_heap);
>
> after removed the code, the cctx->remote_heap is not freed:
> 1. cctx->vmcount == 0
> 2. if (!err) is false
>
> we should free the cctx->remote_heap unconditionally if it exists.
>
> Thanks,
> Jie

You are right, after changing this to kfree here, the lifecycle should 
not be tied to whether the assignment is successful

so here we should kfree remote_heap every time.

err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
                       cctx->remote_heap->size, &src_perms,
                       &dst_perms, 1);

         if (err)
                 dev_err(&rpdev->dev,
                         "Failed to assign memory back to HLOS: dma_addr 
%pad size %#llx err %d\n",
                         &cctx->remote_heap->dma_addr,
                         cctx->remote_heap->size, err);
         }

         kfree(cctx->remote_heap);
         cctx->remote_heap = NULL;

>
>> +    if (cctx->remote_heap && cctx->vmcount) {
>> +        u64 src_perms = 0;
>> +        struct qcom_scm_vmperm dst_perms;
>> +
>> +        for (u32 i = 0; i < cctx->vmcount; i++)
>> +            src_perms |= BIT(cctx->vmperms[i].vmid);
>> +
>> +        dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>> +        dst_perms.perm = QCOM_SCM_PERM_RWX;
>> +
>> +        err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
>> +                      cctx->remote_heap->size, &src_perms,
>> +                      &dst_perms, 1);
>> +        if (!err)
>> +            kfree(cctx->remote_heap);
>> +    }
>>         of_platform_depopulate(&rpdev->dev);
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool
  2026-06-02  7:17 ` [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool Jianping Li
@ 2026-06-08  6:06   ` Dmitry Baryshkov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-06-08  6:06 UTC (permalink / raw)
  To: Jianping Li
  Cc: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez,
	Ekansh Gupta, linux-arm-msm, dri-devel, linux-kernel,
	quic_chennak, stable

On Tue, Jun 02, 2026 at 03:17:46PM +0800, Jianping Li wrote:
> From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> 
> The initial buffer allocated for the Audio PD memory pool is never added
> to the pool because pageslen is set to 0. As a result, the buffer is not
> registered with Audio PD and is never used, causing a memory leak. Audio
> PD immediately falls back to allocating memory from the remote heap since
> the pool starts out empty.
> 
> Fix this by setting pageslen to 1 so that the initially allocated buffer
> is correctly registered and becomes part of the Audio PD memory pool.
> 
> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
> Cc: stable@kernel.org
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation
  2026-06-02  7:17 ` [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation Jianping Li
@ 2026-06-08  6:06   ` Dmitry Baryshkov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-06-08  6:06 UTC (permalink / raw)
  To: Jianping Li
  Cc: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez,
	Ekansh Gupta, linux-arm-msm, dri-devel, linux-kernel,
	quic_chennak, stable

On Tue, Jun 02, 2026 at 03:17:47PM +0800, Jianping Li wrote:
> From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> 
> fastrpc_req_munmap_impl() is called to unmap any buffer. The buffer is
> getting removed from the list after it is unmapped from DSP. This can
> create potential race conditions if multiple threads invoke unmap
> concurrently, where one thread may remove the entry from the list while
> another thread's unmap operation is still ongoing.
> 
> Fix this by removing the buffer entry from the list before calling the
> unmap operation. If the unmap fails, the entry is re-added to the list
> so that userspace can retry the unmap, or alternatively, the buffer
> will be cleaned up during device release when the DSP process is torn
> down and all DSP-side mappings are freed along with remaining buffers
> in the list.
> 
> Fixes: 2419e55e532de ("misc: fastrpc: add mmap/unmap support")
> Cc: stable@kernel.org
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-02  7:17 ` [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing Jianping Li
  2026-06-02  9:25   ` Jie Gan
@ 2026-06-08  6:11   ` Dmitry Baryshkov
  2026-06-08  6:29     ` Jianping Li
  1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-06-08  6:11 UTC (permalink / raw)
  To: Jianping Li
  Cc: srini, amahesh, arnd, gregkh, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, ekansh.gupta,
	quic_chennak, stable

On Tue, Jun 02, 2026 at 03:17:48PM +0800, Jianping Li wrote:
> Audio PD static process creation assumes that a reserved-memory
> region is defined in DT and exposed via cctx->remote_heap.
> 
> If reserved-memory is missing or incomplete, the driver may pass
> invalid address/size information to the DSP, leading to undefined
> behavior or crashes.
> 
> Add explicit validation for remote_heap presence and size before
> sending the memory to DSP, and fail early if the configuration is
> invalid.
> 
> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
> Cc: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index a8a58f889d07..f46a8f53970d 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	} inbuf;
>  	u32 sc;
>  
> +	if (!fl->cctx->remote_heap ||
> +	    !fl->cctx->remote_heap->dma_addr ||
> +	    !fl->cctx->remote_heap->size) {
> +		err = -ENOMEM;
> +		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");
> +		return err;
> +	}

Won't this check without the next commit break the kernel? I don't see
where else the remote heap is being allocated.

>  	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
>  	if (!args)
>  		return -ENOMEM;
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-08  6:11   ` Dmitry Baryshkov
@ 2026-06-08  6:29     ` Jianping Li
  2026-06-08  6:37       ` Dmitry Baryshkov
  0 siblings, 1 reply; 16+ messages in thread
From: Jianping Li @ 2026-06-08  6:29 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: srini, amahesh, arnd, Greg KH, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, Ekansh Gupta,
	quic_chennak, stable


On 6/8/2026 2:11 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 02, 2026 at 03:17:48PM +0800, Jianping Li wrote:
>> Audio PD static process creation assumes that a reserved-memory
>> region is defined in DT and exposed via cctx->remote_heap.
>>
>> If reserved-memory is missing or incomplete, the driver may pass
>> invalid address/size information to the DSP, leading to undefined
>> behavior or crashes.
>>
>> Add explicit validation for remote_heap presence and size before
>> sending the memory to DSP, and fail early if the configuration is
>> invalid.
>>
>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a8a58f889d07..f46a8f53970d 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	} inbuf;
>>   	u32 sc;
>>   
>> +	if (!fl->cctx->remote_heap ||
>> +	    !fl->cctx->remote_heap->dma_addr ||
>> +	    !fl->cctx->remote_heap->size) {
>> +		err = -ENOMEM;
>> +		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");
>> +		return err;
>> +	}
> Won't this check without the next commit break the kernel? I don't see
> where else the remote heap is being allocated.

Yes, this check depends on remote_heap being populated during rpmsg
probe from the reserved-memory region, introduced in a subsequent
patch in this series.

If there is no dependency on the next series, the remote_heap allocation here after the if check.
The logic will be wrong.

>
>>   	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
>>   	if (!args)
>>   		return -ENOMEM;
>> -- 
>> 2.43.0
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-08  6:29     ` Jianping Li
@ 2026-06-08  6:37       ` Dmitry Baryshkov
  2026-06-08  6:45         ` Jianping Li
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-06-08  6:37 UTC (permalink / raw)
  To: Jianping Li
  Cc: srini, amahesh, arnd, Greg KH, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, Ekansh Gupta,
	quic_chennak, stable

On Mon, Jun 08, 2026 at 02:29:13PM +0800, Jianping Li wrote:
> 
> On 6/8/2026 2:11 PM, Dmitry Baryshkov wrote:
> > On Tue, Jun 02, 2026 at 03:17:48PM +0800, Jianping Li wrote:
> > > Audio PD static process creation assumes that a reserved-memory
> > > region is defined in DT and exposed via cctx->remote_heap.
> > > 
> > > If reserved-memory is missing or incomplete, the driver may pass
> > > invalid address/size information to the DSP, leading to undefined
> > > behavior or crashes.
> > > 
> > > Add explicit validation for remote_heap presence and size before
> > > sending the memory to DSP, and fail early if the configuration is
> > > invalid.
> > > 
> > > Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
> > > Cc: stable@kernel.org
> > > Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index a8a58f889d07..f46a8f53970d 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > >   	} inbuf;
> > >   	u32 sc;
> > > +	if (!fl->cctx->remote_heap ||
> > > +	    !fl->cctx->remote_heap->dma_addr ||
> > > +	    !fl->cctx->remote_heap->size) {
> > > +		err = -ENOMEM;
> > > +		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");
> > > +		return err;
> > > +	}
> > Won't this check without the next commit break the kernel? I don't see
> > where else the remote heap is being allocated.
> 
> Yes, this check depends on remote_heap being populated during rpmsg
> probe from the reserved-memory region, introduced in a subsequent
> patch in this series.
> 
> If there is no dependency on the next series, the remote_heap allocation here after the if check.
> The logic will be wrong.

So, at least you need to squash those two together.

> 
> > 
> > >   	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
> > >   	if (!args)
> > >   		return -ENOMEM;
> > > -- 
> > > 2.43.0
> > > 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing
  2026-06-08  6:37       ` Dmitry Baryshkov
@ 2026-06-08  6:45         ` Jianping Li
  0 siblings, 0 replies; 16+ messages in thread
From: Jianping Li @ 2026-06-08  6:45 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: srini, amahesh, arnd, Greg KH, abelvesa, jorge.ramirez,
	linux-arm-msm, dri-devel, linux-kernel, Ekansh Gupta,
	quic_chennak, stable


On 6/8/2026 2:37 PM, Dmitry Baryshkov wrote:
> On Mon, Jun 08, 2026 at 02:29:13PM +0800, Jianping Li wrote:
>> On 6/8/2026 2:11 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jun 02, 2026 at 03:17:48PM +0800, Jianping Li wrote:
>>>> Audio PD static process creation assumes that a reserved-memory
>>>> region is defined in DT and exposed via cctx->remote_heap.
>>>>
>>>> If reserved-memory is missing or incomplete, the driver may pass
>>>> invalid address/size information to the DSP, leading to undefined
>>>> behavior or crashes.
>>>>
>>>> Add explicit validation for remote_heap presence and size before
>>>> sending the memory to DSP, and fail early if the configuration is
>>>> invalid.
>>>>
>>>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>>>> Cc: stable@kernel.org
>>>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>>>> ---
>>>>    drivers/misc/fastrpc.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index a8a58f889d07..f46a8f53970d 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -1354,6 +1354,13 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>>>    	} inbuf;
>>>>    	u32 sc;
>>>> +	if (!fl->cctx->remote_heap ||
>>>> +	    !fl->cctx->remote_heap->dma_addr ||
>>>> +	    !fl->cctx->remote_heap->size) {
>>>> +		err = -ENOMEM;
>>>> +		dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n");
>>>> +		return err;
>>>> +	}
>>> Won't this check without the next commit break the kernel? I don't see
>>> where else the remote heap is being allocated.
>> Yes, this check depends on remote_heap being populated during rpmsg
>> probe from the reserved-memory region, introduced in a subsequent
>> patch in this series.
>>
>> If there is no dependency on the next series, the remote_heap allocation here after the if check.
>> The logic will be wrong.
> So, at least you need to squash those two together.

ACK.
I'll squash those two patches together in the next revision.

>
>>>>    	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
>>>>    	if (!args)
>>>>    		return -ENOMEM;
>>>> -- 
>>>> 2.43.0
>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-06-08  6:45 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-02  7:17 [PATCH v7 0/5] misc: fastrpc: Add missing bug fixes Jianping Li
2026-06-02  7:17 ` [PATCH v7 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool Jianping Li
2026-06-08  6:06   ` Dmitry Baryshkov
2026-06-02  7:17 ` [PATCH v7 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation Jianping Li
2026-06-08  6:06   ` Dmitry Baryshkov
2026-06-02  7:17 ` [PATCH v7 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing Jianping Li
2026-06-02  9:25   ` Jie Gan
2026-06-08  4:32     ` Jianping Li
2026-06-08  6:11   ` Dmitry Baryshkov
2026-06-08  6:29     ` Jianping Li
2026-06-08  6:37       ` Dmitry Baryshkov
2026-06-08  6:45         ` Jianping Li
2026-06-02  7:17 ` [PATCH v7 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
2026-06-02  9:41   ` Jie Gan
2026-06-08  4:35     ` Jianping Li
2026-06-02  7:17 ` [PATCH v7 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL Jianping Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome