Re: [PATCH] dma-buf: dma-heap: close installed fd when copy_to_user() fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling close_fd() on an installed file descriptor creates a race condition that can inadvertently close an unrelated file descriptor belonging to the process.
--

commit 61669c0c051aa2c310121591ca8897cc10b33840
Author: Baineng Shou <shoubaineng@gmail.com>

dma-buf: dma-heap: close installed fd when copy_to_user() fails

This patch attempts to fix a file descriptor leak when copy_to_user()
fails after allocating a DMA heap buffer. It tries to clean up the
leaked file descriptor by calling close_fd() if the copy to userspace
is unsuccessful.

> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index a76bf3f8b071a..0dd7a84b06bf4 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
[ ... ]
> @@ -181,8 +182,16 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>  		goto err;
>  	}
>  
> -	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
> +	if (copy_to_user((void __user *)arg, kdata, out_size) != 0) {
> +		if (kcmd == DMA_HEAP_IOCTL_ALLOC && ret == 0) {
> +			struct dma_heap_allocation_data *h = (void *)kdata;
> +
> +			close_fd(h->fd);

[Severity: High]
Does calling close_fd() on an installed file descriptor create a race
condition here in dma_heap_ioctl()?

By the time the execution reaches this error path, the file descriptor
has already been published to the process's file descriptor table via
fd_install() inside dma_heap_ioctl_allocate(). This means userspace
can already access it.

A concurrent thread could close the newly allocated file descriptor and
open a new file, reusing the same file descriptor number before the
copy_to_user() completes and fails.

If that happens, wouldn't this close_fd() inadvertently close an unrelated
file descriptor belonging to the process?

> +			h->fd = -1;
> +		}
>  		ret = -EFAULT;
> +	}
> +
>  err:
>  	if (kdata != stack_kdata)
>  		kfree(kdata);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703080922.1838362-1-shoubaineng@gmail.com?part=1