On 24-08-2026 14:19, Jianping Li wrote:
>
> On 8/20/2026 1:10 PM, Ekansh Gupta wrote:
>> 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/20260731093210.473-1-
>>> [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?> +
>
> Just to give some context: I added these helpers based on the v11
> review, which requested replacing the open-coded ADSP/SDSP checks
> with functions describing the per-domain capabilities.
>
> In this patch, they currently have a single caller each.
> Therefore, if you feel it's unnecessary, I can revert
> to the version without helpers.
move the reserved memory initialization block to a new helper function,
something like fastrpc_init_reserved_mem()>
>>> 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)
>
> Agreed. I'll wrap the qcom_scm_assign_mem() call in if (data->vmcount) in v13.
>
>>> 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);
>>>
