Re: general protection fault in lo_ioctl (2)

[Tetsuo Handa]

Theodore Y. Ts'o wrote:
> On Tue, May 08, 2018 at 08:05:12PM +0900, Tetsuo Handa wrote:
> > 
> > So, it is time to think how to solve this race condition, as well as how to 
> > solve
> > lockdep's deadlock warning (and I guess that syzbot is actually hitting 
> > deadlocks).
> > An approach which serializes loop operations using global lock was proposed 
> > at
> > https://groups.google.com/d/msg/syzkaller-bugs/2Rw8-OM6IbM/PzdobV8kAgAJ .
> > Please respond...
> 
> I'm looking at your patch which you proposed on this, and the locking
> architecture still looks way too complex.  Things like
> loop_mutex_owner, and all of the infrastructure around
> lo->ioctl_in_progress should be removed, if at all possible.

The patch in the above link no longer uses "lo->ioctl_in_progress".
You looked at previous version rather than current version.

> 
> I believe it should be possible to do things with a single global
> mutex, some code refactoring, and some unlocked versions of some of
> the functions.

The patch in the above link uses single global mutex "loop_mutex".

> 
> Again, this requires root, and it requires someone deliberately trying
> to induce a race.  So "it's time" is not necessarily the priority I
> would set for this item.  But if we are going to fix it, let's fix it
> right, and not make the code more complex and less maintainable, all
> in the name of trying to make a rare, not-likely-to-happen-in-real-life
> syzbot reported problem to go away.

While NULL pointer dereference would be rare, deadlocks might not be rare
enough to postpone the patch. Deadlocks can cause pile of false-positive
hung task reports and can prevent syzbot from finding other bugs. That's
why I say "it is time to think".

Re: general protection fault in lo_ioctl (2)

[Theodore Y. Ts'o]

On Tue, May 08, 2018 at 08:05:12PM +0900, Tetsuo Handa wrote:
> 
> So, it is time to think how to solve this race condition, as well as how to 
> solve
> lockdep's deadlock warning (and I guess that syzbot is actually hitting 
> deadlocks).
> An approach which serializes loop operations using global lock was proposed at
> https://groups.google.com/d/msg/syzkaller-bugs/2Rw8-OM6IbM/PzdobV8kAgAJ .
> Please respond...

I'm looking at your patch which you proposed on this, and the locking
architecture still looks way too complex.  Things like
loop_mutex_owner, and all of the infrastructure around
lo->ioctl_in_progress should be removed, if at all possible.

I believe it should be possible to do things with a single global
mutex, some code refactoring, and some unlocked versions of some of
the functions.

Again, this requires root, and it requires someone deliberately trying
to induce a race.  So "it's time" is not necessarily the priority I
would set for this item.  But if we are going to fix it, let's fix it
right, and not make the code more complex and less maintainable, all
in the name of trying to make a rare, not-likely-to-happen-in-real-life
syzbot reported problem to go away.

Cheers,

- Ted

Re: general protection fault in lo_ioctl (2)

[Tetsuo Handa]

On 2018/05/08 5:56, Tetsuo Handa wrote:
> On 2018/05/02 20:23, Dmitry Vyukov wrote:
>> #syz dup: INFO: rcu detected stall in blkdev_ioctl
> 
> The cause of stall turned out to be ioctl(loop_fd, LOOP_CHANGE_FD, loop_fd).
> 
> But we haven't explained the cause of NULL pointer dereference which can
> occur when raced with ioctl(LOOP_CLR_FD). Therefore,
> 
> #syz undup
> 

Using sleep injection patch and reproducer shown below, I can reproduce
the crashes. It is a race between ioctl(loop_fd, LOOP_CLR_FD, 0) versus
ioctl(other_loop_fd, LOOP_SET_FD, loop_fd).

Unless we hold corresponding lo->lo_ctl_mutex (or keep corresponding
lo->lo_refcnt elevated) when traversing other loop devices,
"/* Avoid recursion */" loop from loop_set_fd()/loop_change_fd() will
suffer from races by loop_clr_fd().

So, it is time to think how to solve this race condition, as well as how to 
solve
lockdep's deadlock warning (and I guess that syzbot is actually hitting 
deadlocks).
An approach which serializes loop operations using global lock was proposed at
https://groups.google.com/d/msg/syzkaller-bugs/2Rw8-OM6IbM/PzdobV8kAgAJ .
Please respond...


--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -909,6 +909,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
error = -EINVAL;
goto out_putf;
}
+   pr_err("Start sleeping\n");
+   schedule_timeout_killable(3 * HZ);
+   pr_err("End sleeping\n");
f = l->lo_backing_file;
}
 



#include 
#include 
#include 
#include 
#include 
#include 
#include 

int main(int argc, char *argv[])
{
int fd0 = open("/dev/loop0", O_RDONLY);
int fd1 = open("/dev/loop1", O_RDONLY);
int fd2 = open("/tmp/file", O_RDWR | O_CREAT | O_TRUNC, 0600);
ioctl(fd1, LOOP_SET_FD, fd2);
if (fork() == 0) {
sleep(1);
ioctl(fd1, LOOP_CLR_FD, 0);
_exit(0);
}
ioctl(fd0, LOOP_SET_FD, fd1);
return 0;
}



[   14.119073] loop: module loaded
[   17.363610] Start sleeping
[   20.383442] End sleeping
[   20.386511] BUG: unable to handle kernel NULL pointer dereference at 
0008
[   20.394779] PGD 13377d067 P4D 13377d067 PUD 131509067 PMD 0 
[   20.400847] Oops:  [#1] SMP
[   20.403875] Modules linked in: loop
[   20.406188] CPU: 6 PID: 6470 Comm: a.out Tainted: GT 
4.17.0-rc4+ #540
[   20.411266] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
Desktop Reference Platform, BIOS 6.00 05/19/2017
[   20.418169] RIP: 0010:lo_ioctl+0x7ef/0x840 [loop]
[   20.421272] RSP: 0018:c9bbbd88 EFLAGS: 00010282
[   20.424661] RAX:  RBX:  RCX: 83679478
[   20.429271] RDX: 8801332e9c00 RSI: 0086 RDI: 0286
[   20.434517] RBP: c9bbbdd8 R08: 0638 R09: 
[   20.436879] R10: 0190 R11: 0720072007200720 R12: 8801314ab118
[   20.439076] R13: 880138deae40 R14: 8801311f7780 R15: 8801314ab000
[   20.441144] FS:  7f0b57743740() GS:88013a78() 
knlGS:
[   20.443588] CS:  0010 DS:  ES:  CR0: 80050033
[   20.445284] CR2: 0008 CR3: 000138efb002 CR4: 000606e0
[   20.447381] Call Trace:
[   20.448149]  blkdev_ioctl+0x88d/0x950
[   20.449237]  block_ioctl+0x38/0x40
[   20.450269]  do_vfs_ioctl+0xaa/0x650
[   20.451479]  ? handle_mm_fault+0x108/0x250
[   20.452704]  ksys_ioctl+0x70/0x80
[   20.453737]  __x64_sys_ioctl+0x15/0x20
[   20.454887]  do_syscall_64+0x5d/0x100
[   20.456014]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   20.457519] RIP: 0033:0x7f0b57267107
[   20.458644] RSP: 002b:7fff8a0fd698 EFLAGS: 0246 ORIG_RAX: 
0010
[   20.460853] RAX: ffda RBX: 0004 RCX: 7f0b57267107
[   20.462952] RDX: 0004 RSI: 4c00 RDI: 0003
[   20.465023] RBP: 0003 R08: 7f0b57743740 R09: 
[   20.467091] R10: 7f0b57743a10 R11: 0246 R12: 004005ef
[   20.469361] R13: 7fff8a0fd790 R14:  R15: 
[   20.471657] Code: a0 48 89 55 d0 e8 e0 5f 1d e1 bf b8 0b 00 00 e8 78 9e 7c 
e2 48 c7 c7 a9 40 00 a0 e8 ca 5f 1d e1 48 8b 55 d0 48 8b 82 f0 00 00 00 <48> 8b 
40 08 48 8b 40 68 48 85 c0 0f 84 15 fd ff ff 0f b7 90 b8 
[   20.477207] RIP: lo_ioctl+0x7ef/0x840 [loop] RSP: c9bbbd88
[   20.479027] CR2: 0008
[   20.480063] ---[ end trace 925bc1b992d96cb3 ]---
[   20.481441] Kernel panic - not syncing: Fatal exception
[  

Re: general protection fault in lo_ioctl (2)

[Tetsuo Handa]

On 2018/05/02 20:23, Dmitry Vyukov wrote:
> #syz dup: INFO: rcu detected stall in blkdev_ioctl

The cause of stall turned out to be ioctl(loop_fd, LOOP_CHANGE_FD, loop_fd).

But we haven't explained the cause of NULL pointer dereference which can
occur when raced with ioctl(LOOP_CLR_FD). Therefore,

#syz undup

Re: general protection fault in lo_ioctl (2)

[Dmitry Vyukov]

On Wed, May 2, 2018 at 9:33 AM, syzbot
 wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:fff75eb2a08c Merge tag 'errseq-v4.17' of
> git://git.kernel.o...
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?id=5301511529693184
> kernel config:
> https://syzkaller.appspot.com/x/.config?id=6493557782959164711
> dashboard link: https://syzkaller.appspot.com/bug?extid=bf89c128e05dd6c62523
> compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
> userspace arch: i386
> syzkaller
> repro:https://syzkaller.appspot.com/x/repro.syz?id=5000693362458624

#syz dup: INFO: rcu detected stall in blkdev_ioctl

> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+bf89c128e05dd6c62...@syzkaller.appspotmail.com
>
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 10637 Comm: syz-executor1 Not tainted 4.17.0-rc3+ #52
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:loop_set_fd drivers/block/loop.c:901 [inline]
> RIP: 0010:lo_ioctl+0x1ae8/0x2130 drivers/block/loop.c:1397
> RSP: 0018:8801bd6dfc08 EFLAGS: 00010202
> RAX: 0037 RBX: 8801d296db00 RCX: dc00
> RDX:  RSI: 84b2cd57 RDI: 01b8
> RBP: 8801bd6dfc80 R08: 8801c46943c0 R09: ed003b5c46c2
> R10: 0003 R11: 0001 R12: 
> R13:  R14: 8801b58561a0 R15: 8801b58560c0
> FS:  () GS:8801dae0(0063) knlGS:f7f2eb40
> CS:  0010 DS: 002b ES: 002b CR0: 80050033
> CR2: 08138024 CR3: 0001d03df000 CR4: 001406f0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> Call Trace:
>  lo_compat_ioctl+0xc1/0x170 drivers/block/loop.c:1602
>  compat_blkdev_ioctl+0x3c2/0x1b20 block/compat_ioctl.c:406
>  __do_compat_sys_ioctl fs/compat_ioctl.c:1461 [inline]
>  __se_compat_sys_ioctl fs/compat_ioctl.c:1407 [inline]
>  __ia32_compat_sys_ioctl+0x221/0x640 fs/compat_ioctl.c:1407
>  do_syscall_32_irqs_on arch/x86/entry/common.c:323 [inline]
>  do_fast_syscall_32+0x345/0xf9b arch/x86/entry/common.c:394
>  entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
> RIP: 0023:0xf7f32cb9
> RSP: 002b:f7f2e0ac EFLAGS: 0282 ORIG_RAX: 0036
> RAX: ffda RBX: 0004 RCX: 4c00
> RDX: 0003 RSI:  RDI: 
> RBP:  R08:  R09: 
> R10:  R11: 0292 R12: 
> R13:  R14:  R15: 
> Code: e8 03 80 3c 08 00 0f 85 2c 02 00 00 4d 8b ad f0 00 00 00 48 b9 00 00
> 00 00 00 fc ff df 49 8d bd b8 01 00 00 48 89 f8 48 c1 e8 03 <80> 3c 08 00 0f
> 85 f9 01 00 00 4d 8b ad b8 01 00 00 48 b9 00 00
> RIP: loop_set_fd drivers/block/loop.c:901 [inline] RSP: 8801bd6dfc08
> RIP: lo_ioctl+0x1ae8/0x2130 drivers/block/loop.c:1397 RSP: 8801bd6dfc08
> ---[ end trace a15583efe355602c ]---
>
>
> ---
> 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 syzkal...@googlegroups.com.
>
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is
> merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> If you want to test a patch for this bug, please reply with:
> #syz test: git://repo/address.git branch
> and provide the patch inline or as an attachment.
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug
> report.
> Note: all commands must start from beginning of the line in the email body.
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/cbaaf6056b341859%40google.com.
> For more options, visit https://groups.google.com/d/optout.

general protection fault in lo_ioctl (2)

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:fff75eb2a08c Merge tag 'errseq-v4.17' of  
git://git.kernel.o...

git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?id=5301511529693184
kernel config:   
https://syzkaller.appspot.com/x/.config?id=6493557782959164711

dashboard link: https://syzkaller.appspot.com/bug?extid=bf89c128e05dd6c62523
compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
userspace arch: i386
syzkaller  
repro:https://syzkaller.appspot.com/x/repro.syz?id=5000693362458624


IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bf89c128e05dd6c62...@syzkaller.appspotmail.com

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault:  [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 10637 Comm: syz-executor1 Not tainted 4.17.0-rc3+ #52
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011

RIP: 0010:loop_set_fd drivers/block/loop.c:901 [inline]
RIP: 0010:lo_ioctl+0x1ae8/0x2130 drivers/block/loop.c:1397
RSP: 0018:8801bd6dfc08 EFLAGS: 00010202
RAX: 0037 RBX: 8801d296db00 RCX: dc00
RDX:  RSI: 84b2cd57 RDI: 01b8
RBP: 8801bd6dfc80 R08: 8801c46943c0 R09: ed003b5c46c2
R10: 0003 R11: 0001 R12: 
R13:  R14: 8801b58561a0 R15: 8801b58560c0
FS:  () GS:8801dae0(0063) knlGS:f7f2eb40
CS:  0010 DS: 002b ES: 002b CR0: 80050033
CR2: 08138024 CR3: 0001d03df000 CR4: 001406f0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
Call Trace:
 lo_compat_ioctl+0xc1/0x170 drivers/block/loop.c:1602
 compat_blkdev_ioctl+0x3c2/0x1b20 block/compat_ioctl.c:406
 __do_compat_sys_ioctl fs/compat_ioctl.c:1461 [inline]
 __se_compat_sys_ioctl fs/compat_ioctl.c:1407 [inline]
 __ia32_compat_sys_ioctl+0x221/0x640 fs/compat_ioctl.c:1407
 do_syscall_32_irqs_on arch/x86/entry/common.c:323 [inline]
 do_fast_syscall_32+0x345/0xf9b arch/x86/entry/common.c:394
 entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7f32cb9
RSP: 002b:f7f2e0ac EFLAGS: 0282 ORIG_RAX: 0036
RAX: ffda RBX: 0004 RCX: 4c00
RDX: 0003 RSI:  RDI: 
RBP:  R08:  R09: 
R10:  R11: 0292 R12: 
R13:  R14:  R15: 
Code: e8 03 80 3c 08 00 0f 85 2c 02 00 00 4d 8b ad f0 00 00 00 48 b9 00 00  
00 00 00 fc ff df 49 8d bd b8 01 00 00 48 89 f8 48 c1 e8 03 <80> 3c 08 00  
0f 85 f9 01 00 00 4d 8b ad b8 01 00 00 48 b9 00 00

RIP: loop_set_fd drivers/block/loop.c:901 [inline] RSP: 8801bd6dfc08
RIP: lo_ioctl+0x1ae8/0x2130 drivers/block/loop.c:1397 RSP: 8801bd6dfc08
---[ end trace a15583efe355602c ]---


---
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 syzkal...@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged

into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.

Note: all commands must start from beginning of the line in the email body.