Re: [PATCH 1/4] new helper: drm_gem_prime_handle_to_dmabuf()

[Al Viro <viro@zeniv.linux.org.uk>]

On Thu, Aug 22, 2024 at 04:41:59PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.08.24 um 08:59 schrieb Al Viro:
> > Once something had been put into descriptor table, the only thing you
> > can do with it is returning descriptor to userland - you can't withdraw
> > it on subsequent failure exit, etc.  You certainly can't count upon
> > it staying in the same slot of descriptor table - another thread
> > could've played with close(2)/dup2(2)/whatnot.
> 
> This paragraph appears to refer to the newly added call to fd_install().

It refers to dma_buf_fd() call that had been there all along, actually.
dma_buf_fd() is get_unused_fd_flags() + fd_install().  The reason
for splitting it in new variant and calling get_unused_fd_flags() and
fd_install() separately is that it makes for simpler cleanup; we could
use dma_buf_fd() instead - it would be a bit more clumsy, but that's
it.

The real issue is that drm_gem_prime_handle_to_fd() forces us to make
the thing reachable via descriptor table; it's just what we need when
all we are going to do is returning descriptor to userland, but it's
inherently racy for internal uses - anything put into descriptor table,
be it by fd_install() or by dma_buf_fd(), is fair game for all syscalls
by other threads.
 
> > Add drm_gem_prime_handle_to_dmabuf() - the "set dmabuf up" parts of
> > drm_gem_prime_handle_to_fd() without the descriptor-related ones.
> > Instead of inserting into descriptor table and returning the file
> > descriptor it just returns the struct file.
> > 
> > drm_gem_prime_handle_to_fd() becomes a wrapper for it.  Other users
> > will be introduced in the next commit.

> > -int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> > +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev,
> 
> If it's exported it should have kernel docs. At least copy-paste the docs
> from drm_gem_prime_handle_to_fd()
> and reword a few bits.

Point...