Re: [syzbot] [hfs] general protection fault in tomoyo_check_acl (3)

[Jan Kara <jack@suse.cz>]

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=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@suse.com>
SUSE Labs, CR