Re: [PATCH v2] fs: sort out stale commentary about races between fd alloc and dup2()

[Christian Brauner <brauner@kernel.org>]

On Thu, Mar 20, 2025 at 11:49:22AM +0100, Mateusz Guzik wrote:
> Userspace may be trying to dup2() over a fd which is allocated but not
> yet populated.
> 
> Commentary about it is split in 2 parts (and both warrant changes):
> 
> 1. in dup2()
> 
> It claims the issue is only relevant for shared descriptor tables which
> is of no concern for POSIX (but then is POSIX of concern to anyone
> today?), which I presume predates standarized threading.
> 
> The comment also mentions the following systems:
> - OpenBSD installing a larval file -- they moved away from it, file is
> installed late and EBUSY is returned on conflict
> - FreeBSD returning EBADF -- reworked to install the file early like
> OpenBSD used to do
> - NetBSD "deadlocks in amusing ways" -- their solution looks
> Solaris-inspired (not a compliment) and I would not be particularly
> surprised if it indeed deadlocked, in amusing ways or otherwise
> 
> I don't believe mentioning any of these adds anything and the statement
> about the issue not being POSIX-relevant is outdated.
> 
> dup2 description in POSIX still does not mention the problem.
> 
> 2. above fd_install()
> 
> <quote>
> > We need to detect this and fput() the struct file we are about to
> > overwrite in this case.
> >
> > It should never happen - if we allow dup2() do it, _really_ bad things
> > will follow.
> </quote>
> 
> I have difficulty parsing it. The first sentence would suggest
> fd_install() tries to detect and recover from the race (it does not),
> the next one claims the race needs to be dealt with (it is, by dup2()).
> 
> Given that fd_install() does not suffer the burden, this patch removes
> the above and instead expands on the race in dup2() commentary.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
> 
> This contains the new commentary from:
> https://lore.kernel.org/linux-fsdevel/20250320102637.1924183-1-mjguzik@gmail.com/T/#u
> 
> and obsoletes this guy hanging out in -next:
> ommit ec052fae814d467d6aa7e591b4b24531b87e65ec

This is already upstream as of v6.14-rc1. So please make it a diff on
top. ;)