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: <20250206054504.2950516-9-neilb@suse.de>
Date: Thu,  6 Feb 2025 16:42:45 +1100
From: NeilBrown <neilb@...e.de>
To: Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>,
	Jan Kara <jack@...e.cz>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jeff Layton <jlayton@...nel.org>,
	Dave Chinner <david@...morbit.com>
Cc: linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH 08/19] VFS: introduce lookup_and_lock() and friends

lookup_and_lock() combines locking the directory and performing a lookup
prior to a change to the directory.
Abstracting this prepares for changing the locking requirements.

done_lookup_and_lock() provides the inverse of putting the dentry and
unlocking.

For "silly_rename" we will need to lookup_and_lock() in a directory that
is already locked.  For this purpose we add LOOKUP_PARENT_LOCKED.

Like lookup_len_qstr(), lookup_and_lock() returns -ENOENT if
LOOKUP_CREATE was NOT given and the name cannot be found,, and returns
-EEXIST if LOOKUP_EXCL WAS given and the name CAN be found.

These functions replace all uses of lookup_one_qstr() in namei.c
except for those used for rename.

The name might seem backwards as the lock happens before the lookup.
A future patch will change this so that only a shared lock is taken
before the lookup, and an exclusive lock on the dentry is taken after a
successful lookup.  So the order "lookup" then "lock" will make sense.

This functionality is exported as lookup_and_lock_one() which takes a
name and len rather than a qstr.

Signed-off-by: NeilBrown <neilb@...e.de>
---
 fs/namei.c            | 102 ++++++++++++++++++++++++++++--------------
 include/linux/namei.h |  15 ++++++-
 2 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 69610047f6c6..3c0feca081a2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1715,6 +1715,41 @@ struct dentry *lookup_one_qstr(const struct qstr *name,
 }
 EXPORT_SYMBOL(lookup_one_qstr);
 
+static struct dentry *lookup_and_lock_nested(const struct qstr *last,
+					     struct dentry *base,
+					     unsigned int lookup_flags,
+					     unsigned int subclass)
+{
+	struct dentry *dentry;
+
+	if (!(lookup_flags & LOOKUP_PARENT_LOCKED))
+		inode_lock_nested(base->d_inode, subclass);
+
+	dentry = lookup_one_qstr(last, base, lookup_flags);
+	if (IS_ERR(dentry) && !(lookup_flags & LOOKUP_PARENT_LOCKED)) {
+			inode_unlock(base->d_inode);
+	}
+	return dentry;
+}
+
+static struct dentry *lookup_and_lock(const struct qstr *last,
+				      struct dentry *base,
+				      unsigned int lookup_flags)
+{
+	return lookup_and_lock_nested(last, base, lookup_flags,
+				      I_MUTEX_PARENT);
+}
+
+void done_lookup_and_lock(struct dentry *base, struct dentry *dentry,
+			  unsigned int lookup_flags)
+{
+	d_lookup_done(dentry);
+	dput(dentry);
+	if (!(lookup_flags & LOOKUP_PARENT_LOCKED))
+		inode_unlock(base->d_inode);
+}
+EXPORT_SYMBOL(done_lookup_and_lock);
+
 /**
  * lookup_fast - do fast lockless (but racy) lookup of a dentry
  * @nd: current nameidata
@@ -2754,12 +2789,9 @@ static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct
 		path_put(path);
 		return ERR_PTR(-EINVAL);
 	}
-	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
-	d = lookup_one_qstr(&last, path->dentry, 0);
-	if (IS_ERR(d)) {
-		inode_unlock(path->dentry->d_inode);
+	d = lookup_and_lock(&last, path->dentry, 0);
+	if (IS_ERR(d))
 		path_put(path);
-	}
 	return d;
 }
 
@@ -3053,6 +3085,22 @@ struct dentry *lookup_positive_unlocked(const char *name,
 }
 EXPORT_SYMBOL(lookup_positive_unlocked);
 
+struct dentry *lookup_and_lock_one(struct mnt_idmap *idmap,
+				   const char *name, int len, struct dentry *base,
+				   unsigned int lookup_flags)
+{
+	struct qstr this;
+	int err;
+
+	if (!idmap)
+		idmap = &nop_mnt_idmap;
+	err = lookup_one_common(idmap, name, base, len, &this);
+	if (err)
+		return ERR_PTR(err);
+	return lookup_and_lock(&this, base, lookup_flags);
+}
+EXPORT_SYMBOL(lookup_and_lock_one);
+
 #ifdef CONFIG_UNIX98_PTYS
 int path_pts(struct path *path)
 {
@@ -4071,7 +4119,6 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
 	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
 	int type;
-	int err2;
 	int error;
 
 	error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
@@ -4083,36 +4130,30 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	 * (foo/., foo/.., /////)
 	 */
 	if (unlikely(type != LAST_NORM))
-		goto out;
+		goto put;
 
 	/* don't fail immediately if it's r/o, at least try to report other errors */
-	err2 = mnt_want_write(path->mnt);
+	error = mnt_want_write(path->mnt);
 	/*
 	 * Do the final lookup.  Suppress 'create' if there is a trailing
 	 * '/', and a directory wasn't requested.
 	 */
 	if (last.name[last.len] && !want_dir)
 		create_flags &= ~LOOKUP_CREATE;
-	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr(&last, path->dentry,
-				 reval_flag | create_flags);
+	dentry = lookup_and_lock(&last, path->dentry, reval_flag | create_flags);
 	if (IS_ERR(dentry))
-		goto unlock;
+		goto drop;
 
-	if (unlikely(err2)) {
-		error = err2;
+	if (unlikely(error))
 		goto fail;
-	}
 	return dentry;
 fail:
-	d_lookup_done(dentry);
-	dput(dentry);
+	done_lookup_and_lock(path->dentry, dentry, reval_flag | create_flags);
 	dentry = ERR_PTR(error);
-unlock:
-	inode_unlock(path->dentry->d_inode);
-	if (!err2)
+drop:
+	if (!error)
 		mnt_drop_write(path->mnt);
-out:
+put:
 	path_put(path);
 	return dentry;
 }
@@ -4130,14 +4171,13 @@ EXPORT_SYMBOL(kern_path_create);
 
 void done_path_create(struct path *path, struct dentry *dentry)
 {
-	dput(dentry);
-	inode_unlock(path->dentry->d_inode);
+	done_lookup_and_lock(path->dentry, dentry, LOOKUP_CREATE);
 	mnt_drop_write(path->mnt);
 	path_put(path);
 }
 EXPORT_SYMBOL(done_path_create);
 
-inline struct dentry *user_path_create(int dfd, const char __user *pathname,
+struct dentry *user_path_create(int dfd, const char __user *pathname,
 				struct path *path, unsigned int lookup_flags)
 {
 	struct filename *filename = getname(pathname);
@@ -4510,19 +4550,18 @@ int do_rmdir(int dfd, struct filename *name)
 	if (error)
 		goto exit2;
 
-	inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr(&last, path.dentry, lookup_flags);
+	dentry = lookup_and_lock(&last, path.dentry, lookup_flags);
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit3;
+
 	error = security_path_rmdir(&path, dentry);
 	if (error)
 		goto exit4;
 	error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode, dentry);
 exit4:
-	dput(dentry);
+	done_lookup_and_lock(path.dentry, dentry, lookup_flags);
 exit3:
-	inode_unlock(path.dentry->d_inode);
 	mnt_drop_write(path.mnt);
 exit2:
 	path_put(&path);
@@ -4639,11 +4678,9 @@ int do_unlinkat(int dfd, struct filename *name)
 	if (error)
 		goto exit2;
 retry_deleg:
-	inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr(&last, path.dentry, lookup_flags);
+	dentry = lookup_and_lock(&last, path.dentry, lookup_flags);
 	error = PTR_ERR(dentry);
 	if (!IS_ERR(dentry)) {
-
 		/* Why not before? Because we want correct error value */
 		if (last.name[last.len])
 			goto slashes;
@@ -4655,9 +4692,8 @@ int do_unlinkat(int dfd, struct filename *name)
 		error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
 				   dentry, &delegated_inode);
 exit3:
-		dput(dentry);
+		done_lookup_and_lock(path.dentry, dentry, lookup_flags);
 	}
-	inode_unlock(path.dentry->d_inode);
 	if (inode)
 		iput(inode);	/* truncate the inode here */
 	inode = NULL;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 0d81e571a159..76c587a5ec3a 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -29,7 +29,11 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
 #define LOOKUP_RCU		BIT(8)	/* RCU pathwalk mode; semi-internal */
 #define LOOKUP_CACHED		BIT(9) /* Only do cached lookup */
 #define LOOKUP_PARENT		BIT(10)	/* Looking up final parent in path */
-/* 5 spare bits for pathwalk */
+#define LOOKUP_PARENT_LOCKED	BIT(11)	/* filesystem sets this for nested
+					 * "lookup_and_lock_one" when it knows
+					 * parent is sufficiently locked.
+					 */
+/* 4 spare bits for pathwalk */
 
 /* These tell filesystem methods that we are dealing with the final component... */
 #define LOOKUP_OPEN		BIT(16)	/* ... in open */
@@ -82,6 +86,15 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap,
 struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap,
 					    const char *name,
 					    struct dentry *base, int len);
+struct dentry *lookup_and_lock_one(struct mnt_idmap *idmap,
+				   const char *name, int len, struct dentry *base,
+				   unsigned int lookup_flags);
+struct dentry *__lookup_and_lock_one(struct mnt_idmap *idmap,
+				     const char *name, int len, struct dentry *base,
+				     unsigned int lookup_flags);
+void done_lookup_and_lock(struct dentry *base, struct dentry *dentry,
+			  unsigned int lookup_flags);
+void __done_lookup_and_lock(struct dentry *dentry);
 
 extern int follow_down_one(struct path *);
 extern int follow_down(struct path *path, unsigned int flags);
-- 
2.47.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ