On 14-08-2026 15:49, 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.
>
> 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: [email protected]
> Signed-off-by: Jianping Li <[email protected]>
> ---
> Patch [v11]:
> https://lore.kernel.org/all/[email protected]/
>
> Changes in v12:
> - Do not fail rpmsg probe when the reserved-memory region is missing,
> validate the region in fastrpc_init_create_static_process() instead,
> so probe keeps working for all domains.
> - Add fastrpc_domain_has_reserved_heap() / fastrpc_domain_uses_static_heap()
> helpers to replace the open-coded ADSP/SDSP domain checks.
>
> Changes in v11:
> - Replace the remote_heap fastrpc_buf pointer with dedicated
> remote_heap_addr and remote_heap_size fields in
> fastrpc_channel_ctx to avoid leaving a partially
> initialized fastrpc_buf.
>
> - Drop ADSP_MMAP_REMOTE_HEAP_ADDR support from
> fastrpc_req_mmap() since the user process should no longer
> grow or shrink the Audio PD remote heap.
>
> Changes in v10:
> - Move Audio PD remote heap validation into
> fastrpc_rpmsg_probe().
>
> - Treat Audio PD remote heap as a mandatory
> resource and fail probe if the reserved
> memory region is missing.
>
> Changes in v9:
> - Make sure fastrpc_init_create_static_process()
> only sets audio_init_mem to false when the sent
> address is actually invalid.
> ---
> drivers/misc/fastrpc.c | 150 +++++++++++++++++++++--------------------
> 1 file changed, 76 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 90fd669636ec..3f14a4673698 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,16 @@ static const struct of_device_id
> fastrpc_poll_supported_machines[] __maybe_unuse
> {},
> };
>
> +static bool fastrpc_domain_has_reserved_heap(u32 domain_id)
> +{
> + return domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID;
> +}
> +
> +static bool fastrpc_domain_uses_static_heap(u32 domain_id)
> +{
> + return domain_id == ADSP_DOMAIN_ID;
> +}
any reason to have functions for one time used checks?> +
> static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> {
> struct device *rdev = &rpdev->dev;
> @@ -2584,20 +2564,25 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device
> *rpdev)
> }
> }
>
> - if (domain_id == SDSP_DOMAIN_ID) {
> + if (fastrpc_domain_has_reserved_heap(domain_id)) {
> struct resource res;
> u64 src_perms;
>
> err = of_reserved_mem_region_to_resource(rdev->of_node, 0,
> &res);
> if (!err) {
> + if (fastrpc_domain_uses_static_heap(domain_id)) {
> + data->remote_heap_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,
> data->vmperms, data->vmcount);
better to check vmcount before calling this> if (err)
> goto err_free_data;
> + } else {
> + err = 0;
> }
> -
> }
>
> secure_dsp = !(of_property_read_bool(rdev->of_node,
> "qcom,non-secure-domain"));
> @@ -2681,6 +2666,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device
> *rpdev)
> struct fastrpc_buf *buf, *b;
> struct fastrpc_user *user;
> unsigned long flags;
> + int err, i;
>
> /* No invocations past this point */
> spin_lock_irqsave(&cctx->lock, flags);
> @@ -2698,8 +2684,24 @@ 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;
> + 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);
>
