[RFC] possible way to deal with dup2() vs. allocated but still not opened descriptors

[RFC] possible way to deal with dup2() vs. allocated but still not opened descriptors

[Al Viro]

	dup2(old, new) has an unpleasant corner case - when new has been
already reserved by e.g. open() from another thread, but that open()
still didn't get to installing an opened file to that descriptor.

	It's outside of POSIX scope and any userland code that might
run into it is buggy.  However, we need to make sure that nothing breaks
kernel-side.  We used to have interesting bugs in that area and so did
*BSD kernels.

	Current solution is to have dup2() fail with -EBUSY.  However,
there's an approach that might avoid that wart.  I'm not sure whether
it's worth bothering with - it will have a cost, but then it might reduce
the pingpong on current->files->file_lock.

	Here's what we could do:
* have alloc_fd() insert (struct file *)((unsigned long)current | 1)
into the array of files.
* have fget() et.al. check if the value they'd fetched is even and
treat odd ones as NULL.
* have fd_install() check if the value being replaced is
(struct file *)((unsigned long)current | 1) and quitely do fput()
otherwise.  That, AFAICS, can be done without ->file_lock, just
with cmpxchg().
* have dup2() skip the "is it claimed, but still not open" logics
and skip the "filp_close() whatever we had there" if the old value
had been odd.
* have file_close_fd_locked() treat odd values as 0.
* have put_unused_fd() check if the value currently there is
(struct file *)((unsigned long)current | 1) and quitely do nothing
otherwise.

That, AFAICS, would treat dup2() racing with open() as if open() has won
the race - the end result is that open() succeeds, returns the descriptor
it would've returned but does not do anything to whatever dup2() has
put there.  Same as if open() had finished first, then lost CPU on the
way out to userland and didn't regain it until dup2() has succeeded.

The tasty part (and the only reason why that might be worth considering)
is that we don't need to grab ->file_lock twice per open() that way -
we do have the second atomic operation on fd_install(), but that's going
to be in the cacheline where the file pointer ends up being installed.

The question is whether that outweights the cost of check LSB and branch
not taken on a bunch of hot paths.

Comments?

Re: [RFC] possible way to deal with dup2() vs. allocated but still not opened descriptors

[Al Viro]

On Wed, Jun 05, 2024 at 03:28:55AM +0100, Al Viro wrote:

> 	Current solution is to have dup2() fail with -EBUSY.  However,
> there's an approach that might avoid that wart.  I'm not sure whether
> it's worth bothering with - it will have a cost, but then it might reduce
> the pingpong on current->files->file_lock.

Gah...  Nevermind, that benefit is no longer there (and I really need
to get some sleep - only went "wait a sec, that lock hadn't been
taken in fd_install() since way back" after sending)

Re: [RFC] possible way to deal with dup2() vs. allocated but still not opened descriptors

[Christian Brauner]

On Wed, Jun 05, 2024 at 03:28:55AM +0100, Al Viro wrote:
> 	dup2(old, new) has an unpleasant corner case - when new has been
> already reserved by e.g. open() from another thread, but that open()
> still didn't get to installing an opened file to that descriptor.
> 
> 	It's outside of POSIX scope and any userland code that might
> run into it is buggy.  However, we need to make sure that nothing breaks
> kernel-side.  We used to have interesting bugs in that area and so did
> *BSD kernels.
> 
> 	Current solution is to have dup2() fail with -EBUSY.  However,
> there's an approach that might avoid that wart.  I'm not sure whether
> it's worth bothering with - it will have a cost, but then it might reduce
> the pingpong on current->files->file_lock.

Fwiw, doesn't seem worth the trouble and would likely just complicate
things for something that we've never really heard being a problem.