[PATCH 07/11] VFS: introduce lookup_and_lock()

NeilBrown
2024-12-20T03:09:23Z

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() will be called by all callers of
lookup_and_lock() to unlock and dput()

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. This is what callers want.

These functions replace all uses of lookup_one_qstr_excl() 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@suse.de>
---
fs/namei.c | 118 ++++++++++++++++++++++--------------------
include/linux/namei.h | 3 ++
2 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 29f86df4b9dc..371c80902c59 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1703,6 +1703,33 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
}
EXPORT_SYMBOL(lookup_one_qstr_excl);

+static struct dentry *lookup_and_lock(const struct qstr *last,
+ struct dentry *base,
+ unsigned int lookup_flags)
+{
+ struct dentry *dentry;
+ int err;
+
+ inode_lock_nested(base->d_inode, I_MUTEX_PARENT);
+ dentry = lookup_one_qstr_excl(last, base, lookup_flags);
+ if (IS_ERR(dentry))
+ goto out;
+ err = -EEXIST;
+ if ((lookup_flags & LOOKUP_EXCL) && d_is_positive(dentry))
+ goto err;
+ err = -ENOENT;
+ if (!(lookup_flags & LOOKUP_CREATE) && d_is_negative(dentry))
+ goto err;
+ return dentry;
+
+err:
+ dput(dentry);
+ dentry = ERR_PTR(err);
+out:
+ inode_unlock(base->d_inode);
+ return dentry;
+}
+
/**
* lookup_fast - do fast lockless (but racy) lookup of a dentry
* @nd: current nameidata
@@ -2741,16 +2768,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_excl(&last, path->dentry, 0);
- if (!IS_ERR(d) && d_is_negative(d)) {
- dput(d);
- d = ERR_PTR(-ENOENT);
- }
- 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;
}

@@ -3051,6 +3071,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)
{
@@ -4080,7 +4116,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);
@@ -4092,50 +4127,31 @@ 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 = 0;
- inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
- dentry = lookup_one_qstr_excl(&last, path->dentry,
- reval_flag | create_flags);
+ create_flags &= ~LOOKUP_CREATE;
+ dentry = lookup_and_lock(&last, path->dentry, reval_flag | create_flags);
if (IS_ERR(dentry))
- goto unlock;
+ goto drop;

- 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;
+ if (unlikely(error))
goto fail;
- }
return dentry;
fail:
d_lookup_done(dentry);
- dput(dentry);
+ done_lookup_and_lock(path->dentry, dentry);
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;
}
@@ -4555,23 +4571,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_excl(&last, path.dentry, lookup_flags);
+ dentry = lookup_and_lock(&last, path.dentry, lookup_flags);
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;
error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode, dentry);
exit4:
- dput(dentry);
+ done_lookup_and_lock(path.dentry, dentry);
exit3:
- inode_unlock(path.dentry->d_inode);
mnt_drop_write(path.mnt);
exit2:
path_put(&path);
@@ -4691,13 +4702,11 @@ 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_excl(&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] || d_is_negative(dentry))
+ if (last.name[last.len])
goto slashes;
inode = dentry->d_inode;
ihold(inode);
@@ -4707,9 +4716,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);
}
- inode_unlock(path.dentry->d_inode);
if (inode)
iput(inode); /* truncate the inode here */
inode = NULL;
@@ -4731,9 +4739,7 @@ int do_unlinkat(int dfd, struct filename *name)
return error;

slashes:
- if (d_is_negative(dentry))
- error = -ENOENT;
- else if (d_is_dir(dentry))
+ if (d_is_dir(dentry))
error = -EISDIR;
else
error = -ENOTDIR;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 898fc8ba37e1..f882874a7b00 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -83,6 +83,9 @@ 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);

extern int follow_down_one(struct path *);
extern int follow_down(struct path *path, unsigned int flags);
--
2.47.0