Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Exported DMA buffers maintain a raw pointer to `fastrpc_user` (`buf->fl`) without holding a reference, leading to a Use-After-Free if the DMA buffer outlives the fastrpc device descriptor. -- --- Patch [2]: [PATCH v7 2/4] misc: fastrpc: don't publish fd before copy_to_user() succeeds --- commit f5005f3a9100abf2f83ab45f3bc5e948d9e97e67 Author: Baineng Shou <[email protected]> misc: fastrpc: don't publish fd before copy_to_user() succeeds This commit fixes an issue where a DMA buffer file descriptor is installed into the fd table before successfully copying the fd number back to userspace. It updates the allocation path to reserve the fd first, attempt the user copy, and only publish the fd upon success to prevent leaking access to the fd on failure. > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -1709,24 +1709,20 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user > *fl, char __user *argp) > return err; > } > > - bp.fd = dma_buf_fd(buf->dmabuf, O_ACCMODE); > + bp.fd = get_unused_fd_flags(O_ACCMODE); > if (bp.fd < 0) { > dma_buf_put(buf->dmabuf); > - return -EINVAL; > + return bp.fd; > } > > if (copy_to_user(argp, &bp, sizeof(bp))) { > - /* > - * The usercopy failed, but we can't do much about it, as > - * dma_buf_fd() already called fd_install() and made the > - * file descriptor accessible for the current process. It > - * might already be closed and dmabuf no longer valid when > - * we reach this point. Therefore "leak" the fd and rely on > - * the process exit path to do any required cleanup. > - */ > + put_unused_fd(bp.fd); > + dma_buf_put(buf->dmabuf); > return -EFAULT; > } > > + dma_buf_fd_install(buf->dmabuf, bp.fd); [Severity: Critical] This is a pre-existing issue, but does exporting this dma-buf introduce a use-after-free risk if the dma-buf fd outlives the fastrpc device fd? When the fastrpc device fd is closed, the fastrpc_user struct (fl) is released. However, the exported dma-buf maintains a raw pointer to it via buf->fl without holding a reference. If an unprivileged user allocates a dma-buf here, closes the fastrpc device fd, and then calls close() or mmap() on the dma-buf fd, it triggers dma_buf_put() or mmap(). Those paths call into fastrpc_buf_free() or fastrpc_mmap(), which dereference the dangling buf->fl pointer: fastrpc_buf_free() fastrpc_ipa_to_dma_addr(buf->fl->cctx, buf->dma_addr) Is there a missing reference count increment on fastrpc_user when exporting the buffer, or should the dma-buf release operations handle the device teardown differently? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
