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

[sashiko-bot@kernel.org]

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