VFS: tidy up do_unlinkat()

The simplification of locking in the previous patch opens up some room
for tidying up do_unlinkat()

- change all "exit" labels to describe what will happen at the label.
- always goto an exit label on an error - unwrap the "if (!IS_ERR())" branch.
- Move the "slashes" handing inline, but mark it as unlikely()
- simplify use of the "inode" variable - we no longer need to test for NULL.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20251113002050.676694-4-neilb@ownmail.net
Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index 3618efd4bcaa33f4e8994eb29365c94a6473b532..9effaad115d9b4ff9e746671cd05c5ba9737c254 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4755,65 +4755,62 @@ int do_unlinkat(int dfd, struct filename *name)
        struct path path;
        struct qstr last;
        int type;
-       struct inode *inode = NULL;
+       struct inode *inode;
        struct inode *delegated_inode = NULL;
        unsigned int lookup_flags = 0;
 retry:
        error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
        if (error)
-               goto exit1;
+               goto exit_putname;
 
        error = -EISDIR;
        if (type != LAST_NORM)
-               goto exit2;
+               goto exit_path_put;
 
        error = mnt_want_write(path.mnt);
        if (error)
-               goto exit2;
+               goto exit_path_put;
 retry_deleg:
        dentry = start_dirop(path.dentry, &last, lookup_flags);
        error = PTR_ERR(dentry);
-       if (!IS_ERR(dentry)) {
+       if (IS_ERR(dentry))
+               goto exit_drop_write;
 
-               /* Why not before? Because we want correct error value */
-               if (last.name[last.len])
-                       goto slashes;
-               inode = dentry->d_inode;
-               ihold(inode);
-               error = security_path_unlink(&path, dentry);
-               if (error)
-                       goto exit3;
-               error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
-                                  dentry, &delegated_inode);
-exit3:
+       /* Why not before? Because we want correct error value */
+       if (unlikely(last.name[last.len])) {
+               if (d_is_dir(dentry))
+                       error = -EISDIR;
+               else
+                       error = -ENOTDIR;
                end_dirop(dentry);
+               goto exit_drop_write;
        }
-       if (inode)
-               iput(inode);    /* truncate the inode here */
-       inode = NULL;
+       inode = dentry->d_inode;
+       ihold(inode);
+       error = security_path_unlink(&path, dentry);
+       if (error)
+               goto exit_end_dirop;
+       error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
+                          dentry, &delegated_inode);
+exit_end_dirop:
+       end_dirop(dentry);
+       iput(inode);    /* truncate the inode here */
        if (delegated_inode) {
                error = break_deleg_wait(&delegated_inode);
                if (!error)
                        goto retry_deleg;
        }
+exit_drop_write:
        mnt_drop_write(path.mnt);
-exit2:
+exit_path_put:
        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
-               inode = NULL;
                goto retry;
        }
-exit1:
+exit_putname:
        putname(name);
        return error;
-
-slashes:
-       if (d_is_dir(dentry))
-               error = -EISDIR;
-       else
-               error = -ENOTDIR;
-       goto exit3;
 }
 
 SYSCALL_DEFINE3(unlinkat, int, dfd, const char __user *, pathname, int, flag)