linux-kernel - Re: [PATCH 0/3} Change ->mkdir() and vfs

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: <173976443235.3118120.11496260792280593655@noble.neil.brown.name>
Date: Mon, 17 Feb 2025 14:53:52 +1100
From: "NeilBrown" <neilb@...e.de>
To: "Al Viro" <viro@...iv.linux.org.uk>
Cc: "Christian Brauner" <brauner@...nel.org>,
 "Trond Myklebust" <trondmy@...nel.org>, "Anna Schumaker" <anna@...nel.org>,
 linux-nfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3} Change ->mkdir() and vfs_mkdir() to return a dentry

On Fri, 14 Feb 2025, Al Viro wrote:
> On Fri, Feb 14, 2025 at 04:16:40PM +1100, NeilBrown wrote:
> > This is a small set of patches which are needed before we can make the
> > locking on directory operations more fine grained.  I think they are
> > useful even if we don't go that direction.
> > 
> > Some callers of vfs_mkdir() need to operation on the resulting directory
> > but cannot be guaranteed that the dentry will be hashed and positive on
> > success - another dentry might have been used.
> > 
> > This patch changes ->mkdir to return a dentry, changes NFS in particular
> > to return the correct dentry (I believe it is the only filesystem to
> > possibly not use the given dentry), and changes vfs_mkdir() to return
> > that dentry, removing the look that a few callers currently need.
> > 
> > I have not Cc: the developers of all the individual filesystems - only
> > NFS.  I have build-tested all the changes except hostfs.  I can email
> > them explicitly if/when this is otherwise acceptable.  If anyone sees
> > this on fs-devel and wants to provide a pre-emptive ack I will collect
> > those and avoid further posting for those fs.
> 
> 1) please, don't sprinkle the PTR_ERR_OR_ZERO() shite all over the place.
> Almost always the same thing can be done without it and it ends up
> being cleaner.  Seriously.

I've removed several PTR_ERR_OR_ZERO() calls.  Some times that could be
seen as a slight improvement, other times possibly a slight negative
(depending on how one feels about PTR_ERR_OR_ZERO of course).  I have
left three as I cannot see how to remove them without making the code
significant more clumsy.  If you find the remaining few to still be
objectionable I would be happy to see what alternate you would propose.

Your other feedback has been quite helpful - thanks.

NeilBrown


> 
> 2) I suspect that having method instances return NULL for "just use the
> argument" would would be harder to fuck up; basically, the same as for
> ->lookup() instances.  I'll try to tweak it and see what falls out...
> 
> 3) I'm pretty sure that NFS is *not* the only filesystem that returns
> unhashed negative in some success cases; will need to go over the instances
> to verify that, though.
>