From 10c92d28b23feaf64886283212eda87ffc6347ef Mon Sep 17 00:00:00 2001 From: Ekansh Gupta Date: Tue, 13 May 2025 09:58:23 +0530 Subject: [PATCH 1/4] FROMLIST: misc: fastrpc: Fix initial memory allocation for Audio PD memory pool 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. Link: https://lore.kernel.org/all/20260407100508.1027-2-jianping.li@oss.qualcomm.com/ Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd") Cc: stable@kernel.org Co-developed-by: Ekansh Gupta Signed-off-by: Ekansh Gupta Signed-off-by: Jianping Li --- 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 c4019db14ac4e..7c5bc88b3d12e 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -1423,7 +1423,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); @@ -1444,12 +1446,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; From 5d08c5ac051a0ccd85835070769d4d0a2cef8987 Mon Sep 17 00:00:00 2001 From: Ekansh Gupta Date: Tue, 13 May 2025 09:58:24 +0530 Subject: [PATCH 2/4] FROMLIST: misc: fastrpc: Remove buffer from list prior to unmap operation 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. Link: https://lore.kernel.org/all/20260409062617.1182-3-jianping.li@oss.qualcomm.com/ Fixes: 2419e55e532de ("misc: fastrpc: add mmap/unmap support") Cc: stable@kernel.org Co-developed-by: Ekansh Gupta Signed-off-by: Ekansh Gupta Signed-off-by: Jianping Li --- 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 7c5bc88b3d12e..ee3db5e20098e 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -1958,9 +1958,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); @@ -1974,6 +1971,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; @@ -1981,6 +1979,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; } @@ -1993,7 +1992,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) @@ -2083,14 +2089,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); From 3f6279ade6ce5bdfba78ab56c8fd336fcd99848d Mon Sep 17 00:00:00 2001 From: Jianping Li Date: Tue, 23 Dec 2025 15:50:59 +0800 Subject: [PATCH 3/4] FROMLIST: misc: fastrpc: Allocate entire reserved memory for Audio PD in probe 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. Link: https://lore.kernel.org/all/20260409062617.1182-4-jianping.li@oss.qualcomm.com/ Signed-off-by: Jianping Li --- drivers/misc/fastrpc.c | 101 +++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index ee3db5e20098e..2f62b92cbab00 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -294,6 +294,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; @@ -1394,15 +1396,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 = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); if (!args) @@ -1426,29 +1429,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->phys, - (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 phys 0x%llx size 0x%llx err %d\n", - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err); - goto err_map; - } - scm_done = true; - inbuf.pageslen = 1; - } - } fl->pd = USER_PD; @@ -1460,8 +1440,25 @@ 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->phys; - pages[0].size = fl->cctx->remote_heap->size; + 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; + } + spin_unlock_irqrestore(&cctx->lock, flags); args[2].ptr = (u64)(uintptr_t) pages; args[2].length = sizeof(*pages); @@ -1479,26 +1476,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->phys, - (u64)fl->cctx->remote_heap->size, - &src_perms, &dst_perms, 1); - if (err) - dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n", - fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err); - } -err_map: - fastrpc_buf_free(fl->cctx->remote_heap); -err_name: + fl->cctx->audio_init_mem = false; kfree(name); err: kfree(args); @@ -2465,7 +2443,7 @@ 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; @@ -2476,7 +2454,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms, data->vmperms, data->vmcount); } - + if (domain_id == ADSP_DOMAIN_ID) { + data->remote_heap = + kzalloc(sizeof(*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); + } } secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain")); @@ -2556,6 +2542,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); @@ -2573,8 +2560,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) + fastrpc_buf_free(cctx->remote_heap); + } of_platform_depopulate(&rpdev->dev); From eda869aa04255157f8011080449eb3890405e404 Mon Sep 17 00:00:00 2001 From: Ekansh Gupta Date: Tue, 13 May 2025 09:58:21 +0530 Subject: [PATCH 4/4] FROMLIST: misc: fastrpc: Allow fastrpc_buf_free() to accept NULL Make fastrpc_buf_free() a no-op when passed a NULL pointer, allowing callers to avoid open-coded NULL checks. Link: https://lore.kernel.org/all/20260409062617.1182-5-jianping.li@oss.qualcomm.com/ Co-developed-by: Ekansh Gupta Signed-off-by: Ekansh Gupta Signed-off-by: Jianping Li --- 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 2f62b92cbab00..3d88e7e7a2ece 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_PHYS(buf->phys)); kfree(buf); @@ -533,8 +536,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 >> 8); @@ -1664,8 +1666,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);