linux-kernel - [PATCH 02/12] 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: <165516230197.21248.5856192432755210577.stgit@noble.brown>
Date:   Tue, 14 Jun 2022 09:18:21 +1000
From:   NeilBrown <neilb@...e.de>
To:     Al Viro <viro@...iv.linux.org.uk>, 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/12] 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.

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

diff --git a/fs/namei.c b/fs/namei.c
index 9a2ca515c219..e7a56d6e2472 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1563,7 +1563,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,
 					 struct dentry *base, unsigned int flags,
@@ -1644,6 +1652,20 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
 		err = PTR_ERR(dentry);
 		goto out_err;
 	}
+	if (flags & LOOKUP_EXCL) {
+		if (d_is_positive(dentry)) {
+			dput(dentry);
+			err = -EEXIST;
+			goto out_err;
+		}
+	}
+	if (!(flags & LOOKUP_CREATE)) {
+		if (!dentry->d_inode) {
+			dput(dentry);
+			err = -ENOENT;
+			goto out_err;
+		}
+	}
 	if (wq && !d_lock_update(dentry, base, name)) {
 		/*
 		 * Failed to get lock due to race with unlink or rename
@@ -3838,7 +3860,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;
 
@@ -3849,26 +3871,17 @@ 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 with 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;
@@ -4264,10 +4277,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;
@@ -4407,8 +4416,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)