public inbox for [email protected]
 help / color / mirror / Atom feed
From: Dmitry Kadashev <[email protected]>
To: [email protected]
Subject: Use of disowned struct filename after 3c5499fa56f5?
Date: Thu, 5 Nov 2020 19:36:28 +0700	[thread overview]
Message-ID: <CAOKbgA5ojRs0xuor9TEtBEHUfhEj5sJewDoNgsbAYruhrFmPQw@mail.gmail.com> (raw)

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

             reply	other threads:[~2020-11-05 12:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05 12:36 Dmitry Kadashev [this message]
2020-11-05 14:22 ` Use of disowned struct filename after 3c5499fa56f5? Pavel Begunkov
2020-11-05 14:26   ` Pavel Begunkov
2020-11-05 14:55   ` Pavel Begunkov
2020-11-05 19:37     ` Jens Axboe
2020-11-05 20:04       ` Pavel Begunkov
2020-11-05 20:18         ` Pavel Begunkov
2020-11-05 20:26         ` Jens Axboe
2020-11-05 20:35           ` Pavel Begunkov
2020-11-05 20:49             ` Jens Axboe
2020-11-05 20:57               ` Pavel Begunkov
2020-11-05 21:12                 ` Jens Axboe
2020-11-06 10:08                 ` Dmitry Kadashev
2020-11-06 12:49                   ` Pavel Begunkov
2020-11-06 13:15                     ` Dmitry Kadashev
2020-11-06 13:27                       ` Pavel Begunkov
2020-11-06 13:35                         ` Dmitry Kadashev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOKbgA5ojRs0xuor9TEtBEHUfhEj5sJewDoNgsbAYruhrFmPQw@mail.gmail.com \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox