namei: change filename_parentat() calling conventions

Since commit 5c31b6cedb675 ("namei: saner calling conventions for
filename_parentat()") filename_parentat() had the following behavior WRT
the passed in struct filename *:

* On error the name is consumed (putname() is called on it);
* On success the name is returned back as the return value;

Now there is a need for filename_create() and filename_lookup() variants
that do not consume the passed filename, and following the same "consume
the name only on error" semantics is proven to be hard to reason about
and result in confusing code.

Hence this preparation change splits filename_parentat() into two: one
that always consumes the name and another that never consumes the name.
This will allow to implement two filename_create() variants in the same
way, and is a consistent and hopefully easier to reason about approach.

Link: https://lore.kernel.org/io-uring/CAOKbgA7MiqZAq3t-HDCpSGUFfco4hMA9ArAE-74fTpU+EkvKPw@mail.gmail.com/
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com>
Link: https://lore.kernel.org/r/20210708063447.3556403-3-dkadashev@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/fs/namei.c b/fs/namei.c
index dc36bda..aaba6b4 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2498,7 +2498,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
        return err;
 }
 
        return err;
 }
 

-static struct filename *filename_parentat(int dfd, struct filename *name,
+static int __filename_parentat(int dfd, struct filename *name,

                                unsigned int flags, struct path *parent,
                                struct qstr *last, int *type)
 {
                                unsigned int flags, struct path *parent,
                                struct qstr *last, int *type)
 {


@@ -2506,7 +2506,7 @@ static struct filename *filename_parentat(int dfd, struct filename *name,
        struct nameidata nd;
 
        if (IS_ERR(name))
        struct nameidata nd;
 
        if (IS_ERR(name))

-               return name;
+               return PTR_ERR(name);

        set_nameidata(&nd, dfd, name, NULL);
        retval = path_parentat(&nd, flags | LOOKUP_RCU, parent);
        if (unlikely(retval == -ECHILD))
        set_nameidata(&nd, dfd, name, NULL);
        retval = path_parentat(&nd, flags | LOOKUP_RCU, parent);
        if (unlikely(retval == -ECHILD))


@@ -2517,29 +2517,34 @@ static struct filename *filename_parentat(int dfd, struct filename *name,
                *last = nd.last;
                *type = nd.last_type;
                audit_inode(name, parent->dentry, AUDIT_INODE_PARENT);
                *last = nd.last;
                *type = nd.last_type;
                audit_inode(name, parent->dentry, AUDIT_INODE_PARENT);

-       } else {
-               putname(name);
-               name = ERR_PTR(retval);

        }
        restore_nameidata();
        }
        restore_nameidata();

-       return name;
+       return retval;
+}
+
+static int filename_parentat(int dfd, struct filename *name,
+                               unsigned int flags, struct path *parent,
+                               struct qstr *last, int *type)
+{
+       int retval = __filename_parentat(dfd, name, flags, parent, last, type);
+
+       putname(name);
+       return retval;

 }
 
 /* does lookup, returns the object with parent locked */
 struct dentry *kern_path_locked(const char *name, struct path *path)
 {
 }
 
 /* does lookup, returns the object with parent locked */
 struct dentry *kern_path_locked(const char *name, struct path *path)
 {

-       struct filename *filename;

        struct dentry *d;
        struct qstr last;
        struct dentry *d;
        struct qstr last;

-       int type;
+       int type, error;

 
 

-       filename = filename_parentat(AT_FDCWD, getname_kernel(name), 0, path,
+       error = filename_parentat(AT_FDCWD, getname_kernel(name), 0, path,

                                    &last, &type);
                                    &last, &type);

-       if (IS_ERR(filename))
-               return ERR_CAST(filename);
+       if (error)
+               return ERR_PTR(error);

        if (unlikely(type != LAST_NORM)) {
                path_put(path);
        if (unlikely(type != LAST_NORM)) {
                path_put(path);

-               putname(filename);

                return ERR_PTR(-EINVAL);
        }
        inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
                return ERR_PTR(-EINVAL);
        }
        inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);


@@ -2548,7 +2553,6 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
                inode_unlock(path->dentry->d_inode);
                path_put(path);
        }
                inode_unlock(path->dentry->d_inode);
                path_put(path);
        }

-       putname(filename);

        return d;
 }
 
        return d;
 }
 


@@ -3585,9 +3589,9 @@ static struct dentry *filename_create(int dfd, struct filename *name,
         */
        lookup_flags &= LOOKUP_REVAL;
 
         */
        lookup_flags &= LOOKUP_REVAL;
 

-       name = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
-       if (IS_ERR(name))
-               return ERR_CAST(name);
+       error = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
+       if (error)
+               return ERR_PTR(error);

 
        /*
         * Yucky last component or no last component at all?
 
        /*
         * Yucky last component or no last component at all?


@@ -3625,7 +3629,6 @@ static struct dentry *filename_create(int dfd, struct filename *name,
                error = err2;
                goto fail;
        }
                error = err2;
                goto fail;
        }

-       putname(name);

        return dentry;
 fail:
        dput(dentry);
        return dentry;
 fail:
        dput(dentry);


@@ -3636,7 +3639,6 @@ unlock:
                mnt_drop_write(path->mnt);
 out:
        path_put(path);
                mnt_drop_write(path->mnt);
 out:
        path_put(path);

-       putname(name);

        return dentry;
 }
 
        return dentry;
 }
 


@@ -3927,59 +3929,59 @@ EXPORT_SYMBOL(vfs_rmdir);
 long do_rmdir(int dfd, struct filename *name)
 {
        struct user_namespace *mnt_userns;
 long do_rmdir(int dfd, struct filename *name)
 {
        struct user_namespace *mnt_userns;

-       int error = 0;
+       int error;

        struct dentry *dentry;
        struct path path;
        struct qstr last;
        int type;
        unsigned int lookup_flags = 0;
 retry:
        struct dentry *dentry;
        struct path path;
        struct qstr last;
        int type;
        unsigned int lookup_flags = 0;
 retry:

-       name = filename_parentat(dfd, name, lookup_flags,
-                               &path, &last, &type);
-       if (IS_ERR(name))
-               return PTR_ERR(name);
+       error = __filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
+       if (error)
+               goto exit1;

 
        switch (type) {
        case LAST_DOTDOT:
                error = -ENOTEMPTY;
 
        switch (type) {
        case LAST_DOTDOT:
                error = -ENOTEMPTY;

-               goto exit1;
+               goto exit2;

        case LAST_DOT:
                error = -EINVAL;
        case LAST_DOT:
                error = -EINVAL;

-               goto exit1;
+               goto exit2;

        case LAST_ROOT:
                error = -EBUSY;
        case LAST_ROOT:
                error = -EBUSY;

-               goto exit1;
+               goto exit2;

        }
 
        error = mnt_want_write(path.mnt);
        if (error)
        }
 
        error = mnt_want_write(path.mnt);
        if (error)

-               goto exit1;
+               goto exit2;

 
        inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
        dentry = __lookup_hash(&last, path.dentry, lookup_flags);
        error = PTR_ERR(dentry);
        if (IS_ERR(dentry))
 
        inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
        dentry = __lookup_hash(&last, path.dentry, lookup_flags);
        error = PTR_ERR(dentry);
        if (IS_ERR(dentry))

-               goto exit2;
+               goto exit3;

        if (!dentry->d_inode) {
                error = -ENOENT;
        if (!dentry->d_inode) {
                error = -ENOENT;

-               goto exit3;
+               goto exit4;

        }
        error = security_path_rmdir(&path, dentry);
        if (error)
        }
        error = security_path_rmdir(&path, dentry);
        if (error)

-               goto exit3;
+               goto exit4;

        mnt_userns = mnt_user_ns(path.mnt);
        error = vfs_rmdir(mnt_userns, path.dentry->d_inode, dentry);
        mnt_userns = mnt_user_ns(path.mnt);
        error = vfs_rmdir(mnt_userns, path.dentry->d_inode, dentry);

-exit3:
+exit4:

        dput(dentry);
        dput(dentry);

-exit2:
+exit3:

        inode_unlock(path.dentry->d_inode);
        mnt_drop_write(path.mnt);
        inode_unlock(path.dentry->d_inode);
        mnt_drop_write(path.mnt);

-exit1:
+exit2:

        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
                goto retry;
        }
        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
                goto retry;
        }

+exit1:

        putname(name);
        return error;
 }
        putname(name);
        return error;
 }


@@ -4073,17 +4075,17 @@ long do_unlinkat(int dfd, struct filename *name)
        struct inode *delegated_inode = NULL;
        unsigned int lookup_flags = 0;
 retry:
        struct inode *delegated_inode = NULL;
        unsigned int lookup_flags = 0;
 retry:

-       name = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
-       if (IS_ERR(name))
-               return PTR_ERR(name);
+       error = __filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
+       if (error)
+               goto exit1;

 
        error = -EISDIR;
        if (type != LAST_NORM)
 
        error = -EISDIR;
        if (type != LAST_NORM)

-               goto exit1;
+               goto exit2;

 
        error = mnt_want_write(path.mnt);
        if (error)
 
        error = mnt_want_write(path.mnt);
        if (error)

-               goto exit1;
+               goto exit2;

 retry_deleg:
        inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
        dentry = __lookup_hash(&last, path.dentry, lookup_flags);
 retry_deleg:
        inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
        dentry = __lookup_hash(&last, path.dentry, lookup_flags);


@@ -4100,11 +4102,11 @@ retry_deleg:
                ihold(inode);
                error = security_path_unlink(&path, dentry);
                if (error)
                ihold(inode);
                error = security_path_unlink(&path, dentry);
                if (error)

-                       goto exit2;
+                       goto exit3;

                mnt_userns = mnt_user_ns(path.mnt);
                error = vfs_unlink(mnt_userns, path.dentry->d_inode, dentry,
                                   &delegated_inode);
                mnt_userns = mnt_user_ns(path.mnt);
                error = vfs_unlink(mnt_userns, path.dentry->d_inode, dentry,
                                   &delegated_inode);

-exit2:
+exit3:

                dput(dentry);
        }
        inode_unlock(path.dentry->d_inode);
                dput(dentry);
        }
        inode_unlock(path.dentry->d_inode);


@@ -4117,13 +4119,14 @@ exit2:
                        goto retry_deleg;
        }
        mnt_drop_write(path.mnt);
                        goto retry_deleg;
        }
        mnt_drop_write(path.mnt);

-exit1:
+exit2:

        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
                inode = NULL;
                goto retry;
        }
        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
                inode = NULL;
                goto retry;
        }

+exit1:

        putname(name);
        return error;
 
        putname(name);
        return error;
 


@@ -4134,7 +4137,7 @@ slashes:
                error = -EISDIR;
        else
                error = -ENOTDIR;
                error = -EISDIR;
        else
                error = -ENOTDIR;

-       goto exit2;
+       goto exit3;

 }
 
 SYSCALL_DEFINE3(unlinkat, int, dfd, const char __user *, pathname, int, flag)
 }
 
 SYSCALL_DEFINE3(unlinkat, int, dfd, const char __user *, pathname, int, flag)


@@ -4605,29 +4608,25 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
        int error = -EINVAL;
 
        if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
        int error = -EINVAL;
 
        if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))

-               goto put_both;
+               goto put_names;

 
        if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
            (flags & RENAME_EXCHANGE))
 
        if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
            (flags & RENAME_EXCHANGE))

-               goto put_both;
+               goto put_names;

 
        if (flags & RENAME_EXCHANGE)
                target_flags = 0;
 
 retry:
 
        if (flags & RENAME_EXCHANGE)
                target_flags = 0;
 
 retry:

-       from = filename_parentat(olddfd, from, lookup_flags, &old_path,
+       error = __filename_parentat(olddfd, from, lookup_flags, &old_path,

                                        &old_last, &old_type);
                                        &old_last, &old_type);

-       if (IS_ERR(from)) {
-               error = PTR_ERR(from);
-               goto put_new;
-       }
+       if (error)
+               goto put_names;

 
 

-       to = filename_parentat(newdfd, to, lookup_flags, &new_path, &new_last,
+       error = __filename_parentat(newdfd, to, lookup_flags, &new_path, &new_last,

                                &new_type);
                                &new_type);

-       if (IS_ERR(to)) {
-               error = PTR_ERR(to);
+       if (error)

                goto exit1;
                goto exit1;

-       }

 
        error = -EXDEV;
        if (old_path.mnt != new_path.mnt)
 
        error = -EXDEV;
        if (old_path.mnt != new_path.mnt)


@@ -4730,9 +4729,8 @@ exit1:
                lookup_flags |= LOOKUP_REVAL;
                goto retry;
        }
                lookup_flags |= LOOKUP_REVAL;
                goto retry;
        }

-put_both:
+put_names:

        putname(from);
        putname(from);

-put_new:

        putname(to);
        return error;
 }
        putname(to);
        return error;
 }