Re: [syzbot] kernel BUG in dnotify_free_mark

[Jan Kara <jack@suse.cz>]

On Mon 31-10-22 20:18:25, Amir Goldstein wrote:
> On Mon, Oct 31, 2022 at 7:50 PM Jan Kara <jack@suse.cz> wrote:
> > [added some CCs to gather more ideas]
> >
> > On Fri 28-10-22 16:45:33, syzbot wrote:
> > > syzbot found the following issue on:
> > >
> > > HEAD commit:    247f34f7b803 Linux 6.1-rc2
> > > git tree:       upstream
> > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=157f594a880000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=1d3548a4365ba17d
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=06cc05ddc896f12b7ec5
> > > compiler:       Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15585936880000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ec85ba880000
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/a5f39164dea4/disk-247f34f7.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/8d1b92f5a01f/vmlinux-247f34f7.xz
> > > mounted in repro: https://storage.googleapis.com/syzbot-assets/1a4d2943796c/mount_0.gz
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+06cc05ddc896f12b7ec5@syzkaller.appspotmail.com
> > >
> > > ------------[ cut here ]------------
> > > kernel BUG at fs/notify/dnotify/dnotify.c:136!
> >
> > OK, I've tracked this down to the problem in ntfs3 driver or maybe more
> > exactly in bad inode handling. What the reproducer does is that it mounts
> > ntfs3 image, places dnotify mark on filesystem's /, then accesses something
> > which finds that / is corrupted.  This calls ntfs_bad_inode() which calls
> > make_bad_inode() which sets inode->i_mode to S_IFREG. So when the file
> > descriptor is closed, dnotify doesn't get properly shutdown because it
> > works only on directories. Now calling make_bad_inode() on live inode is
> > problematic because it can change inode type (e.g. from directory to
> > regular file) and that tends to confuse things - dnotify in this case.
> >
> > Now it is easy to blame filesystem driver for calling make_bad_inode() on
> > live inode but given it seems to be relatively widespread maybe
> > make_bad_inode() should be more careful not to screw VFS? What do other
> > people think?
> 
> Do you know why make_bad_inode() sets inode->i_mode to S_IFREG?

I suppose because i_mode can be set to some bogus value (e.g. when
make_bad_inode() is called while reading the inode from the disk). One idea
I had was that we'd do this setting only if i_mode was indeed invalid. But
note that make_bad_inode() also sets inode->i_op and inode->i_fop and that
can also cause some surprises for a live inode (e.g. if some concurrent
process is in the middle of some operation on the inode).

> If it did not do that, would it solve the dnotify issue?

Yes, if i_mode was kept untouched, dnotify problem would be fixed.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR