* Re: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
2026-07-07 6:17 [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
@ 2026-07-07 6:47 ` sashiko-bot
2026-07-07 7:01 ` Ekansh Gupta
2026-07-07 7:23 ` Srinivas Kandagatla
2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-07 6:47 UTC (permalink / raw)
To: Jianping Li; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Race condition and logic flaw in clearing `cctx->audio_init_mem` allows unsafe double-initialization of the DSP heap.
- [High] Concurrent execution of `FASTRPC_IOCTL_INIT_CREATE_STATIC` and `fastrpc_rpmsg_remove` causes Use-After-Free on `cctx->remote_heap`.
Pre-existing issues:
- [High] Permanent SCM memory leak from HLOS on probe error path.
- [High] SDSP domain SCM memory leak on driver unbind due to missing `cctx->remote_heap` allocation.
- [High] Memory assigned via `ADSP_MMAP_REMOTE_HEAP_ADDR` is freed without restoring HLOS ownership.
- [High] Misc device is registered and exposed to userspace before its channel context is fully initialized.
--
--- Patch [1]: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe ---
commit a011acdb04cd3c4edb5a740628e2c5ac611ab957
Author: Jianping Li <jianping.li@oss.qualcomm.com>
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. This allocates the entire Audio PD reserved-memory region during
rpmsg probe and ties its lifetime to the rpmsg channel.
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d86e79134c681..1ec37e6c68a28 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1344,15 +1346,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;
> 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) {
[Severity: High]
Can this code race with fastrpc_rpmsg_remove()?
If the rpmsg channel is removed via driver unbind or DSP crash,
fastrpc_rpmsg_remove() frees cctx->remote_heap.
Since these pointers are dereferenced here without taking cctx->lock, could
this lead to a use-after-free if FASTRPC_IOCTL_INIT_CREATE_STATIC is called
concurrently?
> + 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);
[ ... ]
> return 0;
> err_invoke:
> + cctx->audio_init_mem = false;
[Severity: High]
Does this code unconditionally clear the initialization flag on any failure?
If fastrpc_internal_invoke() fails for any caller (like a timeout or bad
argument), this sets cctx->audio_init_mem to false without checking if the
current caller was the one that successfully set it to true.
Could this allow a subsequent caller to re-send the heap coordinates, leading
to an unsafe double-initialization of the DSP heap?
> kfree(name);
> err:
[ ... ]
> @@ -2418,12 +2400,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);
[Severity: High]
This is a pre-existing issue, but does this allocation strictly for ADSP
cause an SDSP memory leak?
Later in fastrpc_rpmsg_remove(), the newly added HLOS unassignment logic is
guarded by:
if (cctx->remote_heap && cctx->vmcount)
Since data->remote_heap is never allocated for SDSP, it will be NULL and
the SCM unassignment will be silently skipped, permanently leaking the SDSP
reserved memory to the DSP on unbind.
> + 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);
[ ... ]
> @@ -2490,6 +2482,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;
> }
[Severity: High]
This is a pre-existing issue, but does this error path permanently leak
TrustZone memory?
If a subsequent step in fastrpc_rpmsg_probe() fails (such as
fastrpc_device_register() or of_platform_populate()), the code jumps here
to free the structures.
However, qcom_scm_assign_mem() was already called to transfer ownership to
the DSP VMIDs, and there is no reverse call here to restore memory ownership
back to HLOS.
[Severity: High]
This is a pre-existing issue, but does fastrpc_rpmsg_probe() expose the
misc device before initialization is complete?
Looking at fastrpc_rpmsg_probe():
err = fastrpc_device_register(rdev, data, secure_dsp, domain);
[ ... ]
kref_init(&data->refcount);
[ ... ]
INIT_LIST_HEAD(&data->users);
[ ... ]
spin_lock_init(&data->lock);
idr_init(&data->ctx_idr);
fastrpc_device_register() makes the file node instantly visible to userspace.
If a user process opens it immediately, could it access uninitialized
data structures like data->lock or data->users in fastrpc_device_open()?
[Severity: High]
This is a pre-existing issue, but does fastrpc_req_munmap_impl() return memory
to the kernel allocator while it is still owned by TrustZone?
When fastrpc_req_mmap() handles ADSP_MMAP_REMOTE_HEAP_ADDR, it assigns the
memory to the DSP:
err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
However, the unmap path in fastrpc_req_munmap_impl() calls
fastrpc_buf_free() which runs dma_free_coherent(). It doesn't appear to
invoke qcom_scm_assign_mem() to revert permissions back to HLOS before
freeing.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707061733.495-1-jianping.li@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
2026-07-07 6:17 [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
2026-07-07 6:47 ` sashiko-bot
@ 2026-07-07 7:01 ` Ekansh Gupta
2026-07-07 8:32 ` Jianping Li
2026-07-07 7:23 ` Srinivas Kandagatla
2 siblings, 1 reply; 6+ messages in thread
From: Ekansh Gupta @ 2026-07-07 7:01 UTC (permalink / raw)
To: Jianping Li, Srinivas Kandagatla
Cc: Arnd Bergmann, Greg Kroah-Hartman, Abel Vesa, linux-arm-msm,
dri-devel, linux-kernel, quic_chennak, stable
On 07-07-2026 11:47, 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: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
Changes in vN and lore link to v(N-1) series would help here.>
drivers/misc/fastrpc.c | 119 +++++++++++++++++++++++------------------
> 1 file changed, 66 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d86e79134c68..1ec37e6c68a2 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;
> @@ -1344,15 +1346,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)
> @@ -1376,31 +1387,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;
>
> @@ -1412,8 +1398,24 @@ 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->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;
> + }
> + spin_unlock_irqrestore(&cctx->lock, flags);
>
> args[2].ptr = (u64)(uintptr_t) pages;
> args[2].length = sizeof(*pages);
> @@ -1431,27 +1433,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;
> kfree(name);
> err:
> kfree(args);
> @@ -2418,12 +2400,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);
drop GFP_KERNEL from kzalloc_obj()> + 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,
> @@ -2431,7 +2424,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"));
> @@ -2490,6 +2482,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;
> }
> @@ -2512,6 +2505,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);
> @@ -2529,8 +2523,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++)
for the iterator use top-of-block declaration style for consistency> +
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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
2026-07-07 7:01 ` Ekansh Gupta
@ 2026-07-07 8:32 ` Jianping Li
0 siblings, 0 replies; 6+ messages in thread
From: Jianping Li @ 2026-07-07 8:32 UTC (permalink / raw)
To: Ekansh Gupta, Srinivas Kandagatla
Cc: arnd, Greg KH, abelvesa, linux-arm-msm, dri-devel, linux-kernel,
quic_chennak, stable
On 7/7/2026 3:01 PM, Ekansh Gupta wrote:
> On 07-07-2026 11:47, 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: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
> Changes in vN and lore link to v(N-1) series would help here.>
> drivers/misc/fastrpc.c | 119 +++++++++++++++++++++++------------------
Will add changelog and lore link in next version.
>> 1 file changed, 66 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index d86e79134c68..1ec37e6c68a2 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;
>> @@ -1344,15 +1346,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)
>> @@ -1376,31 +1387,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;
>>
>> @@ -1412,8 +1398,24 @@ 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->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;
>> + }
>> + spin_unlock_irqrestore(&cctx->lock, flags);
>>
>> args[2].ptr = (u64)(uintptr_t) pages;
>> args[2].length = sizeof(*pages);
>> @@ -1431,27 +1433,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;
>> kfree(name);
>> err:
>> kfree(args);
>> @@ -2418,12 +2400,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);
> drop GFP_KERNEL from kzalloc_obj()> + if (!data->remote_heap) {
ACK,I'll drop GFP_KERNEL.
>> + 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,
>> @@ -2431,7 +2424,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"));
>> @@ -2490,6 +2482,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;
>> }
>> @@ -2512,6 +2505,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);
>> @@ -2529,8 +2523,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++)
> for the iterator use top-of-block declaration style for consistency> +
> src_perms |= BIT(cctx->vmperms[i].vmid);
Will fix.
>> +
>> + 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);
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
2026-07-07 6:17 [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe Jianping Li
2026-07-07 6:47 ` sashiko-bot
2026-07-07 7:01 ` Ekansh Gupta
@ 2026-07-07 7:23 ` Srinivas Kandagatla
2026-07-07 11:20 ` Jianping Li
2 siblings, 1 reply; 6+ messages in thread
From: Srinivas Kandagatla @ 2026-07-07 7:23 UTC (permalink / raw)
To: Jianping Li, Srinivas Kandagatla, Ekansh Gupta
Cc: Arnd Bergmann, Greg Kroah-Hartman, Abel Vesa, linux-arm-msm,
dri-devel, linux-kernel, quic_chennak, stable
On 7/7/26 7:17 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: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
> drivers/misc/fastrpc.c | 119 +++++++++++++++++++++++------------------
> 1 file changed, 66 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d86e79134c68..1ec37e6c68a2 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;
> @@ -1344,15 +1346,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) {
if remote heap is a mandatory for ADSP or SDSP then this should be
enforced at binding level and even at driver probe level.
> + 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)
> @@ -1376,31 +1387,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;
>
> @@ -1412,8 +1398,24 @@ 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->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;
> + }
> + spin_unlock_irqrestore(&cctx->lock, flags);
>
> args[2].ptr = (u64)(uintptr_t) pages;
> args[2].length = sizeof(*pages);
> @@ -1431,27 +1433,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;
You are ignoring my comments that I have provided in the last version.
> kfree(name);
> err:
> kfree(args);
> @@ -2418,12 +2400,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,
> @@ -2431,7 +2424,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"));
> @@ -2490,6 +2482,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;
> }
> @@ -2512,6 +2505,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);
> @@ -2529,8 +2523,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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v9] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
2026-07-07 7:23 ` Srinivas Kandagatla
@ 2026-07-07 11:20 ` Jianping Li
0 siblings, 0 replies; 6+ messages in thread
From: Jianping Li @ 2026-07-07 11:20 UTC (permalink / raw)
To: Srinivas Kandagatla, Ekansh Gupta
Cc: arnd, Greg KH, abelvesa, linux-arm-msm, dri-devel, linux-kernel,
quic_chennak, stable
On 7/7/2026 3:23 PM, Srinivas Kandagatla wrote:
>
> On 7/7/26 7:17 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: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>> drivers/misc/fastrpc.c | 119 +++++++++++++++++++++++------------------
>> 1 file changed, 66 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index d86e79134c68..1ec37e6c68a2 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;
>> @@ -1344,15 +1346,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) {
> if remote heap is a mandatory for ADSP or SDSP then this should be
> enforced at binding level and even at driver probe level.
Agreed. In v10 I will:
- Move the enforcement into fastrpc_rpmsg_probe(): if
of_reserved_mem_region_to_resource() fails for an ADSP
node, probe fails up-front instead of deferring the error to the
user-space attach path.
- Drop this runtime check in fastrpc_init_create_static_process()
entirely, since the invariant is now established at probe time.
>
>> + 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)
>> @@ -1376,31 +1387,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;
>>
>> @@ -1412,8 +1398,24 @@ 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->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;
>> + }
>> + spin_unlock_irqrestore(&cctx->lock, flags);
>>
>> args[2].ptr = (u64)(uintptr_t) pages;
>> args[2].length = sizeof(*pages);
>> @@ -1431,27 +1433,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;
> You are ignoring my comments that I have provided in the last version.
Apologies for the delay in addressing this — you're right, the
unconditional reset is buggy. The concrete failure mode is: 1. First
invoke sends pages=(phys, size) and succeeds. DSP now owns the heap,
audio_init_mem = true. 2. Second invoke sees the flag set, sends
pages=(0, 0), and fails. Because no pages were added in this call, the
DSP does not drop the region on its side. 3. Current code resets
audio_init_mem to false unconditionally, so the third invoke re-sends
(phys, size) and the same region gets registered with the DSP twice.
Fixed in v10 by only clearing the flag when this invocation was the one
that actually carried the heap information: err_invoke: if
(pages[0].addr) cctx->audio_init_mem = false; That is: - first invoke
failed -> pages[0].addr != 0 -> clear the flag, allow a retry to re-send
the region. - later invoke failed with (0, 0) -> pages[0].addr == 0 ->
leave the flag as-is, so we don't re-register the same region. Please
let me know if you'd prefer a stronger scheme.
>
>> kfree(name);
>> err:
>> kfree(args);
>> @@ -2418,12 +2400,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,
>> @@ -2431,7 +2424,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"));
>> @@ -2490,6 +2482,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;
>> }
>> @@ -2512,6 +2505,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);
>> @@ -2529,8 +2523,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);
>>
^ permalink raw reply [flat|nested] 6+ messages in thread