VFS: move mnt_want_write() and locking into lookup_open()

The mnt_want_write() call and the parent inode locking in
open_last_lookups() are only needed for lookup_open().  So we can move
them and all the got_write handling into lookup_open().

Note that we need to also check create_error when determining whether to
unlock shared or not, as O_CREAT can be cleared, but create_error is
only set of O_CREAT was set.

The fsnotify calls come too as they must be in the locked region.

Also use the existing dir_inode uniformly for dir->d_inode.

This is a step towards exporting an better "open/create" interface to nfsd.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260714230534.776886-2-neilb@ownmail.net
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index 5cc9f0f466b8300cce933a8d90a014a8dc4e5a2e..711c7745e74727794a6b85398e1440825d8273a8 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4403,7 +4403,7 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry
  */
 static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                                  const struct open_flags *op,
-                                 bool got_write, struct delegated_inode *delegated_inode)
+                                 struct delegated_inode *delegated_inode)
 {
        struct mnt_idmap *idmap;
        struct dentry *dir = nd->path.dentry;
@@ -4412,9 +4412,25 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
        struct dentry *dentry;
        int error, create_error = 0;
        umode_t mode = op->mode;
+       bool got_write = false;
 
-       if (unlikely(IS_DEADDIR(dir_inode)))
-               return ERR_PTR(-ENOENT);
+       if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
+               got_write = !mnt_want_write(nd->path.mnt);
+               /*
+                * do _not_ fail yet - we might not need that or fail with
+                * a different error; let lookup_open() decide; we'll be
+                * dropping this one anyway.
+                */
+       }
+       if (open_flag & O_CREAT)
+               inode_lock(dir_inode);
+       else
+               inode_lock_shared(dir_inode);
+
+       if (unlikely(IS_DEADDIR(dir_inode))) {
+               dentry = ERR_PTR(-ENOENT);
+               goto out;
+       }
 
        file->f_mode &= ~FMODE_CREATED;
        dentry = d_lookup(dir, &nd->last);
@@ -4422,7 +4438,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                if (!dentry) {
                        dentry = d_alloc_parallel(dir, &nd->last);
                        if (IS_ERR(dentry))
-                               return dentry;
+                               goto out;
                }
                if (d_in_lookup(dentry))
                        break;
@@ -4438,7 +4454,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
        }
        if (dentry->d_inode) {
                /* Cached positive dentry: will open in f_op->open */
-               return dentry;
+               goto out;
        }
 
        if (open_flag & O_CREAT)
@@ -4459,7 +4475,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
        if (open_flag & O_CREAT) {
                if (open_flag & O_EXCL)
                        open_flag &= ~O_TRUNC;
-               mode = vfs_prepare_mode(idmap, dir->d_inode, mode, mode, mode);
+               mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode);
                if (likely(got_write))
                        create_error = may_o_create(idmap, &nd->path,
                                                    dentry, mode);
@@ -4474,7 +4490,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                dentry = atomic_open(&nd->path, dentry, file, open_flag, mode);
                if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
                        dentry = ERR_PTR(create_error);
-               return dentry;
+               goto out;
        }
 
        if (d_in_lookup(dentry)) {
@@ -4514,11 +4530,27 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                error = create_error;
                goto out_dput;
        }
+out:
+       if (!IS_ERR(dentry)) {
+               if (file->f_mode & FMODE_CREATED)
+                       fsnotify_create(dir_inode, dentry);
+               if (file->f_mode & FMODE_OPENED)
+                       fsnotify_open(file);
+       }
+       if ((open_flag & O_CREAT) || create_error)
+               inode_unlock(dir_inode);
+       else
+               inode_unlock_shared(dir_inode);
+
+       if (got_write)
+               mnt_drop_write(nd->path.mnt);
+
        return dentry;
 
 out_dput:
        dput(dentry);
-       return ERR_PTR(error);
+       dentry = ERR_PTR(error);
+       goto out;
 }
 
 static inline bool trailing_slashes(struct nameidata *nd)
@@ -4561,9 +4593,7 @@ static const char *open_last_lookups(struct nameidata *nd,
                   struct file *file, const struct open_flags *op)
 {
        struct delegated_inode delegated_inode = { };
-       struct dentry *dir = nd->path.dentry;
        int open_flag = op->open_flag;
-       bool got_write = false;
        struct dentry *dentry;
        const char *res;
 
@@ -4593,32 +4623,7 @@ static const char *open_last_lookups(struct nameidata *nd,
                }
        }
 retry:
-       if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
-               got_write = !mnt_want_write(nd->path.mnt);
-               /*
-                * do _not_ fail yet - we might not need that or fail with
-                * a different error; let lookup_open() decide; we'll be
-                * dropping this one anyway.
-                */
-       }
-       if (open_flag & O_CREAT)
-               inode_lock(dir->d_inode);
-       else
-               inode_lock_shared(dir->d_inode);
-       dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
-       if (!IS_ERR(dentry)) {
-               if (file->f_mode & FMODE_CREATED)
-                       fsnotify_create(dir->d_inode, dentry);
-               if (file->f_mode & FMODE_OPENED)
-                       fsnotify_open(file);
-       }
-       if (open_flag & O_CREAT)
-               inode_unlock(dir->d_inode);
-       else
-               inode_unlock_shared(dir->d_inode);
-
-       if (got_write)
-               mnt_drop_write(nd->path.mnt);
+       dentry = lookup_open(nd, file, op, &delegated_inode);
 
        if (IS_ERR(dentry)) {
                if (is_delegated(&delegated_inode)) {