linux-kernel - Re: [PATCH v1 0/3] Allow knfsd to use atomic

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: <176351538077.634289.8846523947369398554@noble.neil.brown.name>
Date: Wed, 19 Nov 2025 12:23:00 +1100
From: NeilBrown <neilb@...mail.net>
To: "Benjamin Coddington" <bcodding@...merspace.com>
Cc: "Alexander Viro" <viro@...iv.linux.org.uk>,
 "Christian Brauner" <brauner@...nel.org>, "Jan Kara" <jack@...e.cz>,
 "Chuck Lever" <chuck.lever@...cle.com>, "Jeff Layton" <jlayton@...nel.org>,
 "Olga Kornievskaia" <okorniev@...hat.com>, "Dai Ngo" <Dai.Ngo@...cle.com>,
 "Tom Talpey" <tom@...pey.com>, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org,
 "Trond Myklebust" <trondmy@...nel.org>, "Mike Snitzer" <snitzer@...nel.org>
Subject: Re: [PATCH v1 0/3] Allow knfsd to use atomic_open()

On Wed, 19 Nov 2025, Benjamin Coddington wrote:
> We have workloads that will benefit from allowing knfsd to use atomic_open()
> in the open/create path.  There are two benefits; the first is the original
> matter of correctness: when knfsd must perform both vfs_create() and
> vfs_open() in series there can be races or error results that cause the
> caller to receive unexpected results.  The second benefit is that for some
> network filesystems, we can reduce the number of remote round-trip
> operations by using a single atomic_open() path which provides a performance
> benefit. 
> 
> I've implemented this with the simplest possible change - by modifying
> dentry_create() which has a single user: knfsd.  The changes cause us to
> insert ourselves part-way into the previously closed/static atomic_open()
> path, so I expect VFS folks to have some good ideas about potentially
> superior approaches.

I think using atomic_open is important - thanks for doing this.

I think there is another race this fixes.
If the client ends and unchecked v4 OPEN request, nfsd does a lookup and
finds the name doesn't exist, it will then (currently) use vfs_create()
requesting an exclusive create.  If this races with a create happening
from another client, this could result in -EEXIST which is not what the
client would expect.  Using atomic_open would fix this.

However I cannot see that you ever pass O_EXCL to atomic_open (or did I
miss something?).  So I don't think the code is quite right yet.  O_EXCL
should be passed is an exclusive or checked create was requested.

With a VFS hat on, I would rather there were more shared code between
dentry_create() and lookup_open().  I don't know exactly what this would
look like, and I wouldn't want that desire to hold up this patch, but it
might be worth thinking about to see if there are any easy similarities
to exploit.

Thanks,
NeilBrown

> 
> Thanks for any comment and critique.
> 
> Benjamin Coddington (3):
>   VFS: move dentry_create() from fs/open.c to fs/namei.c
>   VFS: Prepare atomic_open() for dentry_create()
>   VFS/knfsd: Teach dentry_create() to use atomic_open()
> 
>  fs/namei.c         | 84 ++++++++++++++++++++++++++++++++++++++++++----
>  fs/nfsd/nfs4proc.c |  8 +++--
>  fs/open.c          | 41 ----------------------
>  include/linux/fs.h |  2 +-
>  4 files changed, 83 insertions(+), 52 deletions(-)
> 
> -- 
> 2.50.1
> 
>