All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free
@ 2026-07-02  2:21 Eddie Lin
  2026-07-02  2:33 ` sashiko-bot
  2026-07-02  3:56 ` Ekansh Gupta
  0 siblings, 2 replies; 3+ messages in thread
From: Eddie Lin @ 2026-07-02  2:21 UTC (permalink / raw)
  To: Srinivas Kandagatla, Amol Maheshwari, Arnd Bergmann,
	Greg Kroah-Hartman
  Cc: linux-arm-msm, dri-devel, linux-kernel, stable, Eddie Lin

The 'ctx_idr' is initialized but never destroyed when
the channel context is freed, leading to a memory leak.
Add idr_destroy() to properly clean up the IDR resources.

Fixes: f6f9279f2bf0 ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
Cc: stable@vger.kernel.org
Signed-off-by: Eddie Lin <eddie.lin@oss.qualcomm.com>
---
Changes in v3:
- Remove duplicate description from cover letter.
- Link to v2: https://patch.msgid.link/20260617-fastrpc-cctx-cleanup-v2-1-be87c021114a@oss.qualcomm.com

Changes in v2:
- Added Fixes tag.
- Added Cc: stable@vger.kernel.org.
- Removed duplicate description from cover letter.
- Link to v1: https://patch.msgid.link/20260611-fastrpc-cctx-cleanup-v1-1-28097444116c@oss.qualcomm.com
---
 drivers/misc/fastrpc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a9b2ae44c06f..7727850e9240 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -492,6 +492,7 @@ static void fastrpc_channel_ctx_free(struct kref *ref)
 
 	cctx = container_of(ref, struct fastrpc_channel_ctx, refcount);
 
+	idr_destroy(&cctx->ctx_idr);
 	kfree(cctx);
 }
 

---
base-commit: abe651837cb394f76d738a7a747322fca3bf17ba
change-id: 20260611-fastrpc-cctx-cleanup-bfd20aa7b8a0

Best regards,
--  
Eddie Lin <eddie.lin@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free
  2026-07-02  2:21 [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free Eddie Lin
@ 2026-07-02  2:33 ` sashiko-bot
  2026-07-02  3:56 ` Ekansh Gupta
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-02  2:33 UTC (permalink / raw)
  To: Eddie Lin; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:

Pre-existing issues:
- [High] DMA buffers moved to `invoke_interrupted_mmaps` are permanently leaked.
- [High] Premature exposure of the misc device in probe allows uninitialized access, Use-After-Free, and Double Free.
- [High] Concurrent device removal causes a NULL pointer dereference in `fastrpc_invoke_send`.
- [High] Potential Use-After-Free of `cctx->remote_heap` after `fastrpc_rpmsg_remove` frees it.
- [High] Maps allocated for handle arguments are never freed during context teardown.
- [High] Memory assigned to the DSP hypervisor is returned to the Linux page allocator without restoring permissions, causing an XPU fault.
- [High] A double free of dmabuf attachments and refcount underflow of `dma_buf` occurs in the error path of `fastrpc_map_attach`.
- [Medium] Double allocation of `remote_heap` due to missing synchronization.
- [Medium] The channel context refcount is leaked in the error path of `fastrpc_device_open`.
--

commit 7b667747a11655158ab5dcea5700acfc6656da6d
Author: Eddie Lin <eddie.lin@oss.qualcomm.com>

misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free
    
The 'ctx_idr' is initialized but never destroyed when
the channel context is freed, leading to a memory leak.
Add idr_destroy() to properly clean up the IDR resources.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index f3a49384586d1..1a35380a1557a 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -492,6 +492,7 @@ static void fastrpc_channel_ctx_free(struct kref *ref)
>  
>  	cctx = container_of(ref, struct fastrpc_channel_ctx, refcount);
>  
> +	idr_destroy(&cctx->ctx_idr);
>  	kfree(cctx);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the driver permanently
leak dma buffers moved to invoke_interrupted_mmaps?

In fastrpc_internal_invoke(), buffers are moved to the interrupted list to
prevent them from being freed while the dsp is active:

drivers/misc/fastrpc.c:fastrpc_internal_invoke() {
    ...
    list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps);
    ...
}

However, during device teardown in fastrpc_rpmsg_remove(), the list nodes
are unlinked but the associated memory appears to be abandoned, without
calling fastrpc_buf_free():

drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() {
    ...
    list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
        list_del(&buf->node);
    ...
}

Could a user space program exhaust system dma memory by repeatedly triggering
signals during invocation?

[Severity: High]
This is a pre-existing issue, but does the premature exposure of the misc
device in probe allow uninitialized access or a use-after-free?

In fastrpc_rpmsg_probe(), the misc device is registered before the subsequent
kref_init(&data->refcount) is called:

drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
    ...
    err = fastrpc_device_register(rdev, data, true, domain);
    ...
}

If probe subsequently fails, the structure is freed unconditionally without
checking if an open file has taken a reference to it:

drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() {
    ...
    kfree(data);
    ...
}

Could a racing userspace open access the uninitialized refcount via
fastrpc_device_open() and continue to issue ioctls against freed memory if
probe aborts?

[Severity: High]
This isn't a bug introduced by this patch, but can concurrent device removal
cause a null pointer dereference?

In fastrpc_invoke_send(), the cctx->rpdev pointer is dereferenced without
holding locks:

drivers/misc/fastrpc.c:fastrpc_invoke_send() {
    ...
    ret = rpmsg_send(cctx->rpdev->ept, (void *)msg, sizeof(*msg));
    ...
}

Concurrently, fastrpc_rpmsg_remove() sets cctx->rpdev to null while ioctls
might still be running:

drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() {
    ...
    cctx->rpdev = NULL;
    ...
}

Could this cause a kernel panic if user space ioctls run concurrently with a
device unbind or module unload event?

[Severity: High]
This is a pre-existing issue, but is there a potential use-after-free of
cctx->remote_heap?

In fastrpc_rpmsg_remove(), the buffer is freed but leaves cctx->remote_heap
as a dangling pointer:

drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() {
    ...
    if (cctx->remote_heap)
        fastrpc_buf_free(cctx->remote_heap);
    ...
}

Lingering open file descriptors can execute
fastrpc_init_create_static_process(), pass the !fl->cctx->remote_heap check,
and dereference the dangling pointer's fields:

drivers/misc/fastrpc.c:fastrpc_init_create_static_process() {
    ...
    pages[0].addr = fl->cctx->remote_heap->dma_addr;
    ...
}

Could userspace intentionally hold an open file descriptor after the device
is removed to trigger memory corruption?

[Severity: Medium]
This isn't a bug introduced by this patch, but does missing synchronization
lead to a double allocation of remote_heap?

In fastrpc_init_create_static_process(), the check and allocation are
unsynchronized:

drivers/misc/fastrpc.c:fastrpc_init_create_static_process() {
    ...
    if (!fl->cctx->remote_heap) {
        err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
                                        &fl->cctx->remote_heap);
    ...
}

Can multiple concurrent threads pass the condition and allocate memory
simultaneously, leaking the dma buffer?

[Severity: High]
This is a pre-existing issue, but are maps allocated for handle arguments
abandoned during context teardown?

In fastrpc_create_maps(), maps for handle arguments are created up to
ctx->nscalars (where indices >= nbufs represent handle arguments):

drivers/misc/fastrpc.c:fastrpc_create_maps() {
    ...
    else
        err = fastrpc_map_attach(ctx->fl, ctx->args[i].fd,
                                 ctx->args[i].length, ctx->args[i].attr,
                                 &ctx->maps[i]);
    ...
}

However, in fastrpc_context_free(), the cleanup loop terminates early at
nbufs instead of nscalars, leaving the attached map pointers:

drivers/misc/fastrpc.c:fastrpc_context_free() {
    ...
    for (i = 0; i < ctx->nbufs; i++)
        fastrpc_map_put(ctx->maps[i]);
    ...
}

Can userspace continually leak map structures and dma_buf references by
providing handle arguments?

[Severity: High]
This isn't a bug introduced by this patch, but is memory assigned to the
dsp hypervisor returned to the Linux page allocator without restoring
permissions?

In fastrpc_rpmsg_remove(), the remote_heap buffer is freed without reclaiming
scm permissions:

drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() {
    ...
    if (cctx->remote_heap)
        fastrpc_buf_free(cctx->remote_heap);
    ...
}

Similarly, in fastrpc_req_munmap_impl(), mapped user buffers are freed
without reclaiming scm permissions:

drivers/misc/fastrpc.c:fastrpc_req_munmap_impl() {
    ...
    fastrpc_buf_free(buf);
    ...
}

Will returning memory with stale xpu protections to the system page allocator
cause a system crash via hypervisor access control exceptions?

[Severity: High]
This is a pre-existing issue, but does a double free of dmabuf attachments
and a refcount underflow occur in the error path of fastrpc_map_attach()?

The error path performs manual detach and put operations before handing off
to the standard cleanup function which repeats them since map->table was left
non-null:

drivers/misc/fastrpc.c:fastrpc_map_attach() {
    ...
map_err:
    dma_buf_detach(map->buf, map->attach);
attach_err:
    dma_buf_put(map->buf);
get_err:
    fastrpc_map_put(map);
    ...
}

If an error occurs after map->table is populated, fastrpc_free_map() executes
its teardown logic and repeats the same dma_buf_detach() and dma_buf_put()
calls on the same pointers.

Does this lead to memory corruption?

[Severity: Medium]
This isn't a bug introduced by this patch, but is the channel context
refcount leaked in the error path of fastrpc_device_open()?

The error path exits early without dropping the cctx reference:

drivers/misc/fastrpc.c:fastrpc_device_open() {
    ...
    if (!fl->sctx) {
        dev_err(&cctx->rpdev->dev, "No session available\n");
        mutex_destroy(&fl->mutex);
        kfree(fl);

        return -EBUSY;
    }
    ...
}

If fastrpc_session_alloc() fails, does the function forget to call
fastrpc_channel_ctx_put() to balance the fastrpc_channel_ctx_get() called
earlier, allowing a user to exhaust the session count and leak references?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-fastrpc-cctx-cleanup-v3-1-3a73c2e4ce1a@oss.qualcomm.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free
  2026-07-02  2:21 [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free Eddie Lin
  2026-07-02  2:33 ` sashiko-bot
@ 2026-07-02  3:56 ` Ekansh Gupta
  1 sibling, 0 replies; 3+ messages in thread
From: Ekansh Gupta @ 2026-07-02  3:56 UTC (permalink / raw)
  To: Eddie Lin, Srinivas Kandagatla, Amol Maheshwari, Arnd Bergmann,
	Greg Kroah-Hartman
  Cc: linux-arm-msm, dri-devel, linux-kernel, stable

On 02-07-2026 07:51, Eddie Lin wrote:
> The 'ctx_idr' is initialized but never destroyed when
> the channel context is freed, leading to a memory leak.
> Add idr_destroy() to properly clean up the IDR resources.
> 
> Fixes: f6f9279f2bf0 ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eddie Lin <eddie.lin@oss.qualcomm.com>
> ---
> Changes in v3:
> - Remove duplicate description from cover letter.
> - Link to v2: https://patch.msgid.link/20260617-fastrpc-cctx-cleanup-v2-1-be87c021114a@oss.qualcomm.com
> 
> Changes in v2:
> - Added Fixes tag.
> - Added Cc: stable@vger.kernel.org.
> - Removed duplicate description from cover letter.
> - Link to v1: https://patch.msgid.link/20260611-fastrpc-cctx-cleanup-v1-1-28097444116c@oss.qualcomm.com
> ---
>  drivers/misc/fastrpc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index a9b2ae44c06f..7727850e9240 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -492,6 +492,7 @@ static void fastrpc_channel_ctx_free(struct kref *ref)
>  
>  	cctx = container_of(ref, struct fastrpc_channel_ctx, refcount);
>  
> +	idr_destroy(&cctx->ctx_idr);
>  	kfree(cctx);
>  }
>  
Reviewed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> 
> ---
> base-commit: abe651837cb394f76d738a7a747322fca3bf17ba
> change-id: 20260611-fastrpc-cctx-cleanup-bfd20aa7b8a0
> 
> Best regards,
> --  
> Eddie Lin <eddie.lin@oss.qualcomm.com>
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-02  3:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02  2:21 [PATCH v3] misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free Eddie Lin
2026-07-02  2:33 ` sashiko-bot
2026-07-02  3:56 ` Ekansh Gupta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.