linux-kernel - [PATCH 02/10] VFS: move EEXIST and ENOENT tests into lookup_hash

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: <166147984373.25420.5973159222199992210.stgit@noble.brown>
Date:   Fri, 26 Aug 2022 12:10:43 +1000
From:   NeilBrown <neilb@...e.de>
To:     Al Viro <viro@...iv.linux.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Daire Byrne <daire@...g.com>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Chuck Lever <chuck.lever@...cle.com>
Cc:     Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH 02/10] VFS: move EEXIST and ENOENT tests into
 lookup_hash_update()

Moving common error handling into lookup_hash_update() simplifies
callers.
A future patch will export this functionality to nfsd, and the more code
we put in the interface, the less code will be needed in nfsd.

EEXIST is returned if LOOKUP_EXCL is set and dentry is positivie.
ENOENT is returned if LOOKUP_CREAT is NOT set and dentry is negative.

This involves seting LOOKUP_EXCL in cases where it wasn't before.
In particular, when creating a non-dir named "foo/", we want the
EEXIST error, but don't want to actually create anything.
Some filesystems assume LOOKUP_EXCL implies LOOKUP_CREATE, so ensure it
does when calling ->lookup().

Signed-off-by: NeilBrown <neilb@...e.de>
---
 fs/namei.c |   58 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c008dfd01e30..09c2d007814a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1562,7 +1562,13 @@ static struct dentry *lookup_dcache(const struct qstr *name,
 {
 	struct dentry *dentry = d_lookup(dir, name);
 	if (dentry) {
-		int error = d_revalidate(dentry, flags);
+		int error;
+		/* Some filesystems assume EXCL -> CREATE, so make
+		 * sure it does.
+		 */
+		if (!(flags & LOOKUP_CREATE))
+			flags &= ~LOOKUP_EXCL;
+		error = d_revalidate(dentry, flags);
 		if (unlikely(error <= 0)) {
 			if (!error)
 				d_invalidate(dentry);
@@ -1621,6 +1627,8 @@ static struct dentry *__lookup_hash(const struct qstr *name,
  * or shared lock depending on the fs preference, then do a lookup,
  * and then set the DCACHE_PAR_UPDATE bit on the child if a shared lock
  * was taken on the parent.
+ * If LOOKUP_EXCL, name should not already exist, else -EEXIST
+ * If not LOOKUP_CREATE, name should already exist, else -ENOENT
  */
 static struct dentry *lookup_hash_update(
 	const struct qstr *name,
@@ -1651,6 +1659,24 @@ static struct dentry *lookup_hash_update(
 		dput(dentry);
 		goto retry;
 	}
+	if (flags & LOOKUP_EXCL) {
+		if (d_is_positive(dentry)) {
+			d_lookup_done(dentry);
+			d_unlock_update(dentry);
+			dput(dentry);
+			err = -EEXIST;
+			goto out_err;
+		}
+	}
+	if (!(flags & LOOKUP_CREATE)) {
+		if (!dentry->d_inode) {
+			d_lookup_done(dentry);
+			d_unlock_update(dentry);
+			dput(dentry);
+			err = -ENOENT;
+			goto out_err;
+		}
+	}
 	return dentry;
 
 out_err:
@@ -3868,7 +3894,7 @@ static struct dentry *filename_create_one(struct qstr *last, struct path *path,
 	struct dentry *dentry;
 	bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
-	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
+	unsigned int create_flag = LOOKUP_CREATE;
 	int err2;
 	int error;
 
@@ -3879,26 +3905,16 @@ static struct dentry *filename_create_one(struct qstr *last, struct path *path,
 	 * '/', and a directory wasn't requested.
 	 */
 	if (last->name[last->len] && !want_dir)
-		create_flags = 0;
+		/* Name was foo/bar/ but not creating a directory, so
+		 * we won't try to create - result will be either -ENOENT
+		 * or -EEXIST.
+		 */
+		create_flag = 0;
 	dentry = lookup_hash_update(last, path->dentry,
-				    reval_flag | create_flags,  wq);
+				    reval_flag | create_flag | LOOKUP_EXCL, wq);
 	if (IS_ERR(dentry))
 		goto drop_write;
 
-	error = -EEXIST;
-	if (d_is_positive(dentry))
-		goto fail;
-
-	/*
-	 * Special case - lookup gave negative, but... we had foo/bar/
-	 * From the vfs_mknod() POV we just have a negative dentry -
-	 * all is fine. Let's be bastards - you had / on the end, you've
-	 * been asking for (non-existent) directory. -ENOENT for you.
-	 */
-	if (unlikely(!create_flags)) {
-		error = -ENOENT;
-		goto fail;
-	}
 	if (unlikely(err2)) {
 		error = err2;
 		goto fail;
@@ -4292,10 +4308,6 @@ int do_rmdir(int dfd, struct filename *name)
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit3;
-	if (!dentry->d_inode) {
-		error = -ENOENT;
-		goto exit4;
-	}
 	error = security_path_rmdir(&path, dentry);
 	if (error)
 		goto exit4;
@@ -4435,8 +4447,6 @@ int do_unlinkat(int dfd, struct filename *name)
 		if (last.name[last.len])
 			goto slashes;
 		inode = dentry->d_inode;
-		if (d_is_negative(dentry))
-			goto slashes;
 		ihold(inode);
 		error = security_path_unlink(&path, dentry);
 		if (error)