VFS: add common error checks to lookup_one_qstr_excl()

Callers of lookup_one_qstr_excl() often check if the result is negative or
positive.
These changes can easily be moved into lookup_one_qstr_excl() by checking the
lookup flags:
LOOKUP_CREATE means it is NOT an error if the name doesn't exist.
LOOKUP_EXCL means it IS an error if the name DOES exist.

This patch adds these checks, then removes error checks from callers,
and ensures that appropriate flags are passed.

This subtly changes the meaning of LOOKUP_EXCL.  Previously it could
only accompany LOOKUP_CREATE.  Now it can accompany LOOKUP_RENAME_TARGET
as well.  A couple of small changes are needed to accommodate this.  The
NFS change is functionally a no-op but ensures nfs_is_exclusive_create() does
exactly what the name says.

Signed-off-by: NeilBrown <neilb@suse.de>
Link: https://lore.kernel.org/r/20250217003020.3170652-3-neilb@suse.de
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 2ead47e20677fdb28c49d8343229f082475d2930..3ed3f39ecf716b8e08e7812e6ae1a344318b8433 100644 (file)
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -1165,3 +1165,16 @@ magic.
 kern_path_locked() and user_path_locked() no longer return a negative
 dentry so this doesn't need to be checked.  If the name cannot be found,
 ERR_PTR(-ENOENT) is returned.
+
+** recommend**
+
+lookup_one_qstr_excl() is changed to return errors in more cases, so
+these conditions don't require explicit checks:
+
+ - if LOOKUP_CREATE is NOT given, then the dentry won't be negative,
+   ERR_PTR(-ENOENT) is returned instead
+ - if LOOKUP_EXCL IS given, then the dentry won't be positive,
+   ERR_PTR(-EEXIST) is rreturned instread
+
+LOOKUP_EXCL now means "target must not exist".  It can be combined with
+LOOK_CREATE or LOOKUP_RENAME_TARGET.

diff --git a/fs/namei.c b/fs/namei.c
index fb6da3ca0ca5828c2d69529085cb85c8ab53c372..b7cdca90280305d2cd209e0ac86cf2d0b38301c6 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1670,6 +1670,8 @@ static struct dentry *lookup_dcache(const struct qstr *name,
  * dentries - as the matter of fact, this only gets called
  * when directory is guaranteed to have no in-lookup children
  * at all.
+ * Will return -ENOENT if name isn't found and LOOKUP_CREATE wasn't passed.
+ * Will return -EEXIST if name is found and LOOKUP_EXCL was passed.
  */
 struct dentry *lookup_one_qstr_excl(const struct qstr *name,
                                    struct dentry *base,
@@ -1680,7 +1682,7 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
        struct inode *dir = base->d_inode;
 
        if (dentry)
-               return dentry;
+               goto found;
 
        /* Don't create child dentry for a dead directory. */
        if (unlikely(IS_DEADDIR(dir)))
@@ -1695,6 +1697,17 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
                dput(dentry);
                dentry = old;
        }
+found:
+       if (IS_ERR(dentry))
+               return dentry;
+       if (d_is_negative(dentry) && !(flags & LOOKUP_CREATE)) {
+               dput(dentry);
+               return ERR_PTR(-ENOENT);
+       }
+       if (d_is_positive(dentry) && (flags & LOOKUP_EXCL)) {
+               dput(dentry);
+               return ERR_PTR(-EEXIST);
+       }
        return dentry;
 }
 EXPORT_SYMBOL(lookup_one_qstr_excl);
@@ -2741,10 +2754,6 @@ static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct
        }
        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);
                path_put(path);
@@ -4082,27 +4091,13 @@ static struct dentry *filename_create(int dfd, struct filename *name,
         * '/', and a directory wasn't requested.
         */
        if (last.name[last.len] && !want_dir)
-               create_flags = 0;
+               create_flags &= ~LOOKUP_CREATE;
        inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
        dentry = lookup_one_qstr_excl(&last, path->dentry,
                                      reval_flag | create_flags);
        if (IS_ERR(dentry))
                goto unlock;
 
-       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;
@@ -4449,10 +4444,6 @@ retry:
        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;
@@ -4583,7 +4574,7 @@ retry_deleg:
        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);
@@ -4617,9 +4608,7 @@ exit1:
        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;
@@ -5119,7 +5108,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
        struct qstr old_last, new_last;
        int old_type, new_type;
        struct inode *delegated_inode = NULL;
-       unsigned int lookup_flags = 0, target_flags = LOOKUP_RENAME_TARGET;
+       unsigned int lookup_flags = 0, target_flags =
+               LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
        bool should_retry = false;
        int error = -EINVAL;
 
@@ -5132,6 +5122,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
 
        if (flags & RENAME_EXCHANGE)
                target_flags = 0;
+       if (flags & RENAME_NOREPLACE)
+               target_flags |= LOOKUP_EXCL;
 
 retry:
        error = filename_parentat(olddfd, from, lookup_flags, &old_path,
@@ -5173,23 +5165,12 @@ retry_deleg:
        error = PTR_ERR(old_dentry);
        if (IS_ERR(old_dentry))
                goto exit3;
-       /* source must exist */
-       error = -ENOENT;
-       if (d_is_negative(old_dentry))
-               goto exit4;
        new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
                                          lookup_flags | target_flags);
        error = PTR_ERR(new_dentry);
        if (IS_ERR(new_dentry))
                goto exit4;
-       error = -EEXIST;
-       if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry))
-               goto exit5;
        if (flags & RENAME_EXCHANGE) {
-               error = -ENOENT;
-               if (d_is_negative(new_dentry))
-                       goto exit5;
-
                if (!d_is_dir(new_dentry)) {
                        error = -ENOTDIR;
                        if (new_last.name[new_last.len])

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 2b04038b0e40525be67f1ec512fb0db547e5f6ed..56cf16a7233483bd424415917acb9bb724c93b07 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1532,7 +1532,8 @@ static int nfs_is_exclusive_create(struct inode *dir, unsigned int flags)
 {
        if (NFS_PROTO(dir)->version == 2)
                return 0;
-       return flags & LOOKUP_EXCL;
+       return (flags & (LOOKUP_CREATE | LOOKUP_EXCL)) ==
+               (LOOKUP_CREATE | LOOKUP_EXCL);
 }
 
 /*

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 6890016e1923ed4f7dc1eeb5d83fe56650edc1df..fe29acef58724616f99fd3cdbd58762005518ad0 100644 (file)
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -113,11 +113,6 @@ static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf,
        if (IS_ERR(d))
                goto err_out;
 
-       if (d_is_negative(d)) {
-               dput(d);
-               goto err_out;
-       }
-
        path->dentry = d;
        path->mnt = mntget(parent_path->mnt);
 
@@ -693,6 +688,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
        struct ksmbd_file *parent_fp;
        int new_type;
        int err, lookup_flags = LOOKUP_NO_SYMLINKS;
+       int target_lookup_flags = LOOKUP_RENAME_TARGET;
 
        if (ksmbd_override_fsids(work))
                return -ENOMEM;
@@ -703,6 +699,14 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
                goto revert_fsids;
        }
 
+       /*
+        * explicitly handle file overwrite case, for compatibility with
+        * filesystems that may not support rename flags (e.g: fuse)
+        */
+       if (flags & RENAME_NOREPLACE)
+               target_lookup_flags |= LOOKUP_EXCL;
+       flags &= ~(RENAME_NOREPLACE);
+
 retry:
        err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,
                                     &new_path, &new_last, &new_type,
@@ -743,7 +747,7 @@ retry:
        }
 
        new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
-                                         lookup_flags | LOOKUP_RENAME_TARGET);
+                                         lookup_flags | target_lookup_flags);
        if (IS_ERR(new_dentry)) {
                err = PTR_ERR(new_dentry);
                goto out3;
@@ -754,16 +758,6 @@ retry:
                goto out4;
        }
 
-       /*
-        * explicitly handle file overwrite case, for compatibility with
-        * filesystems that may not support rename flags (e.g: fuse)
-        */
-       if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry)) {
-               err = -EEXIST;
-               goto out4;
-       }
-       flags &= ~(RENAME_NOREPLACE);
-
        if (old_child == trap) {
                err = -EINVAL;
                goto out4;