linux-kernel - Re: [PATCH 10/11] VFS: use d_alloc_parallel() in lookup_one_qstr

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: <175513301880.2234665.7949166216437739702@noble.neil.brown.name>
Date: Thu, 14 Aug 2025 10:56:58 +1000
From: "NeilBrown" <neil@...wn.name>
To: "Al Viro" <viro@...iv.linux.org.uk>
Cc: "Christian Brauner" <brauner@...nel.org>, "Jan Kara" <jack@...e.cz>,
 "David Howells" <dhowells@...hat.com>,
 "Marc Dionne" <marc.dionne@...istor.com>, "Xiubo Li" <xiubli@...hat.com>,
 "Ilya Dryomov" <idryomov@...il.com>, "Tyler Hicks" <code@...icks.com>,
 "Miklos Szeredi" <miklos@...redi.hu>, "Richard Weinberger" <richard@....at>,
 "Anton Ivanov" <anton.ivanov@...bridgegreys.com>,
 "Johannes Berg" <johannes@...solutions.net>,
 "Trond Myklebust" <trondmy@...nel.org>, "Anna Schumaker" <anna@...nel.org>,
 "Chuck Lever" <chuck.lever@...cle.com>, "Jeff Layton" <jlayton@...nel.org>,
 "Amir Goldstein" <amir73il@...il.com>, "Steve French" <sfrench@...ba.org>,
 "Namjae Jeon" <linkinjeon@...nel.org>, "Carlos Maiolino" <cem@...nel.org>,
 linux-fsdevel@...r.kernel.org, linux-afs@...ts.infradead.org,
 netfs@...ts.linux.dev, ceph-devel@...r.kernel.org, ecryptfs@...r.kernel.org,
 linux-um@...ts.infradead.org, linux-nfs@...r.kernel.org,
 linux-unionfs@...r.kernel.org, linux-cifs@...r.kernel.org,
 linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject:
 Re: [PATCH 10/11] VFS: use d_alloc_parallel() in lookup_one_qstr_excl().

On Wed, 13 Aug 2025, Al Viro wrote:
> On Tue, Aug 12, 2025 at 12:25:13PM +1000, NeilBrown wrote:
> 
> > + * If it is d_in_lookup() then these conditions can only be checked by the
> > + * file system when carrying out the intent (create or rename).
> 
> I do not understand.  In which cases would that happen and what would happen
> prior to that patch in the same cases?
> 

NFS (and I think it is only NFS) returns NULL from ->lookup() without
instantiating the dentry and without clearing DENTRY_PAR_LOOKUP if
passed "LOOKUP_CREATE | LOOKUP_EXCL" or "LOOKUP_RENAME_TARGET".

So when e.g. filename_create() calls lookup_one_qstr_excl() the result could
be a d_in_lookup() dentry.  It could be that the name exists on the
server, but the client hasn't bothered to check.  So determining that
the result wasn't ERR_PTR(-EEXIST) does NOT assure us that the name
doesn't exist.

The intent needs to be attempted, such as when do_mknodat() goes on to
call e.g.  vfs_create().  Only once that returns an error can we know if
the name existed.

i.e. the API promise:

+ *   Will return -EEXIST if name is found and LOOKUP_EXCL was passed.

must be understood against the background that the name might not be
found due to the lookup being short-circuited and not attempted.
The other promise:

+ *   Will return -ENOENT if name isn't found and LOOKUP_CREATE wasn't passed.

is currently safe from confusion, but I can imagine that one day a
LOOKUP_UNLINK intent could allow a filesystem to short-circuit the
lookup in do_unlinkat() and simply send an UNLINK request to a server
and return the result.

So I thought it worth highlighting the fact that these errors are
best-effort, and that d_in_lookup() is a real possibility.

NeilBrown