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@xxxxxxx> > >> 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@xxxxxxxx> SUSE Labs, CR
