From: Al Viro <viro@zeniv.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] fix do_last() regression
Date: Sat, 1 Feb 2020 16:26:45 +0000 [thread overview]
Message-ID: <20200201162645.GJ23230@ZenIV.linux.org.uk> (raw)
Brown paperbag time: fetching ->i_uid/->i_mode really should've been
done from nd->inode. I even suggested that, but the reason for that
has slipped through the cracks and I went for dir->d_inode instead -
made for more "obvious" patch.
Analysis:
at the entry into do_last() and all the way to step_into(): dir
(aka nd->path.dentry) is known not to have been freed; so's nd->inode
and it's equal to dir->d_inode unless we are already doomed to -ECHILD.
inode of the file to get opened is not known.
after step_into(): inode of the file to get opened is known;
dir might be pointing to freed memory/be negative/etc.
at the call of may_create_in_sticky(): guaranteed to be out of
RCU mode; inode of the file to get opened is known and pinned;
dir might be garbage.
The last was the reason for the original patch. Except that at the do_last()
entry we can be in RCU mode and it is possible that nd->path.dentry->d_inode
has already changed under us. In that case we are going to fail with -ECHILD,
but we need to be careful; nd->inode is pointing to valid struct inode and
it's the same as nd->path.dentry->d_inode in "won't fail with -ECHILD"
case, so we should use that.
Reported-by: "Rantala, Tommi T. (Nokia - FI/Espoo)" <tommi.t.rantala@nokia.com>
Reported-by: syzbot+190005201ced78a74ad6@syzkaller.appspotmail.com
Wearing-brown-paperbag: Al Viro <viro@zeniv.linux.org.uk>
Cc: stable@kernel.org
Fixes: d0cb50185ae9 (do_last(): fetch directory ->i_mode and ->i_uid before it's too late)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namei.c b/fs/namei.c
index 4167109297e0..db6565c99825 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3333,8 +3333,8 @@ static int do_last(struct nameidata *nd,
struct file *file, const struct open_flags *op)
{
struct dentry *dir = nd->path.dentry;
- kuid_t dir_uid = dir->d_inode->i_uid;
- umode_t dir_mode = dir->d_inode->i_mode;
+ kuid_t dir_uid = nd->inode->i_uid;
+ umode_t dir_mode = nd->inode->i_mode;
int open_flag = op->open_flag;
bool will_truncate = (open_flag & O_TRUNC) != 0;
bool got_write = false;
next reply other threads:[~2020-02-01 16:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-01 16:26 Al Viro [this message]
2020-02-01 18:29 ` [PATCH] fix do_last() regression Linus Torvalds
2020-02-01 18:32 ` Al Viro
2020-02-01 18:37 ` Linus Torvalds
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=20200201162645.GJ23230@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.