On Mon, 24 Jul 2023 at 15:57, Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> On 7/24/23 4:25?PM, Linus Torvalds wrote:
> > This sentence still worries me.
> >
> > Those fixed files had better have their own refcounts from being
> > fixed. So the rules really shouldn't change in any way what-so-ever.
> > So what exactly are you alluding to?
>
> They do, but they only have a single reference, which is what fixes them
> into the io_uring file table for fixed files. With the patch from the
> top of this thread, that should then be fine as we don't need to
> artificially elevator the ref count more than that.
No.
The patch from the top of this thread cannot *possibly* matter for a
io_uring fixed file.
The fdget_pos() always gets the file pointer from the file table. But
that means that it is guaranteed to have a refcount of at least one.
If io_uring fixed file holds a reference (and not holding a reference
would be a huge bug), that in turn means that the minimum refcount is
now two.
So the code in fdget_pos() is correct, with or without the patch.
The *only* problem is when something actually violates the refcounting
rules. Sadly, that's exactly what pidfd_getfd() does, and can
basically make a private file pointer be non-private without
synchronizing with the original owner of the fd.
Now, io_uring may have had its own problems, if it tried to
re-implement some io_uring-specific version of fdget_pos() for the
fixed file case, and thought that it could use the file_count() == 1
trick when it *wasn't* also a file table entry.
But that would be an independent bug from copy-and-pasting code
without taking the surrounding rules into account.
Linus
