Re: [PATCH] file: always lock position

[Christian Brauner <brauner@kernel.org>]

On Wed, Jul 26, 2023 at 10:31:07AM +0000, David Laight wrote:
> From: Christian Brauner
> > Sent: 26 July 2023 09:37
> ...
> > Yes, and to summarize which I tried in my description for the commit.
> > The getdents support patchset would have introduced a bug because the
> > patchset copied the fdget_pos() file_count(file) > 1 optimization into
> > io_uring.
> > 
> > That works fine as long as the original file descriptor used to register
> > the fixed file is kept. The locking will work correctly as
> > file_count(file) > 1 and no races are possible neither via getdent calls
> > using the original file descriptor nor via io_uring using the fixed file
> > or even mixing both.
> > 
> > But as soon as the original file descriptor is closed the f_count for
> > the file drops back to 1 but continues to be usable from io_uring via
> > the fixed file. Now the optimization that the patchset wanted to copy
> > over would cause bugs as multiple racing getdent requests would be
> > possible using the fixed file.
> 
> Could the io_uring code grab two references?
> That would stop the optimisation without affecting any
> normal code paths?

io_uring doesn't use fdget variants and can't for it's purposes as fdget
is for short term references while io_uring holds on to the file. This
whole thing was about the logic that was copied in a patchset not the
actual helper itself. I thought that was clear from "copied the [...]
optimization into io_uring". It should just not do that.