From: Jan Kara <jack@suse.cz>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Jan Kara <jack@suse.cz>, Mark Fasheh <mark@fasheh.com>,
Dave Chinner <david@fromorbit.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
syzbot <syzbot+7a8ba368b47fdefca61e@syzkaller.appspotmail.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Joel Becker <jlbec@evilplan.org>
Subject: Re: KASAN: use-after-free Read in path_lookupat
Date: Thu, 28 Mar 2019 10:00:45 +0100 [thread overview]
Message-ID: <20190328090045.GA22915@quack2.suse.cz> (raw)
In-Reply-To: <20190327185948.GC2217@ZenIV.linux.org.uk>
On Wed 27-03-19 18:59:48, Al Viro wrote:
> On Wed, Mar 27, 2019 at 05:58:31PM +0100, Jan Kara wrote:
> > On Tue 26-03-19 04:15:10, Al Viro wrote:
> > > On Mon, Mar 25, 2019 at 08:18:25PM -0700, Mark Fasheh wrote:
> > >
> > > > Hey Al,
> > > >
> > > > It's been a while since I've looked at that bit of code but it looks like
> > > > Ocfs2 is syncing the inode to disk and disposing of it's memory
> > > > representation (which would include the cluster locks held) so that other
> > > > nodes get a chance to delete the potentially orphaned inode. In Ocfs2 we
> > > > won't delete an inode if it exists in another nodes cache.
> > >
> > > Wait a sec - what's the reason for forcing that write_inode_now(); why
> > > doesn't the normal mechanism work? I'm afraid I still don't get it -
> > > we do wait for writeback in evict_inode(), or the local filesystems
> > > wouldn't work.
> >
> > I'm just guessing here but they don't want an inode cached once its last
> > dentry goes away (it makes cluster wide synchronization easier for them and
> > they do play tricks with cluster lock on dentries).
>
> Sure, but that's as simple as "return 1 from ->drop_inode()".
Right.
> > There is some info in
> > 513e2dae9422 "ocfs2: flush inode data to disk and free inode when i_count
> > becomes zero" which adds this ocfs2_drop_inode() implementation. So when
> > the last inode reference is dropped, they want to flush any dirty data to
> > disk and evict the inode. But AFAICT they should be fine with flushing the
> > inode from their ->evict_inode method. I_FREEING just stops the flusher
> > thread from touching the inode but explicit writeback through
> > write_inode_now(inode, 1) should go through just fine.
>
> Umm... Why is that write_inode_now() needed in either place? I agree that
> moving it to ->evict_inode() ought to be safe, but what makes it necessary
> in the first place? Put it another way, what dirties the data and/or
> metadata without marking it dirty?
Well, the inode & pages are marked dirty and they are dirty when we get to
iput_final(). But if ->drop_inode() returns 1 (which normally happens only
for unlinked files), we will not write out the inode in iput_final() and
the dirty data just gets discarded in ->evict_inode(). OCFS2 doesn't want
this so they have to write-out by hand.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2019-03-28 9:00 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-28 17:40 KASAN: use-after-free Read in path_lookupat syzbot
2019-03-25 0:44 ` syzbot
2019-03-25 1:25 ` Linus Torvalds
2019-03-25 1:23 ` Linus Torvalds
2019-03-25 4:57 ` Al Viro
2019-03-25 9:15 ` Daniel Borkmann
2019-03-25 11:11 ` Al Viro
2019-03-25 11:17 ` Al Viro
2019-03-25 11:21 ` Daniel Borkmann
2019-03-25 18:36 ` Linus Torvalds
2019-03-25 19:18 ` Linus Torvalds
2019-03-25 21:14 ` Al Viro
2019-03-25 21:45 ` Linus Torvalds
2019-03-25 22:04 ` Daniel Borkmann
2019-03-25 22:13 ` Linus Torvalds
2019-03-25 22:41 ` Daniel Borkmann
2019-03-25 22:49 ` Al Viro
2019-03-25 23:37 ` Al Viro
2019-03-25 23:44 ` Alexei Starovoitov
2019-03-26 0:21 ` Al Viro
2019-03-26 1:38 ` ceph: fix use-after-free on symlink traversal Al Viro
2019-03-26 1:39 ` jffs2: " Al Viro
2019-03-26 1:40 ` ubifs: " Al Viro
2019-03-26 1:43 ` debugfs: " Al Viro
2019-03-26 10:41 ` ceph: " Jeff Layton
2019-03-26 11:38 ` Ilya Dryomov
2019-03-26 1:45 ` KASAN: use-after-free Read in path_lookupat Al Viro
2019-04-10 18:11 ` Al Viro
2019-04-10 19:44 ` Linus Torvalds
2019-03-25 19:43 ` Al Viro
2019-03-25 22:48 ` Dave Chinner
2019-03-25 23:02 ` Al Viro
[not found] ` <CAGe7X7mb=gK7zhSwmT_6mmmkcbjhZAOb=wj31BdUcHkNUPsm2Q@mail.gmail.com>
2019-03-26 4:15 ` Al Viro
2019-03-27 16:58 ` Jan Kara
2019-03-27 18:59 ` Al Viro
2019-03-28 9:00 ` Jan Kara [this message]
2019-03-27 17:22 ` Jan Kara
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=20190328090045.GA22915@quack2.suse.cz \
--to=jack@suse.cz \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=david@fromorbit.com \
--cc=jaegeuk@kernel.org \
--cc=jlbec@evilplan.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark@fasheh.com \
--cc=syzbot+7a8ba368b47fdefca61e@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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;
as well as URLs for NNTP newsgroup(s).