Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Build failure due to missing dependency `dma_buf_fd_install`. Pre-existing issues: - [High] Permanent file descriptor and dmabuf leak if copy_to_user() fails in drm_ioctl. -- --- Patch [1]: [PATCH v3] drm/prime: use dma_buf_fd_install() to preserve export tracing --- commit 0d84b19a88b6c4fa09505ca93169d721f3560e20 Author: Baineng Shou <[email protected]> drm/prime: use dma_buf_fd_install() to preserve export tracing This commit replaces fd_install() with dma_buf_fd_install() in drm_gem_prime_handle_to_fd() to ensure DMA_BUF_TRACE() is called when exporting a PRIME handle. It notes a dependency on a separate patch for the new helper. Link: https://lore.kernel.org/dri-devel/[email protected]/ > 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); [Severity: High] Is dma_buf_fd_install() defined in the current tree? The commit message notes a dependency on a patch that introduces this function, but without it present in the branch, won't this cause a build failure due to an implicit function declaration and undefined symbol? [Severity: High] This is a pre-existing issue, but can this leak the fd and dmabuf if copy_to_user() fails later in drm_ioctl()? If userspace calls the DRM_IOCTL_PRIME_HANDLE_TO_FD ioctl with a read-only memory mapping for the argument pointer, drm_gem_prime_handle_to_fd() installs the fd: drivers/gpu/drm/drm_prime.c:drm_gem_prime_handle_to_fd() { ... dma_buf_fd_install(dmabuf, fd); ... } Afterward, the generic ioctl dispatcher attempts to write the resulting fd back to userspace: drivers/gpu/drm/drm_ioctl.c:drm_ioctl() { ... if (copy_to_user((void __user *)arg, kdata, out_size) != 0) retcode = -EFAULT; ... } If this copy fails, the ioctl returns -EFAULT and the user application never receives the fd number to close it. Will the fd remain open in the process, permanently leaking the dma_buf and associated GEM object until the process exits? > *prime_fd = fd; > return 0; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
