From 2159430fe26068ca3c89557ddd2d5eb17a5a8cf0 Mon Sep 17 00:00:00 2001 From: Yifei Gao Date: Tue, 11 Aug 2026 22:51:01 +0000 Subject: misc: fastrpc: fix double-free in fastrpc_map_attach() error path map->table is assigned right after dma_buf_map_attachment_unlocked() succeeds. The two failure checks that follow, the len > map->size test and, where subsystem VMIDs are configured, a failed qcom_scm_assign_mem(), jump to map_err with map->table already set. map_err manually calls dma_buf_detach() and dma_buf_put() and then falls through to fastrpc_map_put(). Since that change the error path tail is fastrpc_map_put() -> fastrpc_free_map(), and fastrpc_free_map() already unmaps, detaches and puts the dma-buf whenever map->table is set. The two operations therefore run twice: the second dma_buf_put() drops an extra reference on map->buf, and dma_buf_unmap_attachment_unlocked() dereferences the map->attach already freed by the manual dma_buf_detach(). kref_init() sets the refcount to 1 with no intervening get, so the final fastrpc_map_put() frees the map synchronously and the redundant cleanup is deterministic. The len > map->size branch is reachable by an unprivileged process via FASTRPC_IOCTL_MEM_MAP with an fd whose dma-buf is smaller than the requested length, before any DSP invocation. Route both map->table-is-set failure branches to get_err instead of map_err, so fastrpc_free_map() is the single owner of the unmap/detach/put sequence. map_err is retained for the dma_buf_map_attachment_unlocked() failure, which is reached with map->table still NULL and an attachment that fastrpc_free_map() will not clean up, so its dma_buf_detach()/dma_buf_put() must still run manually. Fixes: 334f1a1cbe03 ("misc: fastrpc: Use fastrpc_map_put in fastrpc_map_create on fail") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Yifei Gao Reviewed-by: Ekansh Gupta Signed-off-by: Srinivas Kandagatla --- drivers/misc/fastrpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 90fd669636ec..4a18081845d9 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -927,7 +927,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd, dev_dbg(sess->dev, "Bad size passed len 0x%llx map size 0x%llx\n", len, map->size); err = -EINVAL; - goto map_err; + goto get_err; } map->va = sg_virt(map->table->sgl); map->len = len; @@ -950,7 +950,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd, dev_err(sess->dev, "Failed to assign memory with dma_addr %pad size 0x%llx err %d\n", &map->dma_addr, map->len, err); - goto map_err; + goto get_err; } } spin_lock(&fl->lock); -- cgit v1.2.3 From 9981fbe599e80d0fa264b714ebf14d8d80d9790a Mon Sep 17 00:00:00 2001 From: Jianping Li Date: Wed, 26 Aug 2026 11:38:03 +0800 Subject: 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. 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. The reserved-memory region is now mandatory for the Audio PD domain. Rather than failing rpmsg probe when it is missing, validate it in fastrpc_init_create_static_process() and reject only the static-process creation. This keeps the fastrpc device probing for all other domains even on a misconfigured device tree. Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd") Cc: stable@kernel.org Signed-off-by: Jianping Li Reviewed-by: Ekansh Gupta Link: https://lore.kernel.org/all/20260814101955.234238-1-jianping.li@oss.qualcomm.com/ Signed-off-by: Srinivas Kandagatla --- drivers/misc/fastrpc.c | 188 ++++++++++++++++++++++++++----------------------- 1 file changed, 101 insertions(+), 87 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 4a18081845d9..9310ef1cb4a6 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -70,8 +70,6 @@ #define ADSP_MMAP_HEAP_ADDR 4 /* MAP static DMA buffer on DSP User PD */ #define ADSP_MMAP_DMA_BUFFER 6 -/* Add memory to static PD pool protection thru hypervisor */ -#define ADSP_MMAP_REMOTE_HEAP_ADDR 8 /* Add memory to userPD pool, for user heap */ #define ADSP_MMAP_ADD_PAGES 0x1000 /* Add memory to userPD pool, for LLC heap */ @@ -314,10 +312,14 @@ 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; + /* Audio PD reserved remote heap region */ + phys_addr_t remote_heap_addr; + u64 remote_heap_size; u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES]; struct fastrpc_device *secure_fdevice; struct fastrpc_device *fdevice; - struct fastrpc_buf *remote_heap; struct list_head invoke_interrupted_mmaps; bool secure; bool unsigned_support; @@ -1454,15 +1456,24 @@ 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; + bool sent_heap = false; + + if (!cctx->remote_heap_addr || !cctx->remote_heap_size) { + err = -ENOMEM; + dev_err(fl->sctx->dev, + "remote heap memory region is not added\n"); + return err; + } args = kzalloc_objs(*args, FASTRPC_CREATE_STATIC_PROCESS_NARGS); if (!args) @@ -1486,31 +1497,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; @@ -1522,8 +1508,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->dma_addr; - pages[0].size = fl->cctx->remote_heap->size; + /* + * Audio PD is a static PD and retains the remote heap + * information across daemon restarts. Therefore only + * the first attach should provide heap information to + * DSP. Subsequent attaches reuse the previously + * initialized memory pool. + */ + spin_lock_irqsave(&cctx->lock, flags); + if (!cctx->audio_init_mem) { + pages[0].addr = cctx->remote_heap_addr; + pages[0].size = cctx->remote_heap_size; + cctx->audio_init_mem = true; + inbuf.pageslen = 1; + sent_heap = true; + } 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); @@ -1541,27 +1544,11 @@ 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); + if (sent_heap) { + spin_lock_irqsave(&cctx->lock, flags); + cctx->audio_init_mem = false; + spin_unlock_irqrestore(&cctx->lock, flags); } -err_map: - fastrpc_buf_free(fl->cctx->remote_heap); - fl->cctx->remote_heap = NULL; -err_name: kfree(name); err: kfree(args); @@ -2090,7 +2077,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) if (copy_from_user(&req, argp, sizeof(req))) return -EFAULT; - if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) { + if (req.flags != ADSP_MMAP_ADD_PAGES) { dev_err(dev, "flag not supported 0x%x\n", req.flags); return -EINVAL; @@ -2101,10 +2088,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) return -EINVAL; } - if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR) - err = fastrpc_remote_heap_alloc(fl, dev, req.size, &buf); - else - err = fastrpc_buf_alloc(fl, dev, req.size, &buf); + err = fastrpc_buf_alloc(fl, dev, req.size, &buf); if (err) { dev_err(dev, "failed to allocate buffer\n"); @@ -2143,20 +2127,6 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) /* let the client know the address to use */ req.vaddrout = rsp_msg.vaddr; - /* Add memory to static PD pool, protection thru hypervisor */ - if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) { - u64 src_perms = BIT(QCOM_SCM_VMID_HLOS); - - err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size, - &src_perms, fl->cctx->vmperms, fl->cctx->vmcount); - if (err) { - dev_err(fl->sctx->dev, - "Failed to assign memory dma_addr %pad size 0x%llx err %d", - &buf->dma_addr, buf->size, err); - goto err_assign; - } - } - spin_lock(&fl->lock); list_add_tail(&buf->node, &fl->mmaps); spin_unlock(&fl->lock); @@ -2537,6 +2507,45 @@ static const struct of_device_id fastrpc_poll_supported_machines[] __maybe_unuse {}, }; +static int fastrpc_init_reserved_mem(struct fastrpc_channel_ctx *cctx, + struct device *rdev, u32 domain_id) +{ + struct resource res; + u64 src_perms; + int err; + + /* Only SDSP and ADSP domains use a reserved remote heap region */ + if (domain_id != SDSP_DOMAIN_ID && domain_id != ADSP_DOMAIN_ID) + return 0; + + err = of_reserved_mem_region_to_resource(rdev->of_node, 0, &res); + if (err) { + /* + * The reserved-memory region is optional at probe time. For + * the Audio PD (ADSP) domain its absence is validated later in + * fastrpc_init_create_static_process(), so keep probing here. + */ + return 0; + } + + /* + * Audio PD (ADSP) is a static PD: cache the region so its lifetime + * is tied to the rpmsg channel instead of being controlled by + * userspace. + */ + if (domain_id == ADSP_DOMAIN_ID) { + cctx->remote_heap_addr = res.start; + cctx->remote_heap_size = resource_size(&res); + } + + if (!cctx->vmcount) + return 0; + + src_perms = BIT(QCOM_SCM_VMID_HLOS); + return qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms, + cctx->vmperms, cctx->vmcount); +} + static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) { struct device *rdev = &rpdev->dev; @@ -2584,21 +2593,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) } } - if (domain_id == SDSP_DOMAIN_ID) { - struct resource res; - u64 src_perms; - - err = of_reserved_mem_region_to_resource(rdev->of_node, 0, &res); - if (!err) { - src_perms = BIT(QCOM_SCM_VMID_HLOS); - - err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms, - data->vmperms, data->vmcount); - if (err) - goto err_free_data; - } - - } + err = fastrpc_init_reserved_mem(data, rdev, domain_id); + if (err) + goto err_free_data; secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain")); data->secure = secure_dsp; @@ -2698,8 +2695,25 @@ 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_size && cctx->vmcount) { + u64 src_perms = 0; + int err, i; + struct qcom_scm_vmperm dst_perms; + + for (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_addr, + cctx->remote_heap_size, &src_perms, + &dst_perms, 1); + if (err) + dev_err(&rpdev->dev, + "Failed to assign memory back to HLOS: addr %pa size %#llx err %d\n", + &cctx->remote_heap_addr, cctx->remote_heap_size, err); + } of_platform_depopulate(&rpdev->dev); -- cgit v1.2.3 From 94778611d71b57ff917de507229e1c58ad9deba7 Mon Sep 17 00:00:00 2001 From: Jianping Li Date: Mon, 7 Sep 2026 15:03:56 +0800 Subject: misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages On some platforms the DSP firmware does not support the fastrpc capability/attribute query. The DSP side returns an error (err: -1) while handling the request, so fastrpc_get_info_from_dsp() does not return DSP_UNSUPPORTED_API but a generic error, and the "else if (err)" branch in fastrpc_get_info_from_kernel() is taken. cctx->valid_attributes is therefore never set and every open()/query re-prints the same "dsp information is incorrect" line. This is not tied to boot timing: it can be reproduced at any time by manually starting adsprpcd. On RC builds with metadata flashing enabled several clients come up and flood the console. As this is an expected, platform-specific and non-fatal condition, downgrade the message to dev_dbg. Similarly, the absence of reserved DMA memory is a valid configuration and does not indicate an error, so downgrade that message to dev_dbg as well. Also rate-limit the "No session available" message with dev_err_ratelimited(): it reflects a genuine failure (open() returns -EBUSY) so it should still be visible, but must not flood dmesg when the session pool is exhausted. No functional change intended. Signed-off-by: Jianping Li Link: https://lore.kernel.org/all/20260514062825.50172-1-jianping.li@oss.qualcomm.com/ Signed-off-by: Srinivas Kandagatla --- drivers/misc/fastrpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 90fd669636ec..f40356c95051 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -1783,7 +1783,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) fl->sctx = fastrpc_session_alloc(fl); if (!fl->sctx) { - dev_err(&cctx->rpdev->dev, "No session available\n"); + dev_err_ratelimited(&cctx->rpdev->dev, "No session available\n"); mutex_destroy(&fl->mutex); kfree(fl); fastrpc_channel_ctx_put(cctx); @@ -1941,7 +1941,7 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap, kfree(dsp_attributes); return -EOPNOTSUPP; } else if (err) { - dev_err(&cctx->rpdev->dev, "Error: dsp information is incorrect err: %d\n", err); + dev_dbg(&cctx->rpdev->dev, "Error: dsp information is incorrect err: %d\n", err); kfree(dsp_attributes); return err; } @@ -2563,7 +2563,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) } if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0)) - dev_info(rdev, "no reserved DMA memory for FASTRPC\n"); + dev_dbg(rdev, "no reserved DMA memory for FASTRPC\n"); vmcount = of_property_read_variable_u32_array(rdev->of_node, "qcom,vmids", &vmids[0], 0, FASTRPC_MAX_VMIDS); -- cgit v1.2.3 From c89a16a5226910d06c0324cbb292cc4d0d7f0325 Mon Sep 17 00:00:00 2001 From: Mukesh Ojha Date: Thu, 3 Sep 2026 01:10:41 +0530 Subject: misc: fastrpc: Use devm_of_reserved_mem_device_init() Use the devres-managed devm_of_reserved_mem_device_init() to ensure the reserved memory region is released on device removal, fixing a missing of_reserved_mem_device_release() in fastrpc_rpmsg_remove(). Reviewed-by: Ekansh Gupta Signed-off-by: Mukesh Ojha Reviewed-by: Konrad Dybcio Signed-off-by: Srinivas Kandagatla --- drivers/misc/fastrpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index f40356c95051..0a3b6321ad0d 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -2562,7 +2562,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) return -EINVAL; } - if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0)) + if (devm_of_reserved_mem_device_init(rdev)) dev_dbg(rdev, "no reserved DMA memory for FASTRPC\n"); vmcount = of_property_read_variable_u32_array(rdev->of_node, -- cgit v1.2.3