Use of disowned struct filename after 3c5499fa56f5?

[Dmitry Kadashev <dkadashev@xxxxxxxxx>]

Hi Jens,

I am trying to implement mkdirat support in io_uring and was using
commit 3c5499fa56f5 ("fs: make do_renameat2() take struct filename") as
an example (kernel newbie here). But either I do not understand how it
works, or on retry struct filename is used that is not owned anymore
(and is probably freed).

Here is the relevant part of the patch:

diff --git a/fs/namei.c b/fs/namei.c
index d4a6dd772303..a696f99eef5c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4346,8 +4346,8 @@ int vfs_rename(struct inode *old_dir, struct
dentry *old_dentry,
 }
 EXPORT_SYMBOL(vfs_rename);

-static int do_renameat2(int olddfd, const char __user *oldname, int newdfd,
-                       const char __user *newname, unsigned int flags)
+int do_renameat2(int olddfd, struct filename *oldname, int newdfd,
+                struct filename *newname, unsigned int flags)
 {
        struct dentry *old_dentry, *new_dentry;
        struct dentry *trap;
@@ -4359,28 +4359,28 @@ static int do_renameat2(int olddfd, const char
__user *oldname, int newdfd,
        struct filename *to;
        unsigned int lookup_flags = 0, target_flags = LOOKUP_RENAME_TARGET;
        bool should_retry = false;
-       int error;
+       int error = -EINVAL;

        if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
-               return -EINVAL;
+               goto put_both;

        if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
            (flags & RENAME_EXCHANGE))
-               return -EINVAL;
+               goto put_both;

        if (flags & RENAME_EXCHANGE)
                target_flags = 0;

 retry:
-       from = filename_parentat(olddfd, getname(oldname), lookup_flags,
-                               &old_path, &old_last, &old_type);
+       from = filename_parentat(olddfd, oldname, lookup_flags, &old_path,
+                                       &old_last, &old_type);

With the new code on the first run oldname ownership is released. And if
we do end up on the retry path then it is used again erroneously (also
`from` was already put by that time).

Am I getting it wrong or is there a bug?

do_unlinkat that you reference does things a bit differently, as far as
I can tell the problem does not exist there.

Thanks,
Dmitry