Re: [PATCH v6 3/4] drm/prime: use dma_buf_fd_install() to preserve export tracing

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful mismatch between the commit message's claim and the actual code behavior regarding the file descriptor lifecycle.
--

--- Patch [3]: [PATCH v6 3/4] drm/prime: use dma_buf_fd_install() to preserve export tracing ---
commit 33826692d4a41b387d790330b8cd37a2f20fa524
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(). The change ensures
that the DMA_BUF_TRACE() call is correctly emitted, restoring tracepoint
coverage for DRM PRIME exports.

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: Medium]
Is the statement about the fd lifecycle being already correct accurate?

This isn't a bug introduced by this patch, but rather a pre-existing issue.
Looking at drm_gem_prime_handle_to_fd(), the fd is irrevocably published
via dma_buf_fd_install() before returning:

drivers/gpu/drm/drm_prime.c:drm_gem_prime_handle_to_fd() {
    ...
    dma_buf_fd_install(dmabuf, fd);
    *prime_fd = fd;
    return 0;
}

After returning, the generic ioctl dispatcher performs a fallible copy_to_user():

drivers/gpu/drm/drm_ioctl.c:drm_ioctl() {
    ...
    if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
        retcode = -EFAULT;
    ...
}

If this copy_to_user() fails, does the fd remain permanently published, causing
a leak of the fd and dmabuf reference?

If so, the code might actually need the same delayed publication fix that was
applied to dma-heap, making the commit message's claim incorrect.

> 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