* general protection fault in relay_open_buf
@ 2019-01-30 18:53 syzbot
2019-01-31 9:54 ` Kees Cook
0 siblings, 1 reply; 15+ messages in thread
From: syzbot @ 2019-01-30 18:53 UTC (permalink / raw)
To: akpm, ebiggers, jrdr.linux, keescook, linux-kernel, rientjes,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
#22
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
RIP: 0010:relay_open_buf.part.0+0x7cb/0xb40 kernel/relay.c:458
Code: c1 ea 03 80 3c 02 00 0f 85 4c 03 00 00 49 8d 7d 58 4d 89 ac 24 90 00
00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 1b 03 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
RSP: 0018:ffff888088897670 EFLAGS: 00010203
RAX: dffffc0000000000 RBX: ffff88809f2b3080 RCX: 1ffff110110c6919
RDX: 0000000000000008 RSI: ffffffff81833b80 RDI: 0000000000000047
RBP: ffff8880888976e8 R08: 0000000000000006 R09: ffff8880886348c8
R10: ffff888088634000 R11: 0000000000000000 R12: ffff88809f5ef580
R13: ffffffffffffffef R14: 0000000000000000 R15: 0000000000000000
FS: 00000000021c4880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006cf090 CR3: 00000000a4ccd000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
relay_open_buf kernel/relay.c:447 [inline]
relay_open kernel/relay.c:597 [inline]
relay_open+0x5f3/0xaf0 kernel/relay.c:561
do_blk_trace_setup+0x50d/0xdb0 kernel/trace/blktrace.c:532
__blk_trace_setup+0xe3/0x190 kernel/trace/blktrace.c:577
blk_trace_ioctl+0x170/0x300 kernel/trace/blktrace.c:716
blkdev_ioctl+0x141/0x2120 block/ioctl.c:591
block_ioctl+0xee/0x130 fs/block_dev.c:1914
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]
do_vfs_ioctl+0x107b/0x17d0 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+0x1a3/0x800 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x444af9
Code: e8 cc ab 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 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 db ce fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffd22903da8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000444af9
RDX: 0000000020000040 RSI: 00000000c0481273 RDI: 0000000000000003
RBP: 000000000000d8fa R08: 00000000021c4880 R09: 00000000004002e0
R10: 000000000000000f R11: 0000000000000246 R12: 0000000000401e60
R13: 0000000000401ef0 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
---[ end trace d79fd6b85e15805c ]---
RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
RIP: 0010:relay_open_buf.part.0+0x7cb/0xb40 kernel/relay.c:458
Code: c1 ea 03 80 3c 02 00 0f 85 4c 03 00 00 49 8d 7d 58 4d 89 ac 24 90 00
00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 1b 03 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
RSP: 0018:ffff888088897670 EFLAGS: 00010203
RAX: dffffc0000000000 RBX: ffff88809f2b3080 RCX: 1ffff110110c6919
RDX: 0000000000000008 RSI: ffffffff81833b80 RDI: 0000000000000047
RBP: ffff8880888976e8 R08: 0000000000000006 R09: ffff8880886348c8
R10: ffff888088634000 R11: 0000000000000000 R12: ffff88809f5ef580
R13: ffffffffffffffef R14: 0000000000000000 R15: 0000000000000000
FS: 00000000021c4880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006cf090 CR3: 00000000a4ccd000 CR4: 00000000001406f0
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-30 18:53 general protection fault in relay_open_buf syzbot
@ 2019-01-31 9:54 ` Kees Cook
2019-01-31 10:44 ` Greg KH
2019-01-31 10:51 ` Greg KH
0 siblings, 2 replies; 15+ messages in thread
From: Kees Cook @ 2019-01-31 9:54 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 7:53 AM syzbot
<syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> #22
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
static inline void relay_set_buf_dentry(struct rchan_buf *buf,
struct dentry *dentry)
{
buf->dentry = dentry;
d_inode(buf->dentry)->i_size = buf->early_bytes; <--
}
Doing a bisect landed on this:
ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
values, not NULL")
If I revert this patch, I can't reproduce any more. I don't see a
relationship, though...
My crash appears as:
[ 121.934378] BUG: unable to handle kernel NULL pointer dereference
at 0000000000000047
[ 121.937187] #PF error: [normal kernel read fault]
[ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
[ 121.941166] Oops: 0000 [#1] SMP PTI
[ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
5.0.0-rc4-next-20190130 #1020
[ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.10.2-1ubuntu1 04/01/2014
[ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
...
[ 121.960021] Call Trace:
[ 121.960453] relay_open+0x18e/0x2c0
[ 121.961070] __blk_trace_setup+0x1af/0x350
[ 121.961777] blk_trace_ioctl+0x93/0x100
$ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
relay_open_buf.part.10+0x2b8/0x330:
relay_set_buf_dentry at kernel/relay.c:412
(inlined by) relay_open_buf at kernel/relay.c:458
So it's the same location, but not sure about 0x47 offset. d_inode is
0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
but rather an ERR_PTR, the errno is either:
EBADF 9 Bad file descriptor
EEXIST 17 File exists
Neither are used in the debugfs patch, but debugfs is clearly used in
do_blk_trace_setup():
if (!blk_debugfs_root)
return -ENOENT;
...
dir = debugfs_lookup(buts->name, blk_debugfs_root);
if (!dir)
bt->dir = dir = debugfs_create_dir(buts->name,
blk_debugfs_root);
if (!dir)
goto err;
...
bt->rchan = relay_open("trace", dir, buts->buf_size,
buts->buf_nr, &blk_relay_callbacks, bt);
Which is confirmed by the next line in my traceback:
$ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
__blk_trace_setup+0x1af/0x350:
do_blk_trace_setup at kernel/trace/blktrace.c:534
(inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
-Kees
> RIP: 0010:relay_open_buf.part.0+0x7cb/0xb40 kernel/relay.c:458
> Code: c1 ea 03 80 3c 02 00 0f 85 4c 03 00 00 49 8d 7d 58 4d 89 ac 24 90 00
> 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
> 85 1b 03 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
> RSP: 0018:ffff888088897670 EFLAGS: 00010203
> RAX: dffffc0000000000 RBX: ffff88809f2b3080 RCX: 1ffff110110c6919
> RDX: 0000000000000008 RSI: ffffffff81833b80 RDI: 0000000000000047
> RBP: ffff8880888976e8 R08: 0000000000000006 R09: ffff8880886348c8
> R10: ffff888088634000 R11: 0000000000000000 R12: ffff88809f5ef580
> R13: ffffffffffffffef R14: 0000000000000000 R15: 0000000000000000
> FS: 00000000021c4880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000006cf090 CR3: 00000000a4ccd000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> relay_open_buf kernel/relay.c:447 [inline]
> relay_open kernel/relay.c:597 [inline]
> relay_open+0x5f3/0xaf0 kernel/relay.c:561
> do_blk_trace_setup+0x50d/0xdb0 kernel/trace/blktrace.c:532
> __blk_trace_setup+0xe3/0x190 kernel/trace/blktrace.c:577
> blk_trace_ioctl+0x170/0x300 kernel/trace/blktrace.c:716
> blkdev_ioctl+0x141/0x2120 block/ioctl.c:591
> block_ioctl+0xee/0x130 fs/block_dev.c:1914
> vfs_ioctl fs/ioctl.c:46 [inline]
> file_ioctl fs/ioctl.c:509 [inline]
> do_vfs_ioctl+0x107b/0x17d0 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+0x1a3/0x800 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x444af9
> Code: e8 cc ab 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 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 db ce fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffd22903da8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000444af9
> RDX: 0000000020000040 RSI: 00000000c0481273 RDI: 0000000000000003
> RBP: 000000000000d8fa R08: 00000000021c4880 R09: 00000000004002e0
> R10: 000000000000000f R11: 0000000000000246 R12: 0000000000401e60
> R13: 0000000000401ef0 R14: 0000000000000000 R15: 0000000000000000
> Modules linked in:
> ---[ end trace d79fd6b85e15805c ]---
> RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
> RIP: 0010:relay_open_buf.part.0+0x7cb/0xb40 kernel/relay.c:458
> Code: c1 ea 03 80 3c 02 00 0f 85 4c 03 00 00 49 8d 7d 58 4d 89 ac 24 90 00
> 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
> 85 1b 03 00 00 48 b8 00 00 00 00 00 fc ff df 4d 8b
> RSP: 0018:ffff888088897670 EFLAGS: 00010203
> RAX: dffffc0000000000 RBX: ffff88809f2b3080 RCX: 1ffff110110c6919
> RDX: 0000000000000008 RSI: ffffffff81833b80 RDI: 0000000000000047
> RBP: ffff8880888976e8 R08: 0000000000000006 R09: ffff8880886348c8
> R10: ffff888088634000 R11: 0000000000000000 R12: ffff88809f5ef580
> R13: ffffffffffffffef R14: 0000000000000000 R15: 0000000000000000
> FS: 00000000021c4880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000006cf090 CR3: 00000000a4ccd000 CR4: 00000000001406f0
> 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
--
Kees Cook
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 9:54 ` Kees Cook
@ 2019-01-31 10:44 ` Greg KH
2019-01-31 10:51 ` Greg KH
1 sibling, 0 replies; 15+ messages in thread
From: Greg KH @ 2019-01-31 10:44 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 10:54:18PM +1300, Kees Cook wrote:
> On Thu, Jan 31, 2019 at 7:53 AM syzbot
> <syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> > dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> > #22
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
>
> static inline void relay_set_buf_dentry(struct rchan_buf *buf,
> struct dentry *dentry)
> {
> buf->dentry = dentry;
> d_inode(buf->dentry)->i_size = buf->early_bytes; <--
> }
>
> Doing a bisect landed on this:
>
> ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
> values, not NULL")
>
> If I revert this patch, I can't reproduce any more. I don't see a
> relationship, though...
>
> My crash appears as:
> [ 121.934378] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000047
> [ 121.937187] #PF error: [normal kernel read fault]
> [ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
> [ 121.941166] Oops: 0000 [#1] SMP PTI
> [ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
> 5.0.0-rc4-next-20190130 #1020
> [ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.10.2-1ubuntu1 04/01/2014
> [ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
> ...
> [ 121.960021] Call Trace:
> [ 121.960453] relay_open+0x18e/0x2c0
> [ 121.961070] __blk_trace_setup+0x1af/0x350
> [ 121.961777] blk_trace_ioctl+0x93/0x100
>
>
> $ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
> relay_open_buf.part.10+0x2b8/0x330:
> relay_set_buf_dentry at kernel/relay.c:412
> (inlined by) relay_open_buf at kernel/relay.c:458
>
> So it's the same location, but not sure about 0x47 offset. d_inode is
> 0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
> but rather an ERR_PTR, the errno is either:
>
> EBADF 9 Bad file descriptor
> EEXIST 17 File exists
>
> Neither are used in the debugfs patch, but debugfs is clearly used in
> do_blk_trace_setup():
>
> if (!blk_debugfs_root)
> return -ENOENT;
> ...
> dir = debugfs_lookup(buts->name, blk_debugfs_root);
> if (!dir)
> bt->dir = dir = debugfs_create_dir(buts->name,
> blk_debugfs_root);
> if (!dir)
> goto err;
> ...
> bt->rchan = relay_open("trace", dir, buts->buf_size,
> buts->buf_nr, &blk_relay_callbacks, bt);
>
> Which is confirmed by the next line in my traceback:
>
> $ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
> __blk_trace_setup+0x1af/0x350:
> do_blk_trace_setup at kernel/trace/blktrace.c:534
> (inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
Ugh. Yes, this makes sense. Someone is setting up the relay code to
create a debugfs file and only checked for NULL on the return value
(which would have blown up without this patch if debugfs was not
enabled...)
Let me go fix this...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 9:54 ` Kees Cook
2019-01-31 10:44 ` Greg KH
@ 2019-01-31 10:51 ` Greg KH
2019-01-31 11:16 ` Dmitry Vyukov
` (3 more replies)
1 sibling, 4 replies; 15+ messages in thread
From: Greg KH @ 2019-01-31 10:51 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 10:54:18PM +1300, Kees Cook wrote:
> On Thu, Jan 31, 2019 at 7:53 AM syzbot
> <syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> > dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> > #22
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
>
> static inline void relay_set_buf_dentry(struct rchan_buf *buf,
> struct dentry *dentry)
> {
> buf->dentry = dentry;
> d_inode(buf->dentry)->i_size = buf->early_bytes; <--
> }
>
> Doing a bisect landed on this:
>
> ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
> values, not NULL")
>
> If I revert this patch, I can't reproduce any more. I don't see a
> relationship, though...
>
> My crash appears as:
> [ 121.934378] BUG: unable to handle kernel NULL pointer dereference
> at 0000000000000047
> [ 121.937187] #PF error: [normal kernel read fault]
> [ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
> [ 121.941166] Oops: 0000 [#1] SMP PTI
> [ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
> 5.0.0-rc4-next-20190130 #1020
> [ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.10.2-1ubuntu1 04/01/2014
> [ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
> ...
> [ 121.960021] Call Trace:
> [ 121.960453] relay_open+0x18e/0x2c0
> [ 121.961070] __blk_trace_setup+0x1af/0x350
> [ 121.961777] blk_trace_ioctl+0x93/0x100
>
>
> $ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
> relay_open_buf.part.10+0x2b8/0x330:
> relay_set_buf_dentry at kernel/relay.c:412
> (inlined by) relay_open_buf at kernel/relay.c:458
>
> So it's the same location, but not sure about 0x47 offset. d_inode is
> 0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
> but rather an ERR_PTR, the errno is either:
>
> EBADF 9 Bad file descriptor
> EEXIST 17 File exists
>
> Neither are used in the debugfs patch, but debugfs is clearly used in
> do_blk_trace_setup():
>
> if (!blk_debugfs_root)
> return -ENOENT;
> ...
> dir = debugfs_lookup(buts->name, blk_debugfs_root);
> if (!dir)
> bt->dir = dir = debugfs_create_dir(buts->name,
> blk_debugfs_root);
> if (!dir)
> goto err;
> ...
> bt->rchan = relay_open("trace", dir, buts->buf_size,
> buts->buf_nr, &blk_relay_callbacks, bt);
>
> Which is confirmed by the next line in my traceback:
>
> $ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
> __blk_trace_setup+0x1af/0x350:
> do_blk_trace_setup at kernel/trace/blktrace.c:534
> (inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
Can you test the patch below?
thanks,
greg k-h
--------------
diff --git a/kernel/relay.c b/kernel/relay.c
index 04f248644e06..9e0f52375487 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
dentry = chan->cb->create_buf_file(tmpname, chan->parent,
S_IRUSR, buf,
&chan->is_global);
+ if (IS_ERR(dentry))
+ dentry = NULL;
kfree(tmpname);
@@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
dentry = chan->cb->create_buf_file(NULL, NULL,
S_IRUSR, buf,
&chan->is_global);
- if (WARN_ON(dentry))
+ if (IS_ERR_OR_NULL(dentry))
goto free_buf;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 10:51 ` Greg KH
@ 2019-01-31 11:16 ` Dmitry Vyukov
2019-01-31 11:22 ` Greg KH
2019-01-31 11:35 ` syzbot
2019-01-31 11:29 ` Tetsuo Handa
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Dmitry Vyukov @ 2019-01-31 11:16 UTC (permalink / raw)
To: Greg KH
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
[-- Attachment #1: Type: text/plain, Size: 5469 bytes --]
On Thu, Jan 31, 2019 at 11:51 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jan 31, 2019 at 10:54:18PM +1300, Kees Cook wrote:
> > On Thu, Jan 31, 2019 at 7:53 AM syzbot
> > <syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> > > git tree: linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> > > #22
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > Google 01/01/2011
> > > RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
> >
> > static inline void relay_set_buf_dentry(struct rchan_buf *buf,
> > struct dentry *dentry)
> > {
> > buf->dentry = dentry;
> > d_inode(buf->dentry)->i_size = buf->early_bytes; <--
> > }
> >
> > Doing a bisect landed on this:
> >
> > ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
> > values, not NULL")
> >
> > If I revert this patch, I can't reproduce any more. I don't see a
> > relationship, though...
> >
> > My crash appears as:
> > [ 121.934378] BUG: unable to handle kernel NULL pointer dereference
> > at 0000000000000047
> > [ 121.937187] #PF error: [normal kernel read fault]
> > [ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
> > [ 121.941166] Oops: 0000 [#1] SMP PTI
> > [ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
> > 5.0.0-rc4-next-20190130 #1020
> > [ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> > BIOS 1.10.2-1ubuntu1 04/01/2014
> > [ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
> > ...
> > [ 121.960021] Call Trace:
> > [ 121.960453] relay_open+0x18e/0x2c0
> > [ 121.961070] __blk_trace_setup+0x1af/0x350
> > [ 121.961777] blk_trace_ioctl+0x93/0x100
> >
> >
> > $ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
> > relay_open_buf.part.10+0x2b8/0x330:
> > relay_set_buf_dentry at kernel/relay.c:412
> > (inlined by) relay_open_buf at kernel/relay.c:458
> >
> > So it's the same location, but not sure about 0x47 offset. d_inode is
> > 0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
> > but rather an ERR_PTR, the errno is either:
> >
> > EBADF 9 Bad file descriptor
> > EEXIST 17 File exists
> >
> > Neither are used in the debugfs patch, but debugfs is clearly used in
> > do_blk_trace_setup():
> >
> > if (!blk_debugfs_root)
> > return -ENOENT;
> > ...
> > dir = debugfs_lookup(buts->name, blk_debugfs_root);
> > if (!dir)
> > bt->dir = dir = debugfs_create_dir(buts->name,
> > blk_debugfs_root);
> > if (!dir)
> > goto err;
> > ...
> > bt->rchan = relay_open("trace", dir, buts->buf_size,
> > buts->buf_nr, &blk_relay_callbacks, bt);
> >
> > Which is confirmed by the next line in my traceback:
> >
> > $ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
> > __blk_trace_setup+0x1af/0x350:
> > do_blk_trace_setup at kernel/trace/blktrace.c:534
> > (inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
>
> Can you test the patch below?
This can be done as self-service by saying:
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
(is it the right tree/base commit for your change? a patch can
generally be applied only to the tree/base commit that you used to
obtain the diff)
See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches
for details.
> thanks,
>
> greg k-h
>
> --------------
>
> diff --git a/kernel/relay.c b/kernel/relay.c
> index 04f248644e06..9e0f52375487 100644
> --- a/kernel/relay.c
> +++ b/kernel/relay.c
> @@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
> dentry = chan->cb->create_buf_file(tmpname, chan->parent,
> S_IRUSR, buf,
> &chan->is_global);
> + if (IS_ERR(dentry))
> + dentry = NULL;
>
> kfree(tmpname);
>
> @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> dentry = chan->cb->create_buf_file(NULL, NULL,
> S_IRUSR, buf,
> &chan->is_global);
> - if (WARN_ON(dentry))
> + if (IS_ERR_OR_NULL(dentry))
> goto free_buf;
> }
[-- Attachment #2: relay.patch --]
[-- Type: text/x-patch, Size: 664 bytes --]
diff --git a/kernel/relay.c b/kernel/relay.c
index 04f248644e06..9e0f52375487 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
dentry = chan->cb->create_buf_file(tmpname, chan->parent,
S_IRUSR, buf,
&chan->is_global);
+ if (IS_ERR(dentry))
+ dentry = NULL;
kfree(tmpname);
@@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
dentry = chan->cb->create_buf_file(NULL, NULL,
S_IRUSR, buf,
&chan->is_global);
- if (WARN_ON(dentry))
+ if (IS_ERR_OR_NULL(dentry))
goto free_buf;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 11:16 ` Dmitry Vyukov
@ 2019-01-31 11:22 ` Greg KH
2019-01-31 11:28 ` Dmitry Vyukov
2019-01-31 11:53 ` syzbot
2019-01-31 11:35 ` syzbot
1 sibling, 2 replies; 15+ messages in thread
From: Greg KH @ 2019-01-31 11:22 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
[-- Attachment #1: Type: text/plain, Size: 4830 bytes --]
On Thu, Jan 31, 2019 at 12:16:42PM +0100, Dmitry Vyukov wrote:
> On Thu, Jan 31, 2019 at 11:51 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jan 31, 2019 at 10:54:18PM +1300, Kees Cook wrote:
> > > On Thu, Jan 31, 2019 at 7:53 AM syzbot
> > > <syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> > > > git tree: linux-next
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> > > > kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> > > > #22
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > Google 01/01/2011
> > > > RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
> > >
> > > static inline void relay_set_buf_dentry(struct rchan_buf *buf,
> > > struct dentry *dentry)
> > > {
> > > buf->dentry = dentry;
> > > d_inode(buf->dentry)->i_size = buf->early_bytes; <--
> > > }
> > >
> > > Doing a bisect landed on this:
> > >
> > > ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
> > > values, not NULL")
> > >
> > > If I revert this patch, I can't reproduce any more. I don't see a
> > > relationship, though...
> > >
> > > My crash appears as:
> > > [ 121.934378] BUG: unable to handle kernel NULL pointer dereference
> > > at 0000000000000047
> > > [ 121.937187] #PF error: [normal kernel read fault]
> > > [ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
> > > [ 121.941166] Oops: 0000 [#1] SMP PTI
> > > [ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
> > > 5.0.0-rc4-next-20190130 #1020
> > > [ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> > > BIOS 1.10.2-1ubuntu1 04/01/2014
> > > [ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
> > > ...
> > > [ 121.960021] Call Trace:
> > > [ 121.960453] relay_open+0x18e/0x2c0
> > > [ 121.961070] __blk_trace_setup+0x1af/0x350
> > > [ 121.961777] blk_trace_ioctl+0x93/0x100
> > >
> > >
> > > $ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
> > > relay_open_buf.part.10+0x2b8/0x330:
> > > relay_set_buf_dentry at kernel/relay.c:412
> > > (inlined by) relay_open_buf at kernel/relay.c:458
> > >
> > > So it's the same location, but not sure about 0x47 offset. d_inode is
> > > 0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
> > > but rather an ERR_PTR, the errno is either:
> > >
> > > EBADF 9 Bad file descriptor
> > > EEXIST 17 File exists
> > >
> > > Neither are used in the debugfs patch, but debugfs is clearly used in
> > > do_blk_trace_setup():
> > >
> > > if (!blk_debugfs_root)
> > > return -ENOENT;
> > > ...
> > > dir = debugfs_lookup(buts->name, blk_debugfs_root);
> > > if (!dir)
> > > bt->dir = dir = debugfs_create_dir(buts->name,
> > > blk_debugfs_root);
> > > if (!dir)
> > > goto err;
> > > ...
> > > bt->rchan = relay_open("trace", dir, buts->buf_size,
> > > buts->buf_nr, &blk_relay_callbacks, bt);
> > >
> > > Which is confirmed by the next line in my traceback:
> > >
> > > $ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
> > > __blk_trace_setup+0x1af/0x350:
> > > do_blk_trace_setup at kernel/trace/blktrace.c:534
> > > (inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
> >
> > Can you test the patch below?
>
>
> This can be done as self-service by saying:
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> master
>
> (is it the right tree/base commit for your change? a patch can
> generally be applied only to the tree/base commit that you used to
> obtain the diff)
It was close, wrong tree, try this:
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git driver-core-linus
And let's see if it works :)
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 665 bytes --]
diff --git a/kernel/relay.c b/kernel/relay.c
index 04f248644e06..9e0f52375487 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
dentry = chan->cb->create_buf_file(tmpname, chan->parent,
S_IRUSR, buf,
&chan->is_global);
+ if (IS_ERR(dentry))
+ dentry = NULL;
kfree(tmpname);
@@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
dentry = chan->cb->create_buf_file(NULL, NULL,
S_IRUSR, buf,
&chan->is_global);
- if (WARN_ON(dentry))
+ if (IS_ERR_OR_NULL(dentry))
goto free_buf;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 11:22 ` Greg KH
@ 2019-01-31 11:28 ` Dmitry Vyukov
2019-01-31 11:53 ` syzbot
1 sibling, 0 replies; 15+ messages in thread
From: Dmitry Vyukov @ 2019-01-31 11:28 UTC (permalink / raw)
To: Greg KH
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 12:22 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jan 31, 2019 at 12:16:42PM +0100, Dmitry Vyukov wrote:
> > On Thu, Jan 31, 2019 at 11:51 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Jan 31, 2019 at 10:54:18PM +1300, Kees Cook wrote:
> > > > On Thu, Jan 31, 2019 at 7:53 AM syzbot
> > > > <syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > syzbot found the following crash on:
> > > > >
> > > > > HEAD commit: 02495e76ded5 Add linux-next specific files for 20190130
> > > > > git tree: linux-next
> > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12cf10df400000
> > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d
> > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=16c3a70e1e9b29346c43
> > > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13266698c00000
> > > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1715bb64c00000
> > > > >
> > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > Reported-by: syzbot+16c3a70e1e9b29346c43@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: 0 PID: 8092 Comm: syz-executor405 Not tainted 5.0.0-rc4-next-20190130
> > > > > #22
> > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > Google 01/01/2011
> > > > > RIP: 0010:relay_set_buf_dentry kernel/relay.c:412 [inline]
> > > >
> > > > static inline void relay_set_buf_dentry(struct rchan_buf *buf,
> > > > struct dentry *dentry)
> > > > {
> > > > buf->dentry = dentry;
> > > > d_inode(buf->dentry)->i_size = buf->early_bytes; <--
> > > > }
> > > >
> > > > Doing a bisect landed on this:
> > > >
> > > > ff9fb72bc07705c00795ca48631f7fffe24d2c6b ("debugfs: return error
> > > > values, not NULL")
> > > >
> > > > If I revert this patch, I can't reproduce any more. I don't see a
> > > > relationship, though...
> > > >
> > > > My crash appears as:
> > > > [ 121.934378] BUG: unable to handle kernel NULL pointer dereference
> > > > at 0000000000000047
> > > > [ 121.937187] #PF error: [normal kernel read fault]
> > > > [ 121.938824] PGD 800000041f699067 P4D 800000041f699067 PUD 42d08f067 PMD 0
> > > > [ 121.941166] Oops: 0000 [#1] SMP PTI
> > > > [ 121.942381] CPU: 2 PID: 3134 Comm: relay Not tainted
> > > > 5.0.0-rc4-next-20190130 #1020
> > > > [ 121.943873] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> > > > BIOS 1.10.2-1ubuntu1 04/01/2014
> > > > [ 121.945395] RIP: 0010:relay_open_buf.part.10+0x2b8/0x330
> > > > ...
> > > > [ 121.960021] Call Trace:
> > > > [ 121.960453] relay_open+0x18e/0x2c0
> > > > [ 121.961070] __blk_trace_setup+0x1af/0x350
> > > > [ 121.961777] blk_trace_ioctl+0x93/0x100
> > > >
> > > >
> > > > $ ./scripts/faddr2line vmlinux relay_open_buf.part.10+0x2b8/0x330
> > > > relay_open_buf.part.10+0x2b8/0x330:
> > > > relay_set_buf_dentry at kernel/relay.c:412
> > > > (inlined by) relay_open_buf at kernel/relay.c:458
> > > >
> > > > So it's the same location, but not sure about 0x47 offset. d_inode is
> > > > 0x58 from dentry. And i_size is 0x50 from inode. If this isn't NULL,
> > > > but rather an ERR_PTR, the errno is either:
> > > >
> > > > EBADF 9 Bad file descriptor
> > > > EEXIST 17 File exists
> > > >
> > > > Neither are used in the debugfs patch, but debugfs is clearly used in
> > > > do_blk_trace_setup():
> > > >
> > > > if (!blk_debugfs_root)
> > > > return -ENOENT;
> > > > ...
> > > > dir = debugfs_lookup(buts->name, blk_debugfs_root);
> > > > if (!dir)
> > > > bt->dir = dir = debugfs_create_dir(buts->name,
> > > > blk_debugfs_root);
> > > > if (!dir)
> > > > goto err;
> > > > ...
> > > > bt->rchan = relay_open("trace", dir, buts->buf_size,
> > > > buts->buf_nr, &blk_relay_callbacks, bt);
> > > >
> > > > Which is confirmed by the next line in my traceback:
> > > >
> > > > $ ./scripts/faddr2line vmlinux __blk_trace_setup+0x1af/0x350
> > > > __blk_trace_setup+0x1af/0x350:
> > > > do_blk_trace_setup at kernel/trace/blktrace.c:534
> > > > (inlined by) __blk_trace_setup at kernel/trace/blktrace.c:577
> > >
> > > Can you test the patch below?
> >
> >
> > This can be done as self-service by saying:
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > master
> >
> > (is it the right tree/base commit for your change? a patch can
> > generally be applied only to the tree/base commit that you used to
> > obtain the diff)
>
> It was close, wrong tree, try this:
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git driver-core-linus
>
> And let's see if it works :)
Just in case, you can actually post patches inline, it should work.
It's just my client is incapable of preserving whitespaces, so I have
to re-attach them (they are also applied with --ignore-whitespace, but
just to be safe).
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 10:51 ` Greg KH
2019-01-31 11:16 ` Dmitry Vyukov
@ 2019-01-31 11:29 ` Tetsuo Handa
2019-01-31 11:54 ` Greg KH
2019-01-31 18:31 ` Kees Cook
2019-02-01 3:57 ` Al Viro
3 siblings, 1 reply; 15+ messages in thread
From: Tetsuo Handa @ 2019-01-31 11:29 UTC (permalink / raw)
To: Greg KH, Kees Cook
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On 2019/01/31 19:51, Greg KH wrote:
> Can you test the patch below?
You can ask syzbot to test the patch. But
> @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> dentry = chan->cb->create_buf_file(NULL, NULL,
> S_IRUSR, buf,
> &chan->is_global);
> - if (WARN_ON(dentry))
> + if (IS_ERR_OR_NULL(dentry))
> goto free_buf;
are you trying to fix a different bug together that old code was by error bailing
out when chan->cb->create_buf_file() returned a valid "struct dentry *" ?
I don't know what WARN_ON() due to a valid "struct dentry *" means...
> }
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 11:16 ` Dmitry Vyukov
2019-01-31 11:22 ` Greg KH
@ 2019-01-31 11:35 ` syzbot
1 sibling, 0 replies; 15+ messages in thread
From: syzbot @ 2019-01-31 11:35 UTC (permalink / raw)
To: akpm, dvyukov, ebiggers, gregkh, jrdr.linux, keescook,
linux-kernel, rientjes, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger
crash:
Reported-and-tested-by:
syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com
Tested on:
commit: af0c9af1b3f6 fs/dcache: Track & report number of negative ..
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=3495238483eab50f
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=166d48df400000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 11:22 ` Greg KH
2019-01-31 11:28 ` Dmitry Vyukov
@ 2019-01-31 11:53 ` syzbot
1 sibling, 0 replies; 15+ messages in thread
From: syzbot @ 2019-01-31 11:53 UTC (permalink / raw)
To: akpm, dvyukov, ebiggers, gregkh, jrdr.linux, keescook,
linux-kernel, rientjes, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger
crash:
Reported-and-tested-by:
syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com
Tested on:
commit: 37ea7b630ae5 debugfs: debugfs_lookup() should return NULL ..
git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
driver-core-linus
kernel config: https://syzkaller.appspot.com/x/.config?x=a04dcc98f2be2fd8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=16ecad28c00000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 11:29 ` Tetsuo Handa
@ 2019-01-31 11:54 ` Greg KH
0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2019-01-31 11:54 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 08:29:37PM +0900, Tetsuo Handa wrote:
> On 2019/01/31 19:51, Greg KH wrote:
> > Can you test the patch below?
>
> You can ask syzbot to test the patch. But
>
> > @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> > dentry = chan->cb->create_buf_file(NULL, NULL,
> > S_IRUSR, buf,
> > &chan->is_global);
> > - if (WARN_ON(dentry))
> > + if (IS_ERR_OR_NULL(dentry))
> > goto free_buf;
>
> are you trying to fix a different bug together that old code was by error bailing
> out when chan->cb->create_buf_file() returned a valid "struct dentry *" ?
> I don't know what WARN_ON() due to a valid "struct dentry *" means...
I don't either, I'm guessing that's a code path that has never been
taken, or everyone just ignores it :)
Anyway, I fixed it up to be safe. And the reproducer shows it now works
properly too.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 10:51 ` Greg KH
2019-01-31 11:16 ` Dmitry Vyukov
2019-01-31 11:29 ` Tetsuo Handa
@ 2019-01-31 18:31 ` Kees Cook
2019-01-31 18:46 ` Greg KH
2019-02-01 3:57 ` Al Viro
3 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2019-01-31 18:31 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 11:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> Can you test the patch below?
>
> thanks,
>
> greg k-h
>
> --------------
>
> diff --git a/kernel/relay.c b/kernel/relay.c
> index 04f248644e06..9e0f52375487 100644
> --- a/kernel/relay.c
> +++ b/kernel/relay.c
> @@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
> dentry = chan->cb->create_buf_file(tmpname, chan->parent,
> S_IRUSR, buf,
> &chan->is_global);
> + if (IS_ERR(dentry))
> + dentry = NULL;
>
> kfree(tmpname);
>
> @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> dentry = chan->cb->create_buf_file(NULL, NULL,
> S_IRUSR, buf,
> &chan->is_global);
> - if (WARN_ON(dentry))
> + if (IS_ERR_OR_NULL(dentry))
> goto free_buf;
> }
>
Thanks! (Can we find other cases of this with static analysis?)
Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL")
Reported-by: syzbot+16c3a70e1e9b29346c43@syzkaller.appspotmail.com
Tested-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 18:31 ` Kees Cook
@ 2019-01-31 18:46 ` Greg KH
0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2019-01-31 18:46 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, syzbot, Eric Biggers, Souptick Joarder, LKML,
David Rientjes, syzkaller-bugs
On Fri, Feb 01, 2019 at 07:31:48AM +1300, Kees Cook wrote:
> On Thu, Jan 31, 2019 at 11:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > Can you test the patch below?
> >
> > thanks,
> >
> > greg k-h
> >
> > --------------
> >
> > diff --git a/kernel/relay.c b/kernel/relay.c
> > index 04f248644e06..9e0f52375487 100644
> > --- a/kernel/relay.c
> > +++ b/kernel/relay.c
> > @@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
> > dentry = chan->cb->create_buf_file(tmpname, chan->parent,
> > S_IRUSR, buf,
> > &chan->is_global);
> > + if (IS_ERR(dentry))
> > + dentry = NULL;
> >
> > kfree(tmpname);
> >
> > @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> > dentry = chan->cb->create_buf_file(NULL, NULL,
> > S_IRUSR, buf,
> > &chan->is_global);
> > - if (WARN_ON(dentry))
> > + if (IS_ERR_OR_NULL(dentry))
> > goto free_buf;
> > }
> >
>
> Thanks! (Can we find other cases of this with static analysis?)
Probably. I have over 100 patches to help clean up a lot of the debugfs
mess. But it is very rare that someone actually tries to use the result
of a debugfs call as a "real" dentry, except to pass it back into
another debugfs call. I "think" I have now caught all of those cases,
and if you can come up with some kind of rule for this, that would be
great.
But note, the create_buf_file() callback is the one that does the
debugfs call, so trying to figure out where that is coming from, what it
does, and what the dentry is later used for, spans lots of different
subsystems and files. I don't think we have tools to do that, other
than grep :)
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-01-31 10:51 ` Greg KH
` (2 preceding siblings ...)
2019-01-31 18:31 ` Kees Cook
@ 2019-02-01 3:57 ` Al Viro
2019-02-01 9:07 ` Greg KH
3 siblings, 1 reply; 15+ messages in thread
From: Al Viro @ 2019-02-01 3:57 UTC (permalink / raw)
To: Greg KH
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
On Thu, Jan 31, 2019 at 11:51:52AM +0100, Greg KH wrote:
> Can you test the patch below?
>
> thanks,
>
> greg k-h
>
> @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> dentry = chan->cb->create_buf_file(NULL, NULL,
> S_IRUSR, buf,
> &chan->is_global);
> - if (WARN_ON(dentry))
> + if (IS_ERR_OR_NULL(dentry))
> goto free_buf;
Huh? That makes no sense; is it IS_ERR on error or is it NULL
on error, or what? Besides, how did it work before?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: general protection fault in relay_open_buf
2019-02-01 3:57 ` Al Viro
@ 2019-02-01 9:07 ` Greg KH
0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2019-02-01 9:07 UTC (permalink / raw)
To: Al Viro
Cc: Kees Cook, Andrew Morton, syzbot, Eric Biggers, Souptick Joarder,
LKML, David Rientjes, syzkaller-bugs
On Fri, Feb 01, 2019 at 03:57:48AM +0000, Al Viro wrote:
> On Thu, Jan 31, 2019 at 11:51:52AM +0100, Greg KH wrote:
> > Can you test the patch below?
> >
> > thanks,
> >
> > greg k-h
> >
>
> > @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> > dentry = chan->cb->create_buf_file(NULL, NULL,
> > S_IRUSR, buf,
> > &chan->is_global);
> > - if (WARN_ON(dentry))
> > + if (IS_ERR_OR_NULL(dentry))
> > goto free_buf;
>
> Huh? That makes no sense; is it IS_ERR on error or is it NULL
> on error, or what?
Some of the fuction pointers seem to do one, some the other. I've
submitted patches to them to unify them now. Will take bit for those to
wind through the merges.
> Besides, how did it work before?
Obviously it never did at all :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-02-01 9:07 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 18:53 general protection fault in relay_open_buf syzbot
2019-01-31 9:54 ` Kees Cook
2019-01-31 10:44 ` Greg KH
2019-01-31 10:51 ` Greg KH
2019-01-31 11:16 ` Dmitry Vyukov
2019-01-31 11:22 ` Greg KH
2019-01-31 11:28 ` Dmitry Vyukov
2019-01-31 11:53 ` syzbot
2019-01-31 11:35 ` syzbot
2019-01-31 11:29 ` Tetsuo Handa
2019-01-31 11:54 ` Greg KH
2019-01-31 18:31 ` Kees Cook
2019-01-31 18:46 ` Greg KH
2019-02-01 3:57 ` Al Viro
2019-02-01 9:07 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).