* general protection fault in loop_validate_file (2) @ 2019-03-18 3:36 syzbot 2019-03-18 6:51 ` Dongli Zhang 2019-03-18 12:27 ` Dongli Zhang 0 siblings, 2 replies; 7+ messages in thread From: syzbot @ 2019-03-18 3:36 UTC (permalink / raw) To: axboe, linux-block, linux-kernel, syzkaller-bugs 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@syzkaller.appspotmail.com 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@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-18 3:36 general protection fault in loop_validate_file (2) syzbot @ 2019-03-18 6:51 ` Dongli Zhang 2019-03-18 10:44 ` Jan Kara 2019-03-18 12:27 ` Dongli Zhang 1 sibling, 1 reply; 7+ messages in thread From: Dongli Zhang @ 2019-03-18 6:51 UTC (permalink / raw) To: linux-block, jack Cc: syzbot, axboe, linux-kernel, syzkaller-bugs, Tetsuo Handa 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@syzkaller.appspotmail.com > > 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@googlegroups.com. > > 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. 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-18 6:51 ` Dongli Zhang @ 2019-03-18 10:44 ` Jan Kara 0 siblings, 0 replies; 7+ messages in thread From: Jan Kara @ 2019-03-18 10:44 UTC (permalink / raw) To: Dongli Zhang Cc: linux-block, jack, syzbot, axboe, linux-kernel, syzkaller-bugs, Tetsuo Handa 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@syzkaller.appspotmail.com > > > > 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@googlegroups.com. > > > > 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@syzkaller.appspotmail.com You can also add my: Reviewed-by: Jan Kara <jack@suse.cz> 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@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-18 3:36 general protection fault in loop_validate_file (2) syzbot 2019-03-18 6:51 ` Dongli Zhang @ 2019-03-18 12:27 ` Dongli Zhang 2019-03-19 9:41 ` Jan Kara 1 sibling, 1 reply; 7+ messages in thread From: Dongli Zhang @ 2019-03-18 12:27 UTC (permalink / raw) To: Jan Kara Cc: syzbot, axboe, linux-block, linux-kernel, syzkaller-bugs, penguin-kernel Hi Jan, Indeed there is another issue implicitly reported by below console output from syzkaller: [ 245.524424][ T9455] loop_reread_partitions: partition scan of loop0 () failed (rc=-13) [ 245.563340][ T9499] kasan: CONFIG_KASAN_INLINE enabled [ 245.576412][ T9489] __loop_clr_fd: partition scan of loop0 failed (rc=-13) [ 245.581275][ T9499] kasan: GPF could be caused by NULL-ptr deref or user memory access [ 245.602596][ T9499] general protection fault: 0000 [#1] PREEMPT SMP KASAN I think rc=-13 is because of below code at line 168: 162 int __blkdev_reread_part(struct block_device *bdev) 163 { 164 struct gendisk *disk = bdev->bd_disk; 165 166 if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) 167 return -EINVAL; 168 if (!capable(CAP_SYS_ADMIN)) 169 return -EACCES; 170 171 lockdep_assert_held(&bdev->bd_mutex); 172 173 return rescan_partitions(disk, bdev); 174 } 175 EXPORT_SYMBOL(__blkdev_reread_part); I can reproduce this by 'chown username /dev/loop0' on my test machine. Taking 'losetup -d /dev/loop0' as sample, as /dev/loop0 belongs to my username, I am able to detach the loop without 'su'. However, because of above line 168, the partition scan would fail. Should we always assume the user should have admin privilege to detach the loop and this is not a bug? Thank you very much! Dongli Zhang On 03/18/2019 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@syzkaller.appspotmail.com > > 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@googlegroups.com. > > syzbot will keep track of this bug report. See: > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-18 12:27 ` Dongli Zhang @ 2019-03-19 9:41 ` Jan Kara 2019-03-20 4:21 ` Dongli Zhang 0 siblings, 1 reply; 7+ messages in thread From: Jan Kara @ 2019-03-19 9:41 UTC (permalink / raw) To: Dongli Zhang Cc: Jan Kara, syzbot, axboe, linux-block, linux-kernel, syzkaller-bugs, penguin-kernel On Mon 18-03-19 20:27:02, Dongli Zhang wrote: > Hi Jan, > > Indeed there is another issue implicitly reported by below console output from > syzkaller: > > [ 245.524424][ T9455] loop_reread_partitions: partition scan of loop0 () failed > (rc=-13) > [ 245.563340][ T9499] kasan: CONFIG_KASAN_INLINE enabled > [ 245.576412][ T9489] __loop_clr_fd: partition scan of loop0 failed (rc=-13) > [ 245.581275][ T9499] kasan: GPF could be caused by NULL-ptr deref or user > memory access > [ 245.602596][ T9499] general protection fault: 0000 [#1] PREEMPT SMP KASAN > > I think rc=-13 is because of below code at line 168: > > 162 int __blkdev_reread_part(struct block_device *bdev) > 163 { > 164 struct gendisk *disk = bdev->bd_disk; > 165 > 166 if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) > 167 return -EINVAL; > 168 if (!capable(CAP_SYS_ADMIN)) > 169 return -EACCES; > 170 > 171 lockdep_assert_held(&bdev->bd_mutex); > 172 > 173 return rescan_partitions(disk, bdev); > 174 } > 175 EXPORT_SYMBOL(__blkdev_reread_part); > > I can reproduce this by 'chown username /dev/loop0' on my test machine. > > Taking 'losetup -d /dev/loop0' as sample, as /dev/loop0 belongs to my username, > I am able to detach the loop without 'su'. > > However, because of above line 168, the partition scan would fail. > > Should we always assume the user should have admin privilege to detach > the loop and this is not a bug? Firstly, __blkdev_reread_part() is used for all devices so we have to be *very* careful when we relax the permission checks there. Secondly, I'm not convinced it is always safe to allow non-priviledged user to force repartitioning of a device. That exposes the whole partition table parsing code to non-priviledged user and thus increases possible attack surface. But in this specific case we call __blkdev_reread_part() only to tear down partitions. So in this specific case calling it by unpriviledged user is fine plus leaving stale partitions behind is certainly not nice. What we could do is: Export drop_partitions() functionality as a function like __blkdev_drop_part() that would call drop_partitions(bdev->bd_disk, bdev). It would expect (and assert) that &bdev->bd_mutex is already locked. It should probably also replicate the check: if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) return -EINVAL; from __blkdev_reread_part(). And we can call this from __loop_clr_fd(). Then we can just unexport __blkdev_reread_part() (since only loop.c is using it) and fold it inside blkdev_reread_part(). What do you think? Honza > On 03/18/2019 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@syzkaller.appspotmail.com > > > > 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@googlegroups.com. > > > > syzbot will keep track of this bug report. See: > > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-19 9:41 ` Jan Kara @ 2019-03-20 4:21 ` Dongli Zhang 2019-03-20 13:43 ` Jan Kara 0 siblings, 1 reply; 7+ messages in thread From: Dongli Zhang @ 2019-03-20 4:21 UTC (permalink / raw) To: Jan Kara Cc: syzbot, axboe, linux-block, linux-kernel, syzkaller-bugs, penguin-kernel On 3/19/19 5:41 PM, Jan Kara wrote: > On Mon 18-03-19 20:27:02, Dongli Zhang wrote: >> Hi Jan, >> >> Indeed there is another issue implicitly reported by below console output from >> syzkaller: >> >> [ 245.524424][ T9455] loop_reread_partitions: partition scan of loop0 () failed >> (rc=-13) >> [ 245.563340][ T9499] kasan: CONFIG_KASAN_INLINE enabled >> [ 245.576412][ T9489] __loop_clr_fd: partition scan of loop0 failed (rc=-13) >> [ 245.581275][ T9499] kasan: GPF could be caused by NULL-ptr deref or user >> memory access >> [ 245.602596][ T9499] general protection fault: 0000 [#1] PREEMPT SMP KASAN >> >> I think rc=-13 is because of below code at line 168: >> >> 162 int __blkdev_reread_part(struct block_device *bdev) >> 163 { >> 164 struct gendisk *disk = bdev->bd_disk; >> 165 >> 166 if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) >> 167 return -EINVAL; >> 168 if (!capable(CAP_SYS_ADMIN)) >> 169 return -EACCES; >> 170 >> 171 lockdep_assert_held(&bdev->bd_mutex); >> 172 >> 173 return rescan_partitions(disk, bdev); >> 174 } >> 175 EXPORT_SYMBOL(__blkdev_reread_part); >> >> I can reproduce this by 'chown username /dev/loop0' on my test machine. >> >> Taking 'losetup -d /dev/loop0' as sample, as /dev/loop0 belongs to my username, >> I am able to detach the loop without 'su'. >> >> However, because of above line 168, the partition scan would fail. >> >> Should we always assume the user should have admin privilege to detach >> the loop and this is not a bug? > > Firstly, __blkdev_reread_part() is used for all devices so we have to be > *very* careful when we relax the permission checks there. > > Secondly, I'm not convinced it is always safe to allow non-priviledged user > to force repartitioning of a device. That exposes the whole partition table > parsing code to non-priviledged user and thus increases possible attack > surface. > > But in this specific case we call __blkdev_reread_part() only to tear down > partitions. So in this specific case calling it by unpriviledged user is > fine plus leaving stale partitions behind is certainly not nice. What we > could do is: > > Export drop_partitions() functionality as a function like > __blkdev_drop_part() that would call drop_partitions(bdev->bd_disk, bdev). > It would expect (and assert) that &bdev->bd_mutex is already locked. It > should probably also replicate the check: > > if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) > return -EINVAL; > > from __blkdev_reread_part(). And we can call this from __loop_clr_fd(). > > Then we can just unexport __blkdev_reread_part() (since only loop.c is > using it) and fold it inside blkdev_reread_part(). > > What do you think? > > Honza > The idea is to duplicate a new caller of drop_partitions() which is specifically used by loop. I think it works. It is safe as well as the functionality is limited within loop. However, perhaps we should not regard this as a bug? Below is the sample to reproduce this issue: $ dd if=/dev/zero of=tmp.raw bs=1M count=100 oflag=direct $ parted tmp.raw --script mklabel msdos mkpart primary 0% 50% mkpart primary 50% 100% $ sudo chown zhang /dev/loop0 $ losetup -P /dev/loop0 tmp.raw $ ls -l /dev | grep loop0 brw-rw---- 1 zhang disk 7, 0 Mar 20 11:42 loop0 --> user (zhang) brw-rw---- 1 root disk 259, 0 Mar 20 11:42 loop0p1 --> root brw-rw---- 1 root disk 259, 1 Mar 20 11:42 loop0p2 --> root $ losetup -d /dev/loop From above case, /dev/loop0 is owned by user (zhang), while the partitions are owned by root. Should the detach of loop owned by user also unmaps all related partitions owned by root? Perhaps we should assume the '-P' option is always used by root? This is similar to the fact that the administrator should always use '-P' in order to enforce partscan when the loop is detached. Otherwise, the partitions belong to the loop would remain after 'losetup -d'. Another example is from kpartx as below. If the partitions are mapped via 'kpartx -av', the user should always run 'kpartx -d' to remove all partition mappings. Otherwise, there would be dm-0 and dm-1 left. $ sudo kpartx -av tmp.raw add map loop0p1 (253:0): 0 102400 linear 7:0 1 add map loop0p2 (253:1): 0 102399 linear 7:0 102401 $ ls -l /dev/mapper/ total 0 crw------- 1 root root 10, 236 3月 20 11:16 control lrwxrwxrwx 1 root root 7 3月 20 11:21 loop0p1 -> ../dm-0 lrwxrwxrwx 1 root root 7 3月 20 11:21 loop0p2 -> ../dm-1 $ sudo losetup -d /dev/loop0 $ ls -l /dev/mapper/ total 0 crw------- 1 root root 10, 236 3月 20 11:16 control lrwxrwxrwx 1 root root 7 3月 20 11:21 loop0p1 -> ../dm-0 lrwxrwxrwx 1 root root 7 3月 20 11:21 loop0p2 -> ../dm-1 Dongli Zhang ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: general protection fault in loop_validate_file (2) 2019-03-20 4:21 ` Dongli Zhang @ 2019-03-20 13:43 ` Jan Kara 0 siblings, 0 replies; 7+ messages in thread From: Jan Kara @ 2019-03-20 13:43 UTC (permalink / raw) To: Dongli Zhang Cc: Jan Kara, syzbot, axboe, linux-block, linux-kernel, syzkaller-bugs, penguin-kernel On Wed 20-03-19 12:21:29, Dongli Zhang wrote: > On 3/19/19 5:41 PM, Jan Kara wrote: > > On Mon 18-03-19 20:27:02, Dongli Zhang wrote: > >> Hi Jan, > >> > >> Indeed there is another issue implicitly reported by below console output from > >> syzkaller: > >> > >> [ 245.524424][ T9455] loop_reread_partitions: partition scan of loop0 () failed > >> (rc=-13) > >> [ 245.563340][ T9499] kasan: CONFIG_KASAN_INLINE enabled > >> [ 245.576412][ T9489] __loop_clr_fd: partition scan of loop0 failed (rc=-13) > >> [ 245.581275][ T9499] kasan: GPF could be caused by NULL-ptr deref or user > >> memory access > >> [ 245.602596][ T9499] general protection fault: 0000 [#1] PREEMPT SMP KASAN > >> > >> I think rc=-13 is because of below code at line 168: > >> > >> 162 int __blkdev_reread_part(struct block_device *bdev) > >> 163 { > >> 164 struct gendisk *disk = bdev->bd_disk; > >> 165 > >> 166 if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) > >> 167 return -EINVAL; > >> 168 if (!capable(CAP_SYS_ADMIN)) > >> 169 return -EACCES; > >> 170 > >> 171 lockdep_assert_held(&bdev->bd_mutex); > >> 172 > >> 173 return rescan_partitions(disk, bdev); > >> 174 } > >> 175 EXPORT_SYMBOL(__blkdev_reread_part); > >> > >> I can reproduce this by 'chown username /dev/loop0' on my test machine. > >> > >> Taking 'losetup -d /dev/loop0' as sample, as /dev/loop0 belongs to my username, > >> I am able to detach the loop without 'su'. > >> > >> However, because of above line 168, the partition scan would fail. > >> > >> Should we always assume the user should have admin privilege to detach > >> the loop and this is not a bug? > > > > Firstly, __blkdev_reread_part() is used for all devices so we have to be > > *very* careful when we relax the permission checks there. > > > > Secondly, I'm not convinced it is always safe to allow non-priviledged user > > to force repartitioning of a device. That exposes the whole partition table > > parsing code to non-priviledged user and thus increases possible attack > > surface. > > > > But in this specific case we call __blkdev_reread_part() only to tear down > > partitions. So in this specific case calling it by unpriviledged user is > > fine plus leaving stale partitions behind is certainly not nice. What we > > could do is: > > > > Export drop_partitions() functionality as a function like > > __blkdev_drop_part() that would call drop_partitions(bdev->bd_disk, bdev). > > It would expect (and assert) that &bdev->bd_mutex is already locked. It > > should probably also replicate the check: > > > > if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) > > return -EINVAL; > > > > from __blkdev_reread_part(). And we can call this from __loop_clr_fd(). > > > > Then we can just unexport __blkdev_reread_part() (since only loop.c is > > using it) and fold it inside blkdev_reread_part(). > > > > What do you think? > > > > Honza > > > > The idea is to duplicate a new caller of drop_partitions() which is > specifically used by loop. I think it works. It is safe as well as the > functionality is limited within loop. > > However, perhaps we should not regard this as a bug? Below is the sample > to reproduce this issue: > > $ dd if=/dev/zero of=tmp.raw bs=1M count=100 oflag=direct > > $ parted tmp.raw --script mklabel msdos mkpart primary 0% 50% mkpart primary 50% > 100% > > $ sudo chown zhang /dev/loop0 > > $ losetup -P /dev/loop0 tmp.raw > > $ ls -l /dev | grep loop0 > brw-rw---- 1 zhang disk 7, 0 Mar 20 11:42 loop0 --> user (zhang) > brw-rw---- 1 root disk 259, 0 Mar 20 11:42 loop0p1 --> root > brw-rw---- 1 root disk 259, 1 Mar 20 11:42 loop0p2 --> root > > $ losetup -d /dev/loop > > From above case, /dev/loop0 is owned by user (zhang), while the partitions are > owned by root. > > Should the detach of loop owned by user also unmaps all related partitions owned > by root? > > Perhaps we should assume the '-P' option is always used by root? Yes, that's another option. And since I'm not aware of any user reports of the problem you've found I'd just say that non-root users don't use loop devices with partitions. > This is similar to the fact that the administrator should always use '-P' > in order to enforce partscan when the loop is detached. Otherwise, the > partitions belong to the loop would remain after 'losetup -d'. Yeah. That seems like another slight catch for the users. Honestly, I don't feel either of these issues is serious enough that I'd go and fix them. But if someone wanted to spend the time with them, I won't object :). Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-20 13:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-18 3:36 general protection fault in loop_validate_file (2) syzbot 2019-03-18 6:51 ` Dongli Zhang 2019-03-18 10:44 ` Jan Kara 2019-03-18 12:27 ` Dongli Zhang 2019-03-19 9:41 ` Jan Kara 2019-03-20 4:21 ` Dongli Zhang 2019-03-20 13:43 ` Jan Kara
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.