On Mon 18-03-19 14:51:59, Dongli Zhang wrote: > > > On 3/18/19 11:36 AM, syzbot wrote: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit: 9c7dc824 Merge tag '5.1-rc-smb3' of git://git.samba.org/sf.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=148a35fb200000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=7e1aaa1cfbfe1abf > > dashboard link: https://syzkaller.appspot.com/bug?extid=9bdc1adc1c55e7fe765b > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > > > Unfortunately, I don't have any reproducer for this crash yet. > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+9bdc1adc1c55e7fe765b@xxxxxxxxxxxxxxxxxxxxxxxxx > > > > kasan: CONFIG_KASAN_INLINE enabled > > kasan: GPF could be caused by NULL-ptr deref or user memory access > > general protection fault: 0000 [#1] PREEMPT SMP KASAN > > CPU: 1 PID: 9499 Comm: syz-executor.4 Not tainted 5.0.0+ #25 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google > > 01/01/2011 > > RIP: 0010:loop_validate_file+0x23e/0x310 drivers/block/loop.c:652 > > Code: 00 48 89 f8 48 c1 e8 03 42 80 3c 38 00 0f 85 d4 00 00 00 4d 8b a4 24 f0 00 > > 00 00 49 8d bc 24 b8 01 00 00 48 89 f8 48 c1 e8 03 <42> 80 3c 38 00 0f 85 a8 00 > > 00 00 4d 8b a4 24 b8 01 00 00 4c 89 e0 > > RSP: 0018:ffff88804d2bfb18 EFLAGS: 00010202 > > RAX: 0000000000000037 RBX: ffff888095ffa3d8 RCX: ffffc9000e657000 > > RDX: 0000000000000077 RSI: ffffffff83e754ed RDI: 00000000000001b8 > > RBP: ffff88804d2bfb40 R08: ffff88808eab41c0 R09: fffffbfff11d981d > > R10: ffff88804d2bfb40 R11: ffffffff88ecc0e7 R12: 0000000000000000 > > R13: 0000000000000002 R14: ffff888095ffa800 R15: dffffc0000000000 > > FS: 00007f04b0c91700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 0000000000625208 CR3: 00000000a7af8000 CR4: 00000000001406e0 > > Call Trace: > > loop_set_fd drivers/block/loop.c:930 [inline] > > lo_ioctl+0x99d/0x2150 drivers/block/loop.c:1542 > > __blkdev_driver_ioctl block/ioctl.c:303 [inline] > > blkdev_ioctl+0xee8/0x1c40 block/ioctl.c:605 > > block_ioctl+0xee/0x130 fs/block_dev.c:1931 > > vfs_ioctl fs/ioctl.c:46 [inline] > > file_ioctl fs/ioctl.c:509 [inline] > > do_vfs_ioctl+0xd6e/0x1390 fs/ioctl.c:696 > > ksys_ioctl+0xab/0xd0 fs/ioctl.c:713 > > __do_sys_ioctl fs/ioctl.c:720 [inline] > > __se_sys_ioctl fs/ioctl.c:718 [inline] > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 > > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > RIP: 0033:0x458079 > > Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 > > d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b > > b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 > > RSP: 002b:00007f04b0c90c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000458079 > > RDX: 0000000000000004 RSI: 0000000000004c00 RDI: 0000000000000003 > > RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000 > > R10: 0000000000000000 R11: 0000000000000246 R12: 00007f04b0c916d4 > > R13: 00000000004c1252 R14: 00000000004d3160 R15: 00000000ffffffff > > Modules linked in: > > ---[ end trace 81b29486bae7280a ]--- > > RIP: 0010:loop_validate_file+0x23e/0x310 drivers/block/loop.c:652 > > Code: 00 48 89 f8 48 c1 e8 03 42 80 3c 38 00 0f 85 d4 00 00 00 4d 8b a4 24 f0 00 > > 00 00 49 8d bc 24 b8 01 00 00 48 89 f8 48 c1 e8 03 <42> 80 3c 38 00 0f 85 a8 00 > > 00 00 4d 8b a4 24 b8 01 00 00 4c 89 e0 > > RSP: 0018:ffff88804d2bfb18 EFLAGS: 00010202 > > RAX: 0000000000000037 RBX: ffff888095ffa3d8 RCX: ffffc9000e657000 > > RDX: 0000000000000077 RSI: ffffffff83e754ed RDI: 00000000000001b8 > > RBP: ffff88804d2bfb40 R08: ffff88808eab41c0 R09: fffffbfff11d981d > > R10: ffff88804d2bfb40 R11: ffffffff88ecc0e7 R12: 0000000000000000 > > R13: 0000000000000002 R14: ffff888095ffa800 R15: dffffc0000000000 > > FS: 00007f04b0c91700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 0000000000625208 CR3: 00000000a7af8000 CR4: 00000000001406e0 > > > > > > --- > > This bug is generated by a bot. It may contain errors. > > See https://goo.gl/tpsmEJ for more information about syzbot. > > syzbot engineers can be reached at syzkaller@xxxxxxxxxxxxxxxx. > > > > syzbot will keep track of this bug report. See: > > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. > > As discussed with Tetsuo in other email thread, the issue can hit when the > backend of the loop is another loop device, e.g., loop0's backend is a file, > while loop1's backend is loop0. > > > loop0's backend is file loop1's backend is loop0 > > __loop_clr_fd > mutex_lock(&loop_ctl_mutex); > lo->lo_backing_file = NULL; > mutex_unlock(&loop_ctl_mutex); > loop_set_fd() > mutex_lock_killable(&loop_ctl_mutex); > loop_validate_file > f = l->lo_backing_file; --> access if > loop0 is not Lo_unbound > mutex_lock(&loop_ctl_mutex); > lo->lo_flags = 0; > mutex_unlock(&loop_ctl_mutex); > > > > Here I post below for more feedback. We should use a loop device as backend only > when it is still bound. Yes, this is the right fix. Thanks for writing it! In fact, the problem has been introduced already in commit 7ccd0791d985 "loop: Push loop_ctl_mutex down into loop_clr_fd()" after which loop_validate_file() could see devices in Lo_rundown state with which it didn't count. It was harmless at that point but still. So when submitting the fix, please include there: Fixes: 7ccd0791d985 "loop: Push loop_ctl_mutex down into loop_clr_fd()" And also appropriate Reported-by for syzbot. Something like: Reported-by: syzbot+9bdc1adc1c55e7fe765b@xxxxxxxxxxxxxxxxxxxxxxxxx You can also add my: Reviewed-by: Jan Kara <jack@xxxxxxx> Thanks! Honza > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index 1e6edd5..bf1c61c 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -656,7 +656,7 @@ static int loop_validate_file(struct file *file, struct > block_device *bdev) > return -EBADF; > > l = f->f_mapping->host->i_bdev->bd_disk->private_data; > - if (l->lo_state == Lo_unbound) { > + if (l->lo_state != Lo_bound) { > return -EINVAL; > } > f = l->lo_backing_file; > > > Thanks Tetsuo for the discussion and feedback! > > Dongli Zhang -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR