Re: [PATCH] misc: fastrpc: fix use-after-free in fastrpc_map_attach() error path

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch prevents a use-after-free by setting `map->table = NULL` in the error path of `fastrpc_map_attach()`, but this erroneously bypasses the required `dma_buf_unmap_attachment_unlocked()` cleanup, causing a severe memory and resource leak.
--

--- Patch [1]: [PATCH] misc: fastrpc: fix use-after-free in fastrpc_map_attach() error path ---
commit cfc93c4f96cbcb09d2d76ab41801241697750f77
Author: Yifei Gao <[email protected]>

misc: fastrpc: fix use-after-free in fastrpc_map_attach() error path

This commit clears map->table in the error path of fastrpc_map_attach() to
prevent a use-after-free condition when fastrpc_free_map() subsequently cleans
up the attachment and buffer.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index f3a49384586d1..d6be951d55389 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -915,6 +915,7 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  
>  map_err:
>  	dma_buf_detach(map->buf, map->attach);
> +	map->table = NULL;

[Severity: High]
Does this bypass the required DMA unmap operations and leak the sg_table and
IOMMU mappings?

If the length validation fails after the attachment is mapped, the code jumps
to map_err where map->table is set to NULL. When execution falls through to
fastrpc_map_put() and eventually calls fastrpc_free_map(), the cleanup check
will completely skip the unmap:

drivers/misc/fastrpc.c:fastrpc_free_map() {
	...
	if (map->table) {
		...
		dma_buf_unmap_attachment_unlocked(map->attach, map->table,
						  DMA_BIDIRECTIONAL);
		dma_buf_detach(map->buf, map->attach);
		dma_buf_put(map->buf);
	}
	...
}

Because map->table is cleared in the error path without first calling
dma_buf_unmap_attachment_unlocked(), the mapping is never freed. Since this
error path can be reached by an unprivileged user passing a small DMA-BUF with
a large requested length, can this be exploited to cause unbounded resource
exhaustion?

>  attach_err:
>  	dma_buf_put(map->buf);
>  get_err:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1