[PATCH v5 0/5] misc: fastrpc: Add missing bug fixes

[PATCH v5 0/5] misc: fastrpc: Add missing bug fixes

[Jianping Li]

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 [v4]: https://lore.kernel.org/all/20260409062617.1182-1-jianping.li@oss.qualcomm.com/

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 | 135 ++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 64 deletions(-)

-- 
2.43.0

[PATCH v5 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool

[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
Co-developed-by: Ekansh Gupta <ekansh.gupta@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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 1080f9acf70a..8b21f85cd9f4 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1324,7 +1324,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);
@@ -1347,12 +1349,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

[PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[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 any other thread removes the entry
from list while unmap operation is ongoing. Remove the entry before
calling unmap operation.

Fixes: 2419e55e532de ("misc: fastrpc: add mmap/unmap support")
Cc: stable@kernel.org
Co-developed-by: Ekansh Gupta <ekansh.gupta@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 | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 8b21f85cd9f4..3c7c3b410d7d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1863,9 +1863,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);
@@ -1879,6 +1876,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;
@@ -1886,6 +1884,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;
 		}
@@ -1898,7 +1897,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)
@@ -1989,14 +1995,17 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 
 	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
 		err = -EFAULT;
-		goto err_assign;
+		goto err_copy;
 	}
 
 	dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
 		buf->raddr, buf->size);
 
 	return 0;
-
+err_copy:
+	spin_lock(&fl->lock);
+	list_del(&buf->node);
+	spin_unlock(&fl->lock);
 err_assign:
 	fastrpc_req_munmap_impl(fl, buf);
 
-- 
2.43.0

[PATCH v5 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing

[Jianping Li]

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 3c7c3b410d7d..a0337cce77f3 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1363,6 +1363,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	args[1].length = inbuf.namelen;
 	args[1].fd = -1;
 
+	if (!fl->cctx->remote_heap ||
+	    !fl->cctx->remote_heap->dma_addr ||
+	    !fl->cctx->remote_heap->size) {
+		err = -ENOMEM;
+		goto err;
+	}
 	pages[0].addr = fl->cctx->remote_heap->dma_addr;
 	pages[0].size = fl->cctx->remote_heap->size;
 
-- 
2.43.0

[PATCH v5 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Jianping Li]

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.

Allocate the entire Audio PD reserved-memory region upfront during rpmsg
probe and tie its lifetime to the rpmsg channel. This avoids userspace-
controlled alloc/free and ensures memory is reclaimed only when the DSP
shuts down.

Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 107 +++++++++++++++++++----------------------
 1 file changed, 49 insertions(+), 58 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a0337cce77f3..9c70788afa0f 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;
@@ -1295,15 +1297,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;
 
 	args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS);
 	if (!args)
@@ -1327,31 +1330,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;
 
@@ -1363,14 +1341,24 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	args[1].length = inbuf.namelen;
 	args[1].fd = -1;
 
-	if (!fl->cctx->remote_heap ||
-	    !fl->cctx->remote_heap->dma_addr ||
-	    !fl->cctx->remote_heap->size) {
-		err = -ENOMEM;
-		goto err;
+	spin_lock_irqsave(&cctx->lock, flags);
+	if (!fl->cctx->audio_init_mem) {
+		if (!fl->cctx->remote_heap ||
+		    !fl->cctx->remote_heap->dma_addr ||
+		    !fl->cctx->remote_heap->size) {
+			spin_unlock_irqrestore(&cctx->lock, flags);
+			err = -ENOMEM;
+			goto err;
+		}
+		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;
 	}
-	pages[0].addr = fl->cctx->remote_heap->dma_addr;
-	pages[0].size = fl->cctx->remote_heap->size;
+	spin_unlock_irqrestore(&cctx->lock, flags);
 
 	args[2].ptr = (u64)(uintptr_t) pages;
 	args[2].length = sizeof(*pages);
@@ -1388,27 +1376,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);
@@ -2397,12 +2365,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,
@@ -2410,7 +2387,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"));
@@ -2491,6 +2467,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);
@@ -2508,8 +2485,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

[PATCH v5 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL

[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.

Co-developed-by: Ekansh Gupta <ekansh.gupta@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 9c70788afa0f..82bfbf342725 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -416,6 +416,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);
@@ -512,8 +515,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);
@@ -1564,8 +1566,7 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
 	list_del(&fl->user);
 	spin_unlock_irqrestore(&cctx->lock, flags);
 
-	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);
-- 
2.43.0

Re: [PATCH v5 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 08:42:13PM +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
> Co-developed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

If it's From:Ekansh, it can't be CDB: Ekansh. How can Ekansh co-develop
the patch with himself?

> 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 1080f9acf70a..8b21f85cd9f4 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1324,7 +1324,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);
> @@ -1347,12 +1349,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
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> from list while unmap operation is ongoing. Remove the entry before

How can it remove the entry from the list?

> calling unmap operation.
> 
> Fixes: 2419e55e532de ("misc: fastrpc: add mmap/unmap support")
> Cc: stable@kernel.org
> Co-developed-by: Ekansh Gupta <ekansh.gupta@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 | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 8b21f85cd9f4..3c7c3b410d7d 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1863,9 +1863,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);
> @@ -1879,6 +1876,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;
> @@ -1886,6 +1884,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;
>  		}
> @@ -1898,7 +1897,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);
> +	}

Is it expected that userspace tries to unmap it again? Or why is it
being added to the list?

> +
> +	return err;
>  }
>  
>  static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> @@ -1989,14 +1995,17 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  
>  	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
>  		err = -EFAULT;
> -		goto err_assign;
> +		goto err_copy;
>  	}
>  
>  	dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
>  		buf->raddr, buf->size);
>  
>  	return 0;
> -
> +err_copy:
> +	spin_lock(&fl->lock);
> +	list_del(&buf->node);
> +	spin_unlock(&fl->lock);

This is a separate fix.

>  err_assign:
>  	fastrpc_req_munmap_impl(fl, buf);
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 08:42:15PM +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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 3c7c3b410d7d..a0337cce77f3 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1363,6 +1363,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	args[1].length = inbuf.namelen;
>  	args[1].fd = -1;
>  
> +	if (!fl->cctx->remote_heap ||
> +	    !fl->cctx->remote_heap->dma_addr ||
> +	    !fl->cctx->remote_heap->size) {

I guess that !dma_addr || !size should fail much earlier than here.

> +		err = -ENOMEM;
> +		goto err;
> +	}
>  	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>  	pages[0].size = fl->cctx->remote_heap->size;
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 08:42:16PM +0800, 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.
> 
> Allocate the entire Audio PD reserved-memory region upfront during rpmsg
> probe and tie its lifetime to the rpmsg channel. This avoids userspace-
> controlled alloc/free and ensures memory is reclaimed only when the DSP
> shuts down.

So, is this a bugfix or not? Is it possible to make the kernel misbehave
without this patch being applied?

> 
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 107 +++++++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 58 deletions(-)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 5/5] misc: fastrpc: Allow fastrpc_buf_free() to accept NULL

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 08:42:17PM +0800, Jianping Li wrote:
> 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.
> 
> Co-developed-by: Ekansh Gupta <ekansh.gupta@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(-)
> 

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


-- 
With best wishes
Dmitry

Re: [PATCH v5 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing

[Konrad Dybcio]

On 5/15/26 2:42 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 3c7c3b410d7d..a0337cce77f3 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1363,6 +1363,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	args[1].length = inbuf.namelen;
>  	args[1].fd = -1;
>  
> +	if (!fl->cctx->remote_heap ||
> +	    !fl->cctx->remote_heap->dma_addr ||
> +	    !fl->cctx->remote_heap->size) {
> +		err = -ENOMEM;
> +		goto err;

Let's maybe add an UERR() with some context

Konrad

Re: [PATCH v5 1/5] misc: fastrpc: Fix initial memory allocation for Audio PD memory pool

[Jianping Li]

On 5/15/2026 9:33 PM, Dmitry Baryshkov wrote:
> On Fri, May 15, 2026 at 08:42:13PM +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
>> Co-developed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> If it's From:Ekansh, it can't be CDB: Ekansh. How can Ekansh co-develop
> the patch with himself?

I will delete CBD:Ekansh.

>
>> 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 1080f9acf70a..8b21f85cd9f4 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1324,7 +1324,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);
>> @@ -1347,12 +1349,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
>>

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Greg KH]

On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> 
> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > from list while unmap operation is ongoing. Remove the entry before
> > How can it remove the entry from the list?
> 
> Multiple threads sharing the same file descriptor may invoke unmap concurrently.

multiple threads sharing the same file descriptor is a horrible
userspace bug.  If you do that, you get what you deserve :)

thanks,

greg k-h

Re: [PATCH v5 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing

[Jianping Li]

On 5/15/2026 9:37 PM, Dmitry Baryshkov wrote:
> On Fri, May 15, 2026 at 08:42:15PM +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 | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 3c7c3b410d7d..a0337cce77f3 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1363,6 +1363,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	args[1].length = inbuf.namelen;
>>   	args[1].fd = -1;
>>   
>> +	if (!fl->cctx->remote_heap ||
>> +	    !fl->cctx->remote_heap->dma_addr ||
>> +	    !fl->cctx->remote_heap->size) {
> I guess that !dma_addr || !size should fail much earlier than here.

ACK

>
>> +		err = -ENOMEM;
>> +		goto err;
>> +	}
>>   	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>   	pages[0].size = fl->cctx->remote_heap->size;
>>   
>> -- 
>> 2.43.0
>>

Re: [PATCH v5 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Jianping Li]

On 5/15/2026 9:38 PM, Dmitry Baryshkov wrote:
> On Fri, May 15, 2026 at 08:42:16PM +0800, 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.
>>
>> Allocate the entire Audio PD reserved-memory region upfront during rpmsg
>> probe and tie its lifetime to the rpmsg channel. This avoids userspace-
>> controlled alloc/free and ensures memory is reclaimed only when the DSP
>> shuts down.
> So, is this a bugfix or not? Is it possible to make the kernel misbehave
> without this patch being applied?

Yes, this is a bug fix.
  
Because currently after the audio PD requests to grow the heap,
the current kernel does not support shrinking the heap, which will all cause memory leaks.

The current modification is to allocate in advance and release uniformly, which will avoid this error.

Thanks,
Jianping.

>
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 107 +++++++++++++++++++----------------------
>>   1 file changed, 49 insertions(+), 58 deletions(-)
>>

Re: [PATCH v5 3/5] misc: fastrpc: Fail Audio PD init when reserved memory is missing

[Jianping Li]

On 5/18/2026 3:59 PM, Konrad Dybcio wrote:
> On 5/15/26 2:42 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 | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 3c7c3b410d7d..a0337cce77f3 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1363,6 +1363,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	args[1].length = inbuf.namelen;
>>   	args[1].fd = -1;
>>   
>> +	if (!fl->cctx->remote_heap ||
>> +	    !fl->cctx->remote_heap->dma_addr ||
>> +	    !fl->cctx->remote_heap->size) {
>> +		err = -ENOMEM;
>> +		goto err;
> Let's maybe add an UERR() with some context
>
> Konrad

Planning to move this check at the start of this function as per Dmitry's suggestion.
I'll also add a dev_dbg log there.

Thanks,
Jianping.

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Greg KH]

On Mon, May 25, 2026 at 11:30:43AM +0800, Jianping Li wrote:
> 
> On 5/22/2026 3:03 PM, Greg KH wrote:
> > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> > > On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > > > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > > > from list while unmap operation is ongoing. Remove the entry before
> > > > How can it remove the entry from the list?
> > > Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > multiple threads sharing the same file descriptor is a horrible
> > userspace bug.  If you do that, you get what you deserve :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> I agree that concurrent unmap on the same buffer from multiple threads
> is a userspace bug.
> 
> However, the fastrpc driver is exposed via ioctl, and any userspace
> (including any random apk) can map directly and then call multiple unmap.

So there is no userspace selinux permissions settings on the fasterpc
device node at all?

> This patch is not intended to support incorrect userspace usage,
> but to ensure that the driver remains robust against invalid or
> racy inputs.

Isn't control to this device an "admin only" thing?

thanks,

greg k-h

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Mon, May 25, 2026 at 07:06:37AM +0200, Greg KH wrote:
> On Mon, May 25, 2026 at 11:30:43AM +0800, Jianping Li wrote:
> > 
> > On 5/22/2026 3:03 PM, Greg KH wrote:
> > > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> > > > On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > > > > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > > > > from list while unmap operation is ongoing. Remove the entry before
> > > > > How can it remove the entry from the list?
> > > > Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > > multiple threads sharing the same file descriptor is a horrible
> > > userspace bug.  If you do that, you get what you deserve :)
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > I agree that concurrent unmap on the same buffer from multiple threads
> > is a userspace bug.
> > 
> > However, the fastrpc driver is exposed via ioctl, and any userspace
> > (including any random apk) can map directly and then call multiple unmap.
> 
> So there is no userspace selinux permissions settings on the fasterpc
> device node at all?
> 
> > This patch is not intended to support incorrect userspace usage,
> > but to ensure that the driver remains robust against invalid or
> > racy inputs.
> 
> Isn't control to this device an "admin only" thing?

No, it's being used by normal user apps. Also, there are systems without
SELinux being enabled / used.

> 
> thanks,
> 
> greg k-h

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> 
> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > from list while unmap operation is ongoing. Remove the entry before
> > How can it remove the entry from the list?
> 
> Multiple threads sharing the same file descriptor may invoke unmap concurrently.

=> commit message

> > > @@ -1898,7 +1897,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);
> > > +	}
> > Is it expected that userspace tries to unmap it again? Or why is it
> > being added to the list?
> 
> User process can call unmap and fastrpc library won't call the unmap again.

In the other email you wrote that the driver can be used by random apps.
So... what happens if userspace unmaps it again? What if the userspace
_doesn't_ unmap it (although you've just readded it back)?

> Fastrpc driver will free up this list during fastrpc user-free.

-- 
With best wishes
Dmitry

Re: [PATCH v5 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Dmitry Baryshkov]

On Fri, May 22, 2026 at 03:47:31PM +0800, Jianping Li wrote:
> 
> On 5/15/2026 9:38 PM, Dmitry Baryshkov wrote:
> > On Fri, May 15, 2026 at 08:42:16PM +0800, 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.
> > > 
> > > Allocate the entire Audio PD reserved-memory region upfront during rpmsg
> > > probe and tie its lifetime to the rpmsg channel. This avoids userspace-
> > > controlled alloc/free and ensures memory is reclaimed only when the DSP
> > > shuts down.
> > So, is this a bugfix or not? Is it possible to make the kernel misbehave
> > without this patch being applied?
> 
> Yes, this is a bug fix.

The tag, cc:stable, clear description as the bugfix? How would anybody
guess if the patch is to be backported to earlier kernels or not?

> Because currently after the audio PD requests to grow the heap,
> the current kernel does not support shrinking the heap, which will all cause memory leaks.
> 
> The current modification is to allocate in advance and release uniformly, which will avoid this error.
> 
> Thanks,
> Jianping.
> 
> > 
> > > Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 107 +++++++++++++++++++----------------------
> > >   1 file changed, 49 insertions(+), 58 deletions(-)
> > > 

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Jianping Li]

On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
> On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
>> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
>>> On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
>>>> from list while unmap operation is ongoing. Remove the entry before
>>> How can it remove the entry from the list?
>> Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> => commit message
>
>>>> @@ -1898,7 +1897,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);
>>>> +	}
>>> Is it expected that userspace tries to unmap it again? Or why is it
>>> being added to the list?
>> User process can call unmap and fastrpc library won't call the unmap again.
> In the other email you wrote that the driver can be used by random apps.
> So... what happens if userspace unmaps it again? What if the userspace
> _doesn't_ unmap it (although you've just readded it back)?

If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
If userspace no longer performs unmap, the driver will not unmap it proactively.
The Fastrpc driver will free up this list during fastrpc user-free.

>
>> Fastrpc driver will free up this list during fastrpc user-free.

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Mon, 25 May 2026 at 11:34, Jianping Li <jianping.li@oss.qualcomm.com> wrote:
>
>
> On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
> > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> >> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> >>> On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> >>>> from list while unmap operation is ongoing. Remove the entry before
> >>> How can it remove the entry from the list?
> >> Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > => commit message
> >
> >>>> @@ -1898,7 +1897,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);
> >>>> +  }
> >>> Is it expected that userspace tries to unmap it again? Or why is it
> >>> being added to the list?
> >> User process can call unmap and fastrpc library won't call the unmap again.
> > In the other email you wrote that the driver can be used by random apps.
> > So... what happens if userspace unmaps it again? What if the userspace
> > _doesn't_ unmap it (although you've just readded it back)?
>
> If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
> If userspace no longer performs unmap, the driver will not unmap it proactively.
> The Fastrpc driver will free up this list during fastrpc user-free.

It will free the list. But what happens with the memory mapping?

>
> >
> >> Fastrpc driver will free up this list during fastrpc user-free.



-- 
With best wishes
Dmitry

Re: [PATCH v5 4/5] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Jianping Li]

On 5/25/2026 4:32 PM, Dmitry Baryshkov wrote:
> On Fri, May 22, 2026 at 03:47:31PM +0800, Jianping Li wrote:
>> On 5/15/2026 9:38 PM, Dmitry Baryshkov wrote:
>>> On Fri, May 15, 2026 at 08:42:16PM +0800, 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.
>>>>
>>>> Allocate the entire Audio PD reserved-memory region upfront during rpmsg
>>>> probe and tie its lifetime to the rpmsg channel. This avoids userspace-
>>>> controlled alloc/free and ensures memory is reclaimed only when the DSP
>>>> shuts down.
>>> So, is this a bugfix or not? Is it possible to make the kernel misbehave
>>> without this patch being applied?
>> Yes, this is a bug fix.
> The tag, cc:stable, clear description as the bugfix? How would anybody
> guess if the patch is to be backported to earlier kernels or not?

When userspace repeatedly grows the heap, the kernel does not
support shrinking it, leading to unbounded memory usage.
This can eventually cause memory leak.

I will mention this point in the commit message.

Thanks,
Jianping.

>
>> Because currently after the audio PD requests to grow the heap,
>> the current kernel does not support shrinking the heap, which will all cause memory leaks.
>>
>> The current modification is to allocate in advance and release uniformly, which will avoid this error.
>>
>> Thanks,
>> Jianping.
>>
>>>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>>>> ---
>>>>    drivers/misc/fastrpc.c | 107 +++++++++++++++++++----------------------
>>>>    1 file changed, 49 insertions(+), 58 deletions(-)
>>>>

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Greg KH]

On Mon, May 25, 2026 at 11:28:48AM +0300, Dmitry Baryshkov wrote:
> On Mon, May 25, 2026 at 07:06:37AM +0200, Greg KH wrote:
> > On Mon, May 25, 2026 at 11:30:43AM +0800, Jianping Li wrote:
> > > 
> > > On 5/22/2026 3:03 PM, Greg KH wrote:
> > > > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> > > > > On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > > > > > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > > > > > from list while unmap operation is ongoing. Remove the entry before
> > > > > > How can it remove the entry from the list?
> > > > > Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > > > multiple threads sharing the same file descriptor is a horrible
> > > > userspace bug.  If you do that, you get what you deserve :)
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > I agree that concurrent unmap on the same buffer from multiple threads
> > > is a userspace bug.
> > > 
> > > However, the fastrpc driver is exposed via ioctl, and any userspace
> > > (including any random apk) can map directly and then call multiple unmap.
> > 
> > So there is no userspace selinux permissions settings on the fasterpc
> > device node at all?
> > 
> > > This patch is not intended to support incorrect userspace usage,
> > > but to ensure that the driver remains robust against invalid or
> > > racy inputs.
> > 
> > Isn't control to this device an "admin only" thing?
> 
> No, it's being used by normal user apps. Also, there are systems without
> SELinux being enabled / used.

Oh wow, this driver is going to be even more of a problem than I
assumed.  Someone needs to throw an LLM at this and fix all the bugs as
soon as possible then...

good luck!

greg k-h

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Jianping Li]

On 5/25/2026 5:35 PM, Dmitry Baryshkov wrote:
> On Mon, 25 May 2026 at 11:34, Jianping Li <jianping.li@oss.qualcomm.com> wrote:
>>
>> On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
>>> On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
>>>> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
>>>>>> from list while unmap operation is ongoing. Remove the entry before
>>>>> How can it remove the entry from the list?
>>>> Multiple threads sharing the same file descriptor may invoke unmap concurrently.
>>> => commit message
>>>
>>>>>> @@ -1898,7 +1897,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);
>>>>>> +  }
>>>>> Is it expected that userspace tries to unmap it again? Or why is it
>>>>> being added to the list?
>>>> User process can call unmap and fastrpc library won't call the unmap again.
>>> In the other email you wrote that the driver can be used by random apps.
>>> So... what happens if userspace unmaps it again? What if the userspace
>>> _doesn't_ unmap it (although you've just readded it back)?
>> If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
>> If userspace no longer performs unmap, the driver will not unmap it proactively.
>> The Fastrpc driver will free up this list during fastrpc user-free.
> It will free the list. But what happens with the memory mapping?

When device release is called, DSP side PD is also cleaned up, which includes cleaning up of DSP side mappings

Before kref_put of fl, we call DSP process release, where DSP PD is cleaned up.

After calling this, we go ahead and do buf_free of the list

Thanks,
Jianping.

>
>>>> Fastrpc driver will free up this list during fastrpc user-free.
>
>

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Dmitry Baryshkov]

On Tue, May 26, 2026 at 02:59:31PM +0800, Jianping Li wrote:
> 
> On 5/25/2026 5:35 PM, Dmitry Baryshkov wrote:
> > On Mon, 25 May 2026 at 11:34, Jianping Li <jianping.li@oss.qualcomm.com> wrote:
> > > 
> > > On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
> > > > On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
> > > > > On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
> > > > > > On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
> > > > > > > from list while unmap operation is ongoing. Remove the entry before
> > > > > > How can it remove the entry from the list?
> > > > > Multiple threads sharing the same file descriptor may invoke unmap concurrently.
> > > > => commit message
> > > > 
> > > > > > > @@ -1898,7 +1897,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);
> > > > > > > +  }
> > > > > > Is it expected that userspace tries to unmap it again? Or why is it
> > > > > > being added to the list?
> > > > > User process can call unmap and fastrpc library won't call the unmap again.
> > > > In the other email you wrote that the driver can be used by random apps.
> > > > So... what happens if userspace unmaps it again? What if the userspace
> > > > _doesn't_ unmap it (although you've just readded it back)?
> > > If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
> > > If userspace no longer performs unmap, the driver will not unmap it proactively.
> > > The Fastrpc driver will free up this list during fastrpc user-free.
> > It will free the list. But what happens with the memory mapping?
> 
> When device release is called, DSP side PD is also cleaned up, which includes cleaning up of DSP side mappings
> 
> Before kref_put of fl, we call DSP process release, where DSP PD is cleaned up.
> 
> After calling this, we go ahead and do buf_free of the list

Okay, capture this discussion in the commit message.

> 
> Thanks,
> Jianping.
> 
> > 
> > > > > Fastrpc driver will free up this list during fastrpc user-free.
> > 
> > 

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] misc: fastrpc: Remove buffer from list prior to unmap operation

[Jianping Li]

On 5/26/2026 3:06 PM, Dmitry Baryshkov wrote:
> On Tue, May 26, 2026 at 02:59:31PM +0800, Jianping Li wrote:
>> On 5/25/2026 5:35 PM, Dmitry Baryshkov wrote:
>>> On Mon, 25 May 2026 at 11:34, Jianping Li <jianping.li@oss.qualcomm.com> wrote:
>>>> On 5/25/2026 4:30 PM, Dmitry Baryshkov wrote:
>>>>> On Fri, May 22, 2026 at 02:55:29PM +0800, Jianping Li wrote:
>>>>>> On 5/15/2026 9:36 PM, Dmitry Baryshkov wrote:
>>>>>>> On Fri, May 15, 2026 at 08:42:14PM +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 any other thread removes the entry
>>>>>>>> from list while unmap operation is ongoing. Remove the entry before
>>>>>>> How can it remove the entry from the list?
>>>>>> Multiple threads sharing the same file descriptor may invoke unmap concurrently.
>>>>> => commit message
>>>>>
>>>>>>>> @@ -1898,7 +1897,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);
>>>>>>>> +  }
>>>>>>> Is it expected that userspace tries to unmap it again? Or why is it
>>>>>>> being added to the list?
>>>>>> User process can call unmap and fastrpc library won't call the unmap again.
>>>>> In the other email you wrote that the driver can be used by random apps.
>>>>> So... what happens if userspace unmaps it again? What if the userspace
>>>>> _doesn't_ unmap it (although you've just readded it back)?
>>>> If the same buf is unmapped again, because it has already been added back to the list, the unmap logic will be executed again.
>>>> If userspace no longer performs unmap, the driver will not unmap it proactively.
>>>> The Fastrpc driver will free up this list during fastrpc user-free.
>>> It will free the list. But what happens with the memory mapping?
>> When device release is called, DSP side PD is also cleaned up, which includes cleaning up of DSP side mappings
>>
>> Before kref_put of fl, we call DSP process release, where DSP PD is cleaned up.
>>
>> After calling this, we go ahead and do buf_free of the list
> Okay, capture this discussion in the commit message.

Got it, thanks.

I'll update the commit message in the next version patch.

>
>> Thanks,
>> Jianping.
>>
>>>>>> Fastrpc driver will free up this list during fastrpc user-free.
>>>