[PATCH] misc: fastrpc: fix use-after-free of fastrpc_user in dma_buf release

[PATCH] misc: fastrpc: fix use-after-free of fastrpc_user in dma_buf release

[Loic Poulain]

When a process closes its fastrpc device file descriptor, the release
function drops the fl refcount via fastrpc_user_put(). If that was the
last reference, fastrpc_user_free() runs immediately and frees fl.

However, any dma_buf exported via fastrpc_dmabuf_alloc() stores a raw
buf->fl pointer without holding a reference on fl. If the dma_buf
outlives the device fd (e.g. passed to another process, or simply
released later via delayed_fput), the dma_buf .release callback fires
after fl is already freed.

Then accessing the now invalid buf->fl triggers a kernel oops:

Unable to handle kernel paging request at virtual address 000103f6a9025bf5
  Mem abort info:
    ESR = 0x0000000096000004
    EC = 0x25: DABT (current EL), IL = 32 bits
    FSC = 0x04: level 0 translation fault
  [000103f6a9025bf5] address between user and kernel address ranges
  Internal error: Oops: 0000000096000004 [#1] SMP
  CPU: 0 PID: 74 Comm: kworker/0:3
  Workqueue: events delayed_fput
  pc : fastrpc_buf_free+0x34/0x6c [fastrpc]
  lr : fastrpc_release+0x14/0x20 [fastrpc]
  x1 : aa0103f6a9025bf5  <- corrupted buf->fl pointer

Fix this by taking a reference to the fastrpc user via fastrpc_user_get(fl)
before calling dma_buf_export(), and releasing it with fastrpc_user_put(fl)
in fastrpc_release() once the dma_buf is freed. This mirrors the reference
management already used by fastrpc_context_alloc/fastrpc_context_free.

Fixes: 35a82b87135d ("misc: fastrpc: Add dma handle implementation")
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index af18ff1992eedb6f44ff57ded4f0c577e040e885..36722ed66edff307d166bda30db1d4fe9f73da0f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -796,8 +796,10 @@ static void fastrpc_unmap_dma_buf(struct dma_buf_attachment *attach,
 static void fastrpc_release(struct dma_buf *dmabuf)
 {
 	struct fastrpc_buf *buffer = dmabuf->priv;
+	struct fastrpc_user *fl = buffer->fl;
 
 	fastrpc_buf_free(buffer);
+	fastrpc_user_put(fl);
 }
 
 static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
@@ -1802,10 +1804,12 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp)
 	exp_info.size = bp.size;
 	exp_info.flags = O_RDWR;
 	exp_info.priv = buf;
+	fastrpc_user_get(fl);
 	buf->dmabuf = dma_buf_export(&exp_info);
 	if (IS_ERR(buf->dmabuf)) {
 		err = PTR_ERR(buf->dmabuf);
 		fastrpc_buf_free(buf);
+		fastrpc_user_put(fl);
 		return err;
 	}
 

---
base-commit: f1ac44c32cba88ad65dcecd4a279661d14e93651
change-id: 20261027-fastrpc-fix-2b9b042df52b

Best regards,
-- 
Loic Poulain <loic.poulain@oss.qualcomm.com>

Re: [PATCH] misc: fastrpc: fix use-after-free of fastrpc_user in dma_buf release

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] misc: fastrpc: reference cycle between fastrpc_user and dma_buf
--

commit dec722bc667a70cb2ae899023a7e5c5a5ba8d6fb
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

misc: fastrpc: fix use-after-free of fastrpc_user in dma_buf release

This commit fixes a use-after-free of struct fastrpc_user during dma_buf
release by taking a reference on fl in fastrpc_dmabuf_alloc() and dropping it
in fastrpc_release().

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index af18ff1992eed..36722ed66edff 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -796,8 +796,10 @@ static void fastrpc_unmap_dma_buf(struct dma_buf_attachment *attach,
>  static void fastrpc_release(struct dma_buf *dmabuf)
>  {
>  	struct fastrpc_buf *buffer = dmabuf->priv;
> +	struct fastrpc_user *fl = buffer->fl;
>  
>  	fastrpc_buf_free(buffer);
> +	fastrpc_user_put(fl);
>  }
>  
>  static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
> @@ -1802,10 +1804,12 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp)
>  	exp_info.size = bp.size;
>  	exp_info.flags = O_RDWR;
>  	exp_info.priv = buf;
> +	fastrpc_user_get(fl);
>  	buf->dmabuf = dma_buf_export(&exp_info);
>  	if (IS_ERR(buf->dmabuf)) {
>  		err = PTR_ERR(buf->dmabuf);
>  		fastrpc_buf_free(buf);
> +		fastrpc_user_put(fl);
>  		return err;
>  	}

[Severity: High]
Does taking a reference on fl for the lifetime of the exported dma_buf create
a circular reference that leaks fl, map, and the coherent DMA buffer when a
dma_buf allocated by fastrpc_dmabuf_alloc() is mapped into the same
fastrpc_user session?

When userspace maps an exported dma_buf into the session (for example via
fastrpc_init_create_process() -> fastrpc_map_create() -> fastrpc_map_attach(),
fastrpc_req_mem_map() -> fastrpc_map_create(), or fastrpc_internal_invoke() ->
fastrpc_get_args() -> fastrpc_create_maps()), fastrpc_map_attach() acquires a
reference on the dma_buf and places the map on fl->maps:

drivers/misc/fastrpc.c:fastrpc_map_attach() {
	...
	map->fl = fl;
	map->fd = fd;
	map->buf = dma_buf_get(fd);
	...
	spin_lock(&fl->lock);
	list_add_tail(&map->node, &fl->maps);
	spin_unlock(&fl->lock);
}

In paths such as fastrpc_init_create_process() (or when a process exits
without calling fastrpc_req_mem_unmap()), that map remains on fl->maps until
the session is torn down.

However, fastrpc_device_release() only frees fl->sctx and drops the file's
reference on fl without draining fl->maps:

drivers/misc/fastrpc.c:fastrpc_device_release() {
	...
	fastrpc_session_free(cctx, fl->sctx);
	file->private_data = NULL;
	/* Release the reference taken in fastrpc_device_open */
	fastrpc_user_put(fl);

	return 0;
}

Because fl->maps is only drained in fastrpc_user_free() when fl->refcount
reaches zero:

drivers/misc/fastrpc.c:fastrpc_user_free() {
	...
	list_for_each_entry_safe(map, m, &fl->maps, node)
		fastrpc_map_put(map);
}

and fastrpc_free_map() is what calls dma_buf_put(map->buf) to trigger
fastrpc_release(), fl->refcount cannot reach zero until fastrpc_release()
calls fastrpc_user_put(fl), while fastrpc_release() cannot run until
fastrpc_user_free() drops the map on fl->maps.

Additionally, even if the dma_buf is not in fl->maps (for instance, when it is
held open by another process after the fastrpc device fd is closed), keeping
fl alive until fastrpc_release() delays fastrpc_user_free() past
fastrpc_device_release(), leaving fl->init_mem, fl->maps, and fl->mmaps
allocated in the SMMU context bank after fastrpc_session_free() has already
released fl->sctx for reuse by another client.

Since fastrpc_buf_free(), fastrpc_dma_buf_attach(), and fastrpc_mmap() only
dereference buf->fl->cctx for fastrpc_ipa_to_dma_addr(), could struct
fastrpc_buf store a pointer (and reference, if needed) to struct
fastrpc_channel_ctx instead of pinning struct fastrpc_user?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-fastrpc-fix-v1-1-4610a10a2901@oss.qualcomm.com?part=1