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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAGudoHFpfJ_m3HTJntxKt-ZF2x1eQ4A4Pjjp6trf6Y5fnY6rhQ@mail.gmail.com>
Date: Thu, 20 Mar 2025 15:03:24 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Christian Brauner <brauner@...nel.org>
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 2:58 PM Christian Brauner <brauner@...nel.org> wrote:
>
> 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. ;)

oops.

Well in that case the previously sent variant applies:
https://lore.kernel.org/linux-fsdevel/20250320102637.1924183-1-mjguzik@gmail.com/T/#u

Although I see the commit message would use a small tweak:
> Given that fd_install() does not suffer the burden, this patch removes
> the above and instead expands on the race in dup2() commentary instead.

s/ instead././

-- 
Mateusz Guzik <mjguzik gmail.com>