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 <[email protected]> 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/[email protected]?part=1
