On 02-07-2026 02:07, Srinivas Kandagatla wrote:
>
>
> On 6/9/26 3:59 AM, 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.
>>
>> Add explicit validation for remote_heap presence and size before sending
>> the memory to DSP, and fail early if the reserved-memory region is
>> missing or incomplete.
>>
>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>> Cc: [email protected]
>> Signed-off-by: Jianping Li <[email protected]>
>> ---
>> drivers/misc/fastrpc.c | 112 ++++++++++++++++++++++-------------------
>> 1 file changed, 59 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 517884000331..1942e74535e5 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;
>> @@ -1341,15 +1343,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;
>> +
>> + if (!cctx->remote_heap || !cctx->remote_heap->dma_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)
>> @@ -1373,31 +1384,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;
>>
>> @@ -1409,8 +1395,17 @@ 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;
>> + spin_lock_irqsave(&cctx->lock, flags);
>> + if (!cctx->audio_init_mem) {
>> + pages[0].addr = cctx->remote_heap->dma_addr;
>> + pages[0].size = cctx->remote_heap->size;
>> + cctx->audio_init_mem = true;
>> + inbuf.pageslen = 1;
>> + } else {
>> + pages[0].addr = 0;
>> + pages[0].size = 0;
>
> What is the expected behavoiur in this case?
Audio daemon is expected to take the memory information to DSP audio PD
only the first time it goes and attaches there.
This is the case where daemon was killed but audio PD session is still
running. In such cases, daemon is not expected to take any memory
information to audio PD as the earlier shared information is already
there with audio PD which it is using irrespective of daemon state.>
>> + }
>> + spin_unlock_irqrestore(&cctx->lock, flags);
>>
>> args[2].ptr = (u64)(uintptr_t) pages;
>> args[2].length = sizeof(*pages);
>> @@ -1428,27 +1423,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:
>> + cctx->audio_init_mem = false;
> this can race.
>
>> kfree(name);
>> err:
>> kfree(args);
>> @@ -2415,12 +2390,23 @@ 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) {
>> + err = -ENOMEM;
>> + goto err_free_data;
>> + }
>> +
>> + 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,
>> @@ -2428,7 +2414,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"));
>> @@ -2487,6 +2472,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device
>> *rpdev)
>> misc_deregister(&data->secure_fdevice->miscdev);
>>
>> err_free_data:
>> + kfree(data->remote_heap);
>> kfree(data);
>> return err;
>> }
>> @@ -2509,6 +2495,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);
>> @@ -2526,8 +2513,27 @@ 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)
>> + dev_err(&rpdev->dev,
>> + "Failed to assign memory back to HLOS: dma_addr
>> %pad size %#llx err %d\n",
>> + &cctx->remote_heap->dma_addr,
>> cctx->remote_heap->size, err);
>> + }
>> +
>> + kfree(cctx->remote_heap);
>> + cctx->remote_heap = NULL;
>>
>> of_platform_depopulate(&rpdev->dev);
>>
>
