Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The file descriptor is published to userspace before the `copy_to_user()` operation succeeds, leading to a leaked fd and `dma_buf` reference if the copy fails. -- --- Patch [3]: [PATCH v5 3/4] drm/prime: use dma_buf_fd_install() to preserve export tracing --- commit 50a50a53aa7179af6880f9711b00fcaf6105ea33 Author: Baineng Shou <[email protected]> drm/prime: use dma_buf_fd_install() to preserve export tracing This commit replaces the open-coded fd reservation and installation in drm_gem_prime_handle_to_fd with dma_buf_fd_install. This restores the trace_dma_buf_fd tracepoint coverage that was bypassed by using fd_install directly. Link: https://lore.kernel.org/dri-devel/[email protected]/ > Replace the bare fd_install() with dma_buf_fd_install(), which wraps > fd_install() together with DMA_BUF_TRACE(), restoring full tracepoint > coverage. No functional change; the fd lifecycle (get_unused_fd_flags > → work → install) is already correct. [Severity: High] This is a pre-existing issue, but is the statement about the fd lifecycle being already correct accurate? In drm_gem_prime_handle_to_fd(), the file descriptor is published to the process's fd table before the ioctl data is copied back to user space: drivers/gpu/drm/drm_prime.c:drm_gem_prime_handle_to_fd() { ... dma_buf_fd_install(dmabuf, fd); *prime_fd = fd; return 0; } The actual copy_to_user() happens later in the DRM ioctl wrapper: drivers/gpu/drm/drm_ioctl.c:drm_ioctl() { ... retcode = drm_ioctl_kernel(filp, func, kdata, ioctl->flags); if (copy_to_user((void __user *)arg, kdata, out_size) != 0) retcode = -EFAULT; ... } If a user application allocates a page, maps it as read-only, constructs a valid drm_prime_handle payload within it, and invokes DRM_IOCTL_PRIME_HANDLE_TO_FD, the core drm_ioctl() successfully reads the input. The handler executes, allocates the dmabuf, and publishes the file descriptor. When drm_ioctl() attempts to write the result back via copy_to_user(), it fails due to the read-only mapping, causing the ioctl to return -EFAULT. Since the fd is already published and the userspace application does not know the fd number to close it, does this result in a leak of the file descriptor and dmabuf references? > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 9b44c78cd77fc..fe3436d1235db 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -524,7 +524,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > return PTR_ERR(dmabuf); > } > > - fd_install(fd, dmabuf->file); > + dma_buf_fd_install(dmabuf, fd); > *prime_fd = fd; > return 0; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
