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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250320-befund-wegnehmen-048d8b9cd252@brauner>
Date: Thu, 20 Mar 2025 14:58:28 +0100
From: Christian Brauner <brauner@...nel.org>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: viro@...iv.linux.org.uk, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2] fs: sort out stale commentary about races between fd
 alloc and dup2()

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@...il.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. ;)