BUG: unable to handle kernel paging request in do_mount

BUG: unable to handle kernel paging request in do_mount

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    a0cb0cabe4bb Add linux-next specific files for 20180920
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=164d6511400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=786006c5dafbadf6
dashboard link: https://syzkaller.appspot.com/bug?extid=73c7fe4f77776505299b
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13650fae400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=118cbf21400000

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

RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440329
RDX: 0000000020000380 RSI: 00000000200002c0 RDI: 0000000000000000
RBP: 00000000006cb018 R08: 0000000000000000 R09: 0000000000000034
R10: 00000000001800a0 R11: 0000000000000246 R12: 0000000000000003
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
BUG: unable to handle kernel paging request at fffffffffffffff4
PGD 966d067 P4D 966d067 PUD 966f067 PMD 0
Oops: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 5551 Comm: syz-executor016 Not tainted  
4.19.0-rc4-next-20180920+ #76
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:parse_monolithic_mount_data fs/namespace.c:2354 [inline]
RIP: 0010:do_remount fs/namespace.c:2387 [inline]
RIP: 0010:do_mount+0xb3b/0x1db0 fs/namespace.c:2950
Code: 06 00 48 89 c2 48 89 c3 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80  
3c 02 00 0f 85 24 11 00 00 48 b8 00 00 00 00 00 fc ff df <4c> 8b 33 49 8d  
7e 18 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 f8 10
RSP: 0018:ffff8801bb99fc28 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: fffffffffffffff4 RCX: 0000000000000000
RDX: 1ffffffffffffffe RSI: ffffffff81ddffac RDI: 0000000000000282
RBP: ffff8801bb99fdb0 R08: ffff8801d8b0e600 R09: ffffed003b5a5b57
R10: ffffed003b5a5b57 R11: ffff8801dad2dabb R12: ffff8801d3666830
R13: ffff8801c942cb80 R14: ffff8801c942cb80 R15: 0000000000000000
FS:  00000000010de880(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffffffffffff4 CR3: 00000001d84b5000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  ksys_mount+0x12d/0x140 fs/namespace.c:3175
  __do_sys_mount fs/namespace.c:3189 [inline]
  __se_sys_mount fs/namespace.c:3186 [inline]
  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3186
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440329
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 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 5b 14 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc6e4c9ec8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440329
RDX: 0000000020000380 RSI: 00000000200002c0 RDI: 0000000000000000
RBP: 00000000006cb018 R08: 0000000000000000 R09: 0000000000000034
R10: 00000000001800a0 R11: 0000000000000246 R12: 0000000000000003
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
CR2: fffffffffffffff4
---[ end trace 8aa3e592ff591401 ]---
RIP: 0010:parse_monolithic_mount_data fs/namespace.c:2354 [inline]
RIP: 0010:do_remount fs/namespace.c:2387 [inline]
RIP: 0010:do_mount+0xb3b/0x1db0 fs/namespace.c:2950
Code: 06 00 48 89 c2 48 89 c3 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80  
3c 02 00 0f 85 24 11 00 00 48 b8 00 00 00 00 00 fc ff df <4c> 8b 33 49 8d  
7e 18 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 f8 10
RSP: 0018:ffff8801bb99fc28 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: fffffffffffffff4 RCX: 0000000000000000
RDX: 1ffffffffffffffe RSI: ffffffff81ddffac RDI: 0000000000000282
RBP: ffff8801bb99fdb0 R08: ffff8801d8b0e600 R09: ffffed003b5a5b57
R10: ffffed003b5a5b57 R11: ffff8801dad2dabb R12: ffff8801d3666830
R13: ffff8801c942cb80 R14: ffff8801c942cb80 R15: 0000000000000000
FS:  00000000010de880(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffffffffffff4 CR3: 00000001d84b5000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
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.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: BUG: unable to handle kernel paging request in do_mount

[Sabin Rapan]

This is fixed by patch vfs: namespace: error pointer dereference in do_remount()

#syz fix: vfs: namespace: error pointer dereference in do_remount()

On 21.09.2018 09:44, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    a0cb0cabe4bb Add linux-next specific files for 20180920
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=164d6511400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=786006c5dafbadf6
> dashboard link: https://syzkaller.appspot.com/bug?extid=73c7fe4f77776505299b
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13650fae400000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=118cbf21400000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com
> 
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440329
> RDX: 0000000020000380 RSI: 00000000200002c0 RDI: 0000000000000000
> RBP: 00000000006cb018 R08: 0000000000000000 R09: 0000000000000034
> R10: 00000000001800a0 R11: 0000000000000246 R12: 0000000000000003
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> BUG: unable to handle kernel paging request at fffffffffffffff4
> PGD 966d067 P4D 966d067 PUD 966f067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 5551 Comm: syz-executor016 Not tainted 4.19.0-rc4-next-20180920+ #76
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:parse_monolithic_mount_data fs/namespace.c:2354 [inline]
> RIP: 0010:do_remount fs/namespace.c:2387 [inline]
> RIP: 0010:do_mount+0xb3b/0x1db0 fs/namespace.c:2950
> Code: 06 00 48 89 c2 48 89 c3 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 24 11 00 00 48 b8 00 00 00 00 00 fc ff df <4c> 8b 33 49 8d 7e 18 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 f8 10
> RSP: 0018:ffff8801bb99fc28 EFLAGS: 00010246
> RAX: dffffc0000000000 RBX: fffffffffffffff4 RCX: 0000000000000000
> RDX: 1ffffffffffffffe RSI: ffffffff81ddffac RDI: 0000000000000282
> RBP: ffff8801bb99fdb0 R08: ffff8801d8b0e600 R09: ffffed003b5a5b57
> R10: ffffed003b5a5b57 R11: ffff8801dad2dabb R12: ffff8801d3666830
> R13: ffff8801c942cb80 R14: ffff8801c942cb80 R15: 0000000000000000
> FS:  00000000010de880(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: fffffffffffffff4 CR3: 00000001d84b5000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  ksys_mount+0x12d/0x140 fs/namespace.c:3175
>  __do_sys_mount fs/namespace.c:3189 [inline]
>  __se_sys_mount fs/namespace.c:3186 [inline]
>  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3186
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x440329
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 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 5b 14 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffc6e4c9ec8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000440329
> RDX: 0000000020000380 RSI: 00000000200002c0 RDI: 0000000000000000
> RBP: 00000000006cb018 R08: 0000000000000000 R09: 0000000000000034
> R10: 00000000001800a0 R11: 0000000000000246 R12: 0000000000000003
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> Modules linked in:
> CR2: fffffffffffffff4
> ---[ end trace 8aa3e592ff591401 ]---
> RIP: 0010:parse_monolithic_mount_data fs/namespace.c:2354 [inline]
> RIP: 0010:do_remount fs/namespace.c:2387 [inline]
> RIP: 0010:do_mount+0xb3b/0x1db0 fs/namespace.c:2950
> Code: 06 00 48 89 c2 48 89 c3 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 24 11 00 00 48 b8 00 00 00 00 00 fc ff df <4c> 8b 33 49 8d 7e 18 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 f8 10
> RSP: 0018:ffff8801bb99fc28 EFLAGS: 00010246
> RAX: dffffc0000000000 RBX: fffffffffffffff4 RCX: 0000000000000000
> RDX: 1ffffffffffffffe RSI: ffffffff81ddffac RDI: 0000000000000282
> RBP: ffff8801bb99fdb0 R08: ffff8801d8b0e600 R09: ffffed003b5a5b57
> R10: ffffed003b5a5b57 R11: ffff8801dad2dabb R12: ffff8801d3666830
> R13: ffff8801c942cb80 R14: ffff8801c942cb80 R15: 0000000000000000
> FS:  00000000010de880(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: fffffffffffffff4 CR3: 00000001d84b5000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> 
> ---
> 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.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
> 

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[syzbot]

This bug is marked as fixed by commit:
vfs: namespace: error pointer dereference in do_remount()
But I can't find it in any tested tree for more than 90 days.
Is it a correct commit? Please update it by replying:
#syz fix: exact-commit-title
Until then the bug is still considered open and
new crashes with the same signature are ignored.

Re: BUG: unable to handle kernel paging request in do_mount

[Al Viro]

On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> This bug is marked as fixed by commit:
> vfs: namespace: error pointer dereference in do_remount()
> But I can't find it in any tested tree for more than 90 days.
> Is it a correct commit? Please update it by replying:
> #syz fix: exact-commit-title
> Until then the bug is still considered open and
> new crashes with the same signature are ignored.

Could somebody explain how the following situation is supposed to
be handled:

1) branch B1 with commits  C1, C2, C3, C4 is pushed out
2) C2 turns out to have a bug, which gets caught and fixed
3) fix is folded in and branch B2 with C1, C2', C3', C4' is
pushed out.  The bug is not in it anymore.
4) B1 is left mouldering (or is entirely removed); B2 is
eventually merged into other trees.

This is normal and it appears to be problematic for syzbot.
How to deal with that?  One thing I will *NOT* do in such
situations is giving up on folding the fixes in.  Bisection
hazards alone make that a bad idea.

Re: BUG: unable to handle kernel paging request in do_mount

[Dmitry Vyukov]

On Fri, May 17, 2019 at 3:48 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> > This bug is marked as fixed by commit:
> > vfs: namespace: error pointer dereference in do_remount()
> > But I can't find it in any tested tree for more than 90 days.
> > Is it a correct commit? Please update it by replying:
> > #syz fix: exact-commit-title
> > Until then the bug is still considered open and
> > new crashes with the same signature are ignored.
>
> Could somebody explain how the following situation is supposed to
> be handled:
>
> 1) branch B1 with commits  C1, C2, C3, C4 is pushed out
> 2) C2 turns out to have a bug, which gets caught and fixed
> 3) fix is folded in and branch B2 with C1, C2', C3', C4' is
> pushed out.  The bug is not in it anymore.
> 4) B1 is left mouldering (or is entirely removed); B2 is
> eventually merged into other trees.
>
> This is normal and it appears to be problematic for syzbot.
> How to deal with that?  One thing I will *NOT* do in such
> situations is giving up on folding the fixes in.  Bisection
> hazards alone make that a bad idea.

linux-next creates a bit of a havoc.

The ideal way of handling this is including Tested-by: tag into C2'.
Reported-by: would work too, but people suggested that Reported-by: is
confusing in this situation because it suggests that the commit fixes
a bug in some previous commit. Technically, syzbot now accepts any
tag, so With-inputs-from:
syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com would work too.

At this point we obvious can't fix up C2'. For such cases syzbot
accepts #syz fix command to associate bugs with fixes. So replying
with "#syz fix: C2'-commit-title" should do.

Re: BUG: unable to handle kernel paging request in do_mount

[Dmitry Vyukov]

On Fri, May 17, 2019 at 4:08 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Fri, May 17, 2019 at 3:48 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> > > This bug is marked as fixed by commit:
> > > vfs: namespace: error pointer dereference in do_remount()
> > > But I can't find it in any tested tree for more than 90 days.
> > > Is it a correct commit? Please update it by replying:
> > > #syz fix: exact-commit-title
> > > Until then the bug is still considered open and
> > > new crashes with the same signature are ignored.
> >
> > Could somebody explain how the following situation is supposed to
> > be handled:
> >
> > 1) branch B1 with commits  C1, C2, C3, C4 is pushed out
> > 2) C2 turns out to have a bug, which gets caught and fixed
> > 3) fix is folded in and branch B2 with C1, C2', C3', C4' is
> > pushed out.  The bug is not in it anymore.
> > 4) B1 is left mouldering (or is entirely removed); B2 is
> > eventually merged into other trees.
> >
> > This is normal and it appears to be problematic for syzbot.
> > How to deal with that?  One thing I will *NOT* do in such
> > situations is giving up on folding the fixes in.  Bisection
> > hazards alone make that a bad idea.
>
> linux-next creates a bit of a havoc.
>
> The ideal way of handling this is including Tested-by: tag into C2'.
> Reported-by: would work too, but people suggested that Reported-by: is
> confusing in this situation because it suggests that the commit fixes
> a bug in some previous commit. Technically, syzbot now accepts any
> tag, so With-inputs-from:
> syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com would work too.
>
> At this point we obvious can't fix up C2'. For such cases syzbot
> accepts #syz fix command to associate bugs with fixes. So replying
> with "#syz fix: C2'-commit-title" should do.

What is that C2'?

Re: BUG: unable to handle kernel paging request in do_mount

[Al Viro]

On Sat, May 18, 2019 at 05:00:39PM +0200, Dmitry Vyukov wrote:
> On Fri, May 17, 2019 at 4:08 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Fri, May 17, 2019 at 3:48 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > >
> > > On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> > > > This bug is marked as fixed by commit:
> > > > vfs: namespace: error pointer dereference in do_remount()
> > > > But I can't find it in any tested tree for more than 90 days.
> > > > Is it a correct commit? Please update it by replying:
> > > > #syz fix: exact-commit-title
> > > > Until then the bug is still considered open and
> > > > new crashes with the same signature are ignored.
> > >
> > > Could somebody explain how the following situation is supposed to
> > > be handled:
> > >
> > > 1) branch B1 with commits  C1, C2, C3, C4 is pushed out
> > > 2) C2 turns out to have a bug, which gets caught and fixed
> > > 3) fix is folded in and branch B2 with C1, C2', C3', C4' is
> > > pushed out.  The bug is not in it anymore.
> > > 4) B1 is left mouldering (or is entirely removed); B2 is
> > > eventually merged into other trees.
> > >
> > > This is normal and it appears to be problematic for syzbot.
> > > How to deal with that?  One thing I will *NOT* do in such
> > > situations is giving up on folding the fixes in.  Bisection
> > > hazards alone make that a bad idea.
> >
> > linux-next creates a bit of a havoc.
> >
> > The ideal way of handling this is including Tested-by: tag into C2'.
> > Reported-by: would work too, but people suggested that Reported-by: is
> > confusing in this situation because it suggests that the commit fixes
> > a bug in some previous commit. Technically, syzbot now accepts any
> > tag, so With-inputs-from:
> > syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com would work too.
> >
> > At this point we obvious can't fix up C2'. For such cases syzbot
> > accepts #syz fix command to associate bugs with fixes. So replying
> > with "#syz fix: C2'-commit-title" should do.
> 
> What is that C2'?

In this case?  Take a look at

commit fd0002870b453c58d0d8c195954f5049bc6675fb
Author: David Howells <dhowells@redhat.com>
Date:   Tue Aug 28 14:45:06 2018 +0100

    vfs: Implement a filesystem superblock creation/configuration context

and compare with

commit f18edd10d3c7d6127b1fa97c8f3299629cf58ed5
Author: David Howells <dhowells@redhat.com>
Date:   Thu Nov 1 23:07:25 2018 +0000

    vfs: Implement a filesystem superblock creation/configuration context

There might have been intermediate forms, but that should illustrate what
happened.  Diff of those two contains (among other things) this:
@@ -985,6 +989,9 @@
 +      fc = vfs_new_fs_context(path->dentry->d_sb->s_type,
 +                              path->dentry, sb_flags, MS_RMT_MASK,
 +                              FS_CONTEXT_FOR_RECONFIGURE);
++      err = PTR_ERR(fc);
++      if (IS_ERR(fc))
++              goto err;
 +
 +      err = parse_monolithic_mount_data(fc, data, data_size);
 +      if (err < 0)

IOW, Dan's fix folded into the offending commit.  And that kind of
pattern is not rare; I would argue that appending Dan's patch at
the end of queue and leaving the crap in between would be a fucking
bad idea - it would've left a massive bisection hazard *and* made
life much more unpleasant when the things got to merging into the
mainline (or reviewing, for that matter).

What would you prefer to happen in such situations?  Commit summaries
modified enough to confuse CI tools into *NOT* noticing that those
are versions of the same patch?  Some kind of metadata telling the
same tools that such-and-such commits got folded in (and they might
have been split in process, with parts folded into different spots
in the series, at that)?

Because "never fold in, never reorder, just accumulate patches in
the end of the series" is not going to fly.  For a lot of reasons.

Re: BUG: unable to handle kernel paging request in do_mount

[Al Viro]

On Sat, May 18, 2019 at 05:21:42PM +0100, Al Viro wrote:
> On Sat, May 18, 2019 at 05:00:39PM +0200, Dmitry Vyukov wrote:
> > On Fri, May 17, 2019 at 4:08 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> > >
> > > On Fri, May 17, 2019 at 3:48 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > >
> > > > On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> > > > > This bug is marked as fixed by commit:
> > > > > vfs: namespace: error pointer dereference in do_remount()
> > > > > But I can't find it in any tested tree for more than 90 days.
> > > > > Is it a correct commit? Please update it by replying:
> > > > > #syz fix: exact-commit-title
> > > > > Until then the bug is still considered open and
> > > > > new crashes with the same signature are ignored.
> > > >
> > > > Could somebody explain how the following situation is supposed to
> > > > be handled:
> > > >
> > > > 1) branch B1 with commits  C1, C2, C3, C4 is pushed out
> > > > 2) C2 turns out to have a bug, which gets caught and fixed
> > > > 3) fix is folded in and branch B2 with C1, C2', C3', C4' is
> > > > pushed out.  The bug is not in it anymore.
> > > > 4) B1 is left mouldering (or is entirely removed); B2 is
> > > > eventually merged into other trees.
> > > >
> > > > This is normal and it appears to be problematic for syzbot.
> > > > How to deal with that?  One thing I will *NOT* do in such
> > > > situations is giving up on folding the fixes in.  Bisection
> > > > hazards alone make that a bad idea.
> > >
> > > linux-next creates a bit of a havoc.
> > >
> > > The ideal way of handling this is including Tested-by: tag into C2'.
> > > Reported-by: would work too, but people suggested that Reported-by: is
> > > confusing in this situation because it suggests that the commit fixes
> > > a bug in some previous commit. Technically, syzbot now accepts any
> > > tag, so With-inputs-from:
> > > syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com would work too.
> > >
> > > At this point we obvious can't fix up C2'. For such cases syzbot
> > > accepts #syz fix command to associate bugs with fixes. So replying
> > > with "#syz fix: C2'-commit-title" should do.
> > 
> > What is that C2'?
> 
> In this case?  Take a look at
> 
> commit fd0002870b453c58d0d8c195954f5049bc6675fb
> Author: David Howells <dhowells@redhat.com>
> Date:   Tue Aug 28 14:45:06 2018 +0100
> 
>     vfs: Implement a filesystem superblock creation/configuration context
> 
> and compare with
> 
> commit f18edd10d3c7d6127b1fa97c8f3299629cf58ed5
> Author: David Howells <dhowells@redhat.com>
> Date:   Thu Nov 1 23:07:25 2018 +0000
> 
>     vfs: Implement a filesystem superblock creation/configuration context
> 
> There might have been intermediate forms, but that should illustrate what
> happened.

While we are at it, even the latter form has *not* made it into the
mainline.  It got split, reordered and massaged quite a bit; the counterpart
of the code in question that went into mainline is

+       fc = fs_context_for_reconfigure(path->dentry, sb_flags, MS_RMT_MASK);
+       if (IS_ERR(fc))
+               return PTR_ERR(fc);

in commit 8d0347f6c3a9d4953ddd636a31c6584da082e084
Author: David Howells <dhowells@redhat.com>
Date:   Sun Nov 4 09:28:36 2018 -0500

    convert do_remount_sb() to fs_context

Re: BUG: unable to handle kernel paging request in do_mount

[Theodore Ts'o]

On Sat, May 18, 2019 at 05:21:42PM +0100, Al Viro wrote:
> IOW, Dan's fix folded into the offending commit.  And that kind of
> pattern is not rare; I would argue that appending Dan's patch at
> the end of queue and leaving the crap in between would be a fucking
> bad idea - it would've left a massive bisection hazard *and* made
> life much more unpleasant when the things got to merging into the
> mainline (or reviewing, for that matter).

When this happens in the ext4 git tree, I usually don't worry about
giving credit to whatever system finds the problem, whether coming
from it's Coverity, or someone running sparse, or syzbot, etc.

There will always be issues where there are no way to clear out the
syzbot report via a commit description --- for example, when a patch
gets dropped entirely from linux-next.  With Coverity, the report gets
dropped automatically.  With syzbot, it will have closed out by hand.

> What would you prefer to happen in such situations?  Commit summaries
> modified enough to confuse CI tools into *NOT* noticing that those
> are versions of the same patch?  Some kind of metadata telling the
> same tools that such-and-such commits got folded in (and they might
> have been split in process, with parts folded into different spots
> in the series, at that)?
> 
> Because "never fold in, never reorder, just accumulate patches in
> the end of the series" is not going to fly.  For a lot of reasons.

As far as I'm concerned, this is the tools problem; I don't think it's
worth it for developers to feel they need to twist themselves into
knots just to try to make the CI tools' life easier.

			     	     	- Ted

Re: BUG: unable to handle kernel paging request in do_mount

[Al Viro]

On Sat, May 18, 2019 at 04:18:43PM -0400, Theodore Ts'o wrote:

> > What would you prefer to happen in such situations?  Commit summaries
> > modified enough to confuse CI tools into *NOT* noticing that those
> > are versions of the same patch?  Some kind of metadata telling the
> > same tools that such-and-such commits got folded in (and they might
> > have been split in process, with parts folded into different spots
> > in the series, at that)?
> > 
> > Because "never fold in, never reorder, just accumulate patches in
> > the end of the series" is not going to fly.  For a lot of reasons.
> 
> As far as I'm concerned, this is the tools problem; I don't think it's
> worth it for developers to feel they need to twist themselves into
> knots just to try to make the CI tools' life easier.

FWIW, what _is_ the underlying problem?  It looks like the basic issue
is with rebase/cherry-pick of a commit; it seems to be trying to
handle two things:
	1) report X' in commit C' is similar to report X in commit C,
with C' apparently being a rebase/cherry-pick/whatnot of C; don't
want to lose that information
	2) reports X, Y and Z in commit C don't seem to be reoccuring
on the current tree, without any claimed fix in it.  Want to keep
an eye on those.

... and getting screwed by a mix of those two: reports X, Y and Z in
commit C don't seem to be reoccuring on the current tree, even though
it does contain a commit C' that seems to be a rebase of C.  A fix for
C is *not* present as an identifiable commit in the current tree.
Was it lost or was it renamed/merged with other commits/replaced by
another fix?

What I don't quite understand is why does the tool care.  Suppose
we have a buggy commit + clearly marked fix.  And see a report
very similar to the original ones, on the tree with alleged fix
clearly present.  IME the earlier reports are often quite relevant -
the fix might have been incomplete/racy/etc., and in that case
the old reports (*AND* pointer to the commit that was supposed to
have fixed those) are very useful.

What's the problem these reminders are trying to solve?  Computational
resources eaten by comparisons?

Re: BUG: unable to handle kernel paging request in do_mount

[Dmitry Vyukov]

On Sat, May 18, 2019 at 11:41 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > What would you prefer to happen in such situations?  Commit summaries
> > > modified enough to confuse CI tools into *NOT* noticing that those
> > > are versions of the same patch?  Some kind of metadata telling the
> > > same tools that such-and-such commits got folded in (and they might
> > > have been split in process, with parts folded into different spots
> > > in the series, at that)?
> > >
> > > Because "never fold in, never reorder, just accumulate patches in
> > > the end of the series" is not going to fly.  For a lot of reasons.
> >
> > As far as I'm concerned, this is the tools problem; I don't think it's
> > worth it for developers to feel they need to twist themselves into
> > knots just to try to make the CI tools' life easier.
>
> FWIW, what _is_ the underlying problem?  It looks like the basic issue
> is with rebase/cherry-pick of a commit; it seems to be trying to
> handle two things:
>         1) report X' in commit C' is similar to report X in commit C,
> with C' apparently being a rebase/cherry-pick/whatnot of C; don't
> want to lose that information
>         2) reports X, Y and Z in commit C don't seem to be reoccuring
> on the current tree, without any claimed fix in it.  Want to keep
> an eye on those.
>
> ... and getting screwed by a mix of those two: reports X, Y and Z in
> commit C don't seem to be reoccuring on the current tree, even though
> it does contain a commit C' that seems to be a rebase of C.  A fix for
> C is *not* present as an identifiable commit in the current tree.
> Was it lost or was it renamed/merged with other commits/replaced by
> another fix?
>
> What I don't quite understand is why does the tool care.  Suppose
> we have a buggy commit + clearly marked fix.  And see a report
> very similar to the original ones, on the tree with alleged fix
> clearly present.  IME the earlier reports are often quite relevant -
> the fix might have been incomplete/racy/etc., and in that case
> the old reports (*AND* pointer to the commit that was supposed to
> have fixed those) are very useful.
>
> What's the problem these reminders are trying to solve?  Computational
> resources eaten by comparisons?

syzbot, as any bug tracking system has notion of "open" and "closed"
bugs. This is useful for 2 main reasons:
 - being able to see what are the currently open bugs (on our plate)
to not go over already closed bugs again and again and to not lose
still relevant bugs (for upstream linux
https://syzkaller.appspot.com/upstream#open)
 - to be able to understand when a new similarly looking crash is
actually a new bug (either not completely fixed old one, or completely
new does not matter much) and report it again (because it's not a good
idea to send an email for every crash as is (hundreds of thousands a
day))

In order to do this tracking syzbot needs the association between
reports and fixing commits. Merely saying "it's fixed" is not enough
because consider you say "it's fixed", but it's fixed only in mm, but
not in net-next. So next second syzbot sees this crash again in
net-next and sends a new bug report as it now thinks the old one is
fixed, so this must be a new one. Only the commit allows it to
precisely understand when the fix is in all trees, and not just in all
trees but in all currently tested builds for these trees.

Above you say "on the tree with alleged fix clearly present". To
understand that syzbot needs to know the commit.

Re: BUG: unable to handle kernel paging request in do_mount

[Dmitry Vyukov]

On Sat, May 18, 2019 at 10:19 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Sat, May 18, 2019 at 05:21:42PM +0100, Al Viro wrote:
> > IOW, Dan's fix folded into the offending commit.  And that kind of
> > pattern is not rare; I would argue that appending Dan's patch at
> > the end of queue and leaving the crap in between would be a fucking
> > bad idea - it would've left a massive bisection hazard *and* made
> > life much more unpleasant when the things got to merging into the
> > mainline (or reviewing, for that matter).
>
> When this happens in the ext4 git tree, I usually don't worry about
> giving credit to whatever system finds the problem, whether coming
> from it's Coverity, or someone running sparse, or syzbot, etc.
>
> There will always be issues where there are no way to clear out the
> syzbot report via a commit description --- for example, when a patch
> gets dropped entirely from linux-next.  With Coverity, the report gets
> dropped automatically.  With syzbot, it will have closed out by hand.
>
> > What would you prefer to happen in such situations?  Commit summaries
> > modified enough to confuse CI tools into *NOT* noticing that those
> > are versions of the same patch?  Some kind of metadata telling the
> > same tools that such-and-such commits got folded in (and they might
> > have been split in process, with parts folded into different spots
> > in the series, at that)?
> >
> > Because "never fold in, never reorder, just accumulate patches in
> > the end of the series" is not going to fly.  For a lot of reasons.
>
> As far as I'm concerned, this is the tools problem; I don't think it's
> worth it for developers to feel they need to twist themselves into
> knots just to try to make the CI tools' life easier.
>
>                                         - Ted


I've added docs re linux-next handling:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#linux-next
In the end it's still just adding a tag. And it's not so much about
crediting or making somebody's life easier, this is mainly about
making Linux less buggy and higher-quality.

Re: BUG: unable to handle kernel paging request in do_mount

[Dmitry Vyukov]

.On Sat, May 18, 2019 at 6:21 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, May 18, 2019 at 05:00:39PM +0200, Dmitry Vyukov wrote:
> > On Fri, May 17, 2019 at 4:08 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> > >
> > > On Fri, May 17, 2019 at 3:48 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > >
> > > > On Fri, May 17, 2019 at 03:17:02AM -0700, syzbot wrote:
> > > > > This bug is marked as fixed by commit:
> > > > > vfs: namespace: error pointer dereference in do_remount()
> > > > > But I can't find it in any tested tree for more than 90 days.
> > > > > Is it a correct commit? Please update it by replying:
> > > > > #syz fix: exact-commit-title
> > > > > Until then the bug is still considered open and
> > > > > new crashes with the same signature are ignored.
> > > >
> > > > Could somebody explain how the following situation is supposed to
> > > > be handled:
> > > >
> > > > 1) branch B1 with commits  C1, C2, C3, C4 is pushed out
> > > > 2) C2 turns out to have a bug, which gets caught and fixed
> > > > 3) fix is folded in and branch B2 with C1, C2', C3', C4' is
> > > > pushed out.  The bug is not in it anymore.
> > > > 4) B1 is left mouldering (or is entirely removed); B2 is
> > > > eventually merged into other trees.
> > > >
> > > > This is normal and it appears to be problematic for syzbot.
> > > > How to deal with that?  One thing I will *NOT* do in such
> > > > situations is giving up on folding the fixes in.  Bisection
> > > > hazards alone make that a bad idea.
> > >
> > > linux-next creates a bit of a havoc.
> > >
> > > The ideal way of handling this is including Tested-by: tag into C2'.
> > > Reported-by: would work too, but people suggested that Reported-by: is
> > > confusing in this situation because it suggests that the commit fixes
> > > a bug in some previous commit. Technically, syzbot now accepts any
> > > tag, so With-inputs-from:
> > > syzbot+73c7fe4f77776505299b@syzkaller.appspotmail.com would work too.
> > >
> > > At this point we obvious can't fix up C2'. For such cases syzbot
> > > accepts #syz fix command to associate bugs with fixes. So replying
> > > with "#syz fix: C2'-commit-title" should do.
> >
> > What is that C2'?
>
> In this case?  Take a look at
>
> commit fd0002870b453c58d0d8c195954f5049bc6675fb
> Author: David Howells <dhowells@redhat.com>
> Date:   Tue Aug 28 14:45:06 2018 +0100
>
>     vfs: Implement a filesystem superblock creation/configuration context
>
> and compare with
>
> commit f18edd10d3c7d6127b1fa97c8f3299629cf58ed5
> Author: David Howells <dhowells@redhat.com>
> Date:   Thu Nov 1 23:07:25 2018 +0000
>
>     vfs: Implement a filesystem superblock creation/configuration context
>
> There might have been intermediate forms, but that should illustrate what
> happened.  Diff of those two contains (among other things) this:
> @@ -985,6 +989,9 @@
>  +      fc = vfs_new_fs_context(path->dentry->d_sb->s_type,
>  +                              path->dentry, sb_flags, MS_RMT_MASK,
>  +                              FS_CONTEXT_FOR_RECONFIGURE);
> ++      err = PTR_ERR(fc);
> ++      if (IS_ERR(fc))
> ++              goto err;
>  +
>  +      err = parse_monolithic_mount_data(fc, data, data_size);
>  +      if (err < 0)
>
> IOW, Dan's fix folded into the offending commit.  And that kind of
> pattern is not rare; I would argue that appending Dan's patch at
> the end of queue and leaving the crap in between would be a fucking
> bad idea - it would've left a massive bisection hazard *and* made
> life much more unpleasant when the things got to merging into the
> mainline (or reviewing, for that matter).
>
> What would you prefer to happen in such situations?  Commit summaries
> modified enough to confuse CI tools into *NOT* noticing that those
> are versions of the same patch?  Some kind of metadata telling the
> same tools that such-and-such commits got folded in (and they might
> have been split in process, with parts folded into different spots
> in the series, at that)?
>
> Because "never fold in, never reorder, just accumulate patches in
> the end of the series" is not going to fly.  For a lot of reasons.

I don't advocate for stopping folding/amending/rebasing patches in any
way. I understand this is required to get sane commits.

But what I said in the previous email still applies:
 - either include the tag into the first commit version that fixes the
reported bug
 - or link report and the fixing commit manually using the final commit title

As far as I understand in this case it would be adding Tested-by (or
some other tag) to f18edd10d3c7d6127b1fa97c8f3299629cf58ed5.
We can't do this now, so this should work:

#syz fix:
vfs: Implement a filesystem superblock creation/configuration context