linux-kernel - Re: [syzbot] [hfs] general protection fault in tomoyo_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANp29Y6uevNW1SmXi_5muEeruP0TVh9Y9xwhgKO==J3fh8oa=w@mail.gmail.com>
Date: Thu, 14 Mar 2024 17:21:30 +0100
From: Aleksandr Nogikh <nogikh@...gle.com>
To: Jan Kara <jack@...e.cz>
Cc: syzbot <syzbot+28aaddd5a3221d7fd709@...kaller.appspotmail.com>, axboe@...nel.dk, 
	brauner@...nel.org, jmorris@...ei.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, 
	paul@...l-moore.com, serge@...lyn.com, syzkaller-bugs@...glegroups.com, 
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>, Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [syzbot] [hfs] general protection fault in tomoyo_check_acl (3)

Hi Jan,

Yes, the CONFIG_BLK_DEV_WRITE_MOUNTED=n change did indeed break our C
executor code (and therefore our C reproducers). I posted a fix[1]
soon afterwards, but the problem is that syzbot will keep on using old
reproducers for old bugs. Syzkaller descriptions change over time, so
during bisection and patch testing we have to use the exact syzkaller
revision that detected the original bug. All older syzkaller revisions
now neither find nor reproduce fs bugs on newer Linux kernel revisions
with CONFIG_BLK_DEV_WRITE_MOUNTED=n.

If the stream of such bisection results is already bothering you and
other fs people, a very quick fix could be to ban this commit from the
possible bisection results (it's just a one line change in the syzbot
config). Then such bugs would just get gradually obsoleted by syzbot
without any noise.

[1] https://github.com/google/syzkaller/commit/551587c192ecb4df26fcdab775ed145ee69c07d4

-- 
Aleksandr

On Thu, Mar 14, 2024 at 4:54 PM Jan Kara <jack@...e.cz> wrote:
>
> On Sun 10-03-24 09:52:01, Tetsuo Handa wrote:
> > On 2024/01/11 18:21, Jan Kara wrote:
> > > On Wed 10-01-24 22:44:04, syzbot wrote:
> > >> syzbot suspects this issue was fixed by commit:
> > >>
> > >> commit 6f861765464f43a71462d52026fbddfc858239a5
> > >> Author: Jan Kara <jack@...e.cz>
> > >> Date:   Wed Nov 1 17:43:10 2023 +0000
> > >>
> > >>     fs: Block writes to mounted block devices
> > >>
> > >> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=15135c0be80000
> > >> start commit:   a901a3568fd2 Merge tag 'iomap-6.5-merge-1' of git://git.ke..
> > >> git tree:       upstream
> > >> kernel config:  https://syzkaller.appspot.com/x/.config?x=7406f415f386e786
> > >> dashboard link: https://syzkaller.appspot.com/bug?extid=28aaddd5a3221d7fd709
> > >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17b5bb80a80000
> > >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10193ee7280000
> > >>
> > >> If the result looks correct, please mark the issue as fixed by replying with:
> > >
> > > Makes some sense since fs cannot be corrupted by anybody while it is
> > > mounted. I just don't see how the reproducer would be corrupting the
> > > image... Still probably:
> > >
> > > #syz fix: fs: Block writes to mounted block devices
> > >
> > > and we'll see if syzbot can find new ways to tickle some similar problem.
> > >
> > >                                                             Honza
> >
> > Since the reproducer is doing open(O_RDWR) before switching loop devices
> > using ioctl(LOOP_SET_FD/LOOP_CLR_FD), I think that that commit converted
> > a run many times, multi threaded program into a run once, single threaded
> > program. That will likely hide all race bugs.
> >
> > Does that commit also affect open(3) (i.e. open for ioctl only) case?
> > If that commit does not affect open(3) case, the reproducer could continue
> > behaving as run many times, multi threaded program that overwrites
> > filesystem images using ioctl(LOOP_SET_FD/LOOP_CLR_FD), by replacing
> > open(O_RDWR) with open(3) ?
>
> Hum, that's a good point. I had a look into details how syskaller sets up
> loop devices and indeed it gets broken by CONFIG_BLK_DEV_WRITE_MOUNTED=n.
> Strace confirms that:
>
> openat(AT_FDCWD, "/dev/loop0", O_RDWR)  = 4
> ioctl(4, LOOP_SET_FD, 3)                = 0
> close(3)                                = 0
> mkdir("./file0", 0777)                  = -1 EEXIST (File exists)
> mount("/dev/loop0", "./file0", "reiserfs", 0, "") = -1 EBUSY (Device or resource busy)
> ioctl(4, LOOP_CLR_FD)                   = 0
> close(4)                                = 0
>
> which explains why syzbot was not able to reproduce some problems for which
> CONFIG_BLK_DEV_WRITE_MOUNTED=n should have made no difference (I wanted to
> have a look into that but other things kept getting higher priority).
>
> It should be easily fixable by opening /dev/loop0 with O_RDONLY instead of
> O_RDWR. Aleksandr?
>
>                                                                 Honza
>
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR