Received-SPF: pass (google.com: domain of linux-kernel+bounces-103564-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <000000000000fcfb4a05ffe48213@google.com> <0000000000009e1b00060ea5df51@google.com>
 <20240111092147.ywwuk4vopsml3plk@quack3> <bbeeb617-6730-4159-80b1-182841925cce@I-love.SAKURA.ne.jp>
 <20240314155417.aysvaktvvqxc34zb@quack3>
In-Reply-To: <20240314155417.aysvaktvvqxc34zb@quack3>
From: Aleksandr Nogikh <nogikh@google.com>
Date: Thu, 14 Mar 2024 17:21:30 +0100
Message-ID: <CANp29Y6uevNW1SmXi_5muEeruP0TVh9Y9xwhgKO==J3fh8oa=w@mail.gmail.com>
Subject: Re: [syzbot] [hfs] general protection fault in tomoyo_check_acl (3)
To: Jan Kara <jack@suse.cz>
Cc: syzbot <syzbot+28aaddd5a3221d7fd709@syzkaller.appspotmail.com>, axboe@kernel.dk, 
	brauner@kernel.org, jmorris@namei.org, linux-fsdevel@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, 
	paul@paul-moore.com, serge@hallyn.com, syzkaller-bugs@googlegroups.com, 
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>, Dmitry Vyukov <dvyukov@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Jan,

Yes, the CONFIG_BLK_DEV_WRITE_MOUNTED=3Dn 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=3Dn.

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/551587c192ecb4df26fcdab775ed=
145ee69c07d4

--=20
Aleksandr

On Thu, Mar 14, 2024 at 4:54=E2=80=AFPM Jan Kara <jack@suse.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@suse.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=3D15135=
c0be80000
> > >> 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=3D7406f415=
f386e786
> > >> dashboard link: https://syzkaller.appspot.com/bug?extid=3D28aaddd5a3=
221d7fd709
> > >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=3D17b5bb=
80a80000
> > >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=3D10193ee7=
280000
> > >>
> > >> If the result looks correct, please mark the issue as fixed by reply=
ing 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 prob=
lem.
> > >
> > >                                                             Honza
> >
> > Since the reproducer is doing open(O_RDWR) before switching loop device=
s
> > using ioctl(LOOP_SET_FD/LOOP_CLR_FD), I think that that commit converte=
d
> > a run many times, multi threaded program into a run once, single thread=
ed
> > 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 conti=
nue
> > 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=3D=
n.
> Strace confirms that:
>
> openat(AT_FDCWD, "/dev/loop0", O_RDWR)  =3D 4
> ioctl(4, LOOP_SET_FD, 3)                =3D 0
> close(3)                                =3D 0
> mkdir("./file0", 0777)                  =3D -1 EEXIST (File exists)
> mount("/dev/loop0", "./file0", "reiserfs", 0, "") =3D -1 EBUSY (Device or=
 resource busy)
> ioctl(4, LOOP_CLR_FD)                   =3D 0
> close(4)                                =3D 0
>
> which explains why syzbot was not able to reproduce some problems for whi=
ch
> CONFIG_BLK_DEV_WRITE_MOUNTED=3Dn 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 o=
f
> O_RDWR. Aleksandr?
>
>                                                                 Honza
>
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR