* BUG: unable to handle kernel paging request in dput (2) @ 2019-01-30 10:35 syzbot 2019-01-30 11:11 ` Tetsuo Handa 2019-01-30 11:20 ` syzbot 0 siblings, 2 replies; 10+ messages in thread From: syzbot @ 2019-01-30 10:35 UTC (permalink / raw) To: linux-fsdevel, linux-kernel, syzkaller-bugs, viro 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=17a5bb64c00000 kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d dashboard link: https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 compiler: gcc (GCC) 9.0.0 20181231 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+b382ba6a802a3d242790@syzkaller.appspotmail.com BUG: unable to handle kernel paging request at ffffffffffffffea #PF error: [normal kernel read fault] PGD 9874067 P4D 9874067 PUD 9876067 PMD 0 Oops: 0000 [#1] PREEMPT SMP KASAN CPU: 1 PID: 18660 Comm: syz-executor2 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:fast_dput fs/dcache.c:700 [inline] RIP: 0010:dput+0x11e/0x790 fs/dcache.c:819 Code: e8 97 2e a8 ff 45 84 ed 0f 84 e2 03 00 00 e8 49 2d a8 ff 4c 89 e0 48 c1 e8 03 0f b6 04 18 84 c0 74 08 3c 03 0f 8e f7 04 00 00 <45> 8b 2c 24 4d 8d bc 24 80 00 00 00 31 ff 41 83 e5 08 44 89 ee e8 RSP: 0018:ffff888050bef740 EFLAGS: 00010246 RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffc9000c426000 RDX: 0000000000000777 RSI: ffffffff81d9ec07 RDI: 0000000000000001 RBP: ffff888050bef7d8 R08: ffff88805fc105c0 R09: ffffed1015ce5b80 R10: ffffed1015ce5b7f R11: ffff8880ae72dbfb R12: ffffffffffffffea R13: 0000000000000001 R14: 1ffff1100a17deea R15: ffff888050bef8a0 FS: 00007f5052230700(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffffffffea CR3: 00000000a5276000 CR4: 00000000001406e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: do_blk_trace_setup+0x8e9/0xdb0 kernel/trace/blktrace.c:561 __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:0x458089 Code: 6d b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 3b b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007f505222fc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000458089 RDX: 00000000200002c0 RSI: 00000000c0481273 RDI: 0000000000000003 RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 00007f50522306d4 R13: 00000000004bf54d R14: 00000000004d0e38 R15: 00000000ffffffff Modules linked in: CR2: ffffffffffffffea ---[ end trace 454866349b9af1ed ]--- RIP: 0010:fast_dput fs/dcache.c:700 [inline] RIP: 0010:dput+0x11e/0x790 fs/dcache.c:819 Code: e8 97 2e a8 ff 45 84 ed 0f 84 e2 03 00 00 e8 49 2d a8 ff 4c 89 e0 48 c1 e8 03 0f b6 04 18 84 c0 74 08 3c 03 0f 8e f7 04 00 00 <45> 8b 2c 24 4d 8d bc 24 80 00 00 00 31 ff 41 83 e5 08 44 89 ee e8 RSP: 0018:ffff888050bef740 EFLAGS: 00010246 RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffc9000c426000 RDX: 0000000000000777 RSI: ffffffff81d9ec07 RDI: 0000000000000001 RBP: ffff888050bef7d8 R08: ffff88805fc105c0 R09: ffffed1015ce5b80 R10: ffffed1015ce5b7f R11: ffff8880ae72dbfb R12: ffffffffffffffea R13: 0000000000000001 R14: 1ffff1100a17deea R15: ffff888050bef8a0 FS: 00007f5052230700(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffffffffea CR3: 00000000a5276000 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 10:35 BUG: unable to handle kernel paging request in dput (2) syzbot @ 2019-01-30 11:11 ` Tetsuo Handa 2019-01-30 11:23 ` Greg Kroah-Hartman 2019-01-30 11:26 ` Tetsuo Handa 2019-01-30 11:20 ` syzbot 1 sibling, 2 replies; 10+ messages in thread From: Tetsuo Handa @ 2019-01-30 11:11 UTC (permalink / raw) To: Omar Sandoval Cc: syzbot, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Greg Kroah-Hartman, Jens Axboe Hello, Omar. syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is dir = debugfs_lookup(buts->name, blk_debugfs_root); if (!dir) bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). Currently, Greg Kroah-Hartman is posting patches: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Omar, what do you want to do for this case? [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 11:11 ` Tetsuo Handa @ 2019-01-30 11:23 ` Greg Kroah-Hartman 2019-01-30 11:26 ` Tetsuo Handa 1 sibling, 0 replies; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-01-30 11:23 UTC (permalink / raw) To: Tetsuo Handa Cc: Omar Sandoval, syzbot, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Jens Axboe On Wed, Jan 30, 2019 at 08:11:17PM +0900, Tetsuo Handa wrote: > Hello, Omar. > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > if (!dir) > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > Currently, Greg Kroah-Hartman is posting patches: > > When calling debugfs functions, there is no need to ever check the > return value. The function can work or not, but the code logic should > never do something different based on this. > > Omar, what do you want to do for this case? > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > Obviously debugfs_lookup() should have its return value checked, so that's fine :) But how are we getting an error with dput()? Are you sure this is a debugfs issue and not a relayfs problem? I'll send patches to clean up the tracing code's use of debugfs later, as it is, I don't see anything obviously wrong with it. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 11:11 ` Tetsuo Handa 2019-01-30 11:23 ` Greg Kroah-Hartman @ 2019-01-30 11:26 ` Tetsuo Handa 2019-01-30 11:34 ` Greg Kroah-Hartman 2019-01-30 11:40 ` Greg Kroah-Hartman 1 sibling, 2 replies; 10+ messages in thread From: Tetsuo Handa @ 2019-01-30 11:26 UTC (permalink / raw) To: Omar Sandoval, Greg Kroah-Hartman Cc: syzbot, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Jens Axboe On 2019/01/30 20:11, Tetsuo Handa wrote: > Hello, Omar. > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > if (!dir) > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > Currently, Greg Kroah-Hartman is posting patches: > > When calling debugfs functions, there is no need to ever check the > return value. The function can work or not, but the code logic should > never do something different based on this. > > Omar, what do you want to do for this case? > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > The function which returned -EINVAL instead of NULL seems to be debugfs_lookup() modified by commit ff9fb72bc07705c0 ("debugfs: return error values, not NULL"). ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 11:26 ` Tetsuo Handa @ 2019-01-30 11:34 ` Greg Kroah-Hartman 2019-01-30 11:40 ` Greg Kroah-Hartman 1 sibling, 0 replies; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-01-30 11:34 UTC (permalink / raw) To: Tetsuo Handa Cc: Omar Sandoval, syzbot, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Jens Axboe On Wed, Jan 30, 2019 at 08:26:24PM +0900, Tetsuo Handa wrote: > On 2019/01/30 20:11, Tetsuo Handa wrote: > > Hello, Omar. > > > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > > if (!dir) > > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > > > Currently, Greg Kroah-Hartman is posting patches: > > > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Omar, what do you want to do for this case? > > > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > > > > The function which returned -EINVAL instead of NULL seems to be debugfs_lookup() > modified by commit ff9fb72bc07705c0 ("debugfs: return error values, not NULL"). Ah, my fault then, yes, debugfs_lookup() should return NULL if nothing is found. My patch was wrong, let me go fix this up, thanks for the report. greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 11:26 ` Tetsuo Handa 2019-01-30 11:34 ` Greg Kroah-Hartman @ 2019-01-30 11:40 ` Greg Kroah-Hartman 2019-01-31 10:09 ` Kees Cook 1 sibling, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-01-30 11:40 UTC (permalink / raw) To: Tetsuo Handa Cc: Omar Sandoval, syzbot, linux-fsdevel, linux-kernel, syzkaller-bugs, viro, Jens Axboe On Wed, Jan 30, 2019 at 08:26:24PM +0900, Tetsuo Handa wrote: > On 2019/01/30 20:11, Tetsuo Handa wrote: > > Hello, Omar. > > > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > > if (!dir) > > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > > > Currently, Greg Kroah-Hartman is posting patches: > > > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Omar, what do you want to do for this case? > > > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > > > > The function which returned -EINVAL instead of NULL seems to be debugfs_lookup() > modified by commit ff9fb72bc07705c0 ("debugfs: return error values, not NULL"). Ok, the patch below should fix this up. thanks, greg k-h ------------------------- From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Subject: [PATCH] debugfs: debugfs_lookup() should return NULL if not found Lots of callers of debugfs_lookup() were just checking NULL to see if the file/directory was found or not. By changing this in ff9fb72bc077 ("debugfs: return error values, not NULL") we caused some subsystems to easily crash. Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL") Reported-by: syzbot+b382ba6a802a3d242790@syzkaller.appspotmail.com Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: Omar Sandoval <osandov@fb.com> Cc: Jens Axboe <axboe@fb.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- fs/debugfs/inode.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index b16f8035b1af..29c68c5d44d5 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -254,8 +254,8 @@ MODULE_ALIAS_FS("debugfs"); * @parent: a pointer to the parent dentry of the file. * * This function will return a pointer to a dentry if it succeeds. If the file - * doesn't exist or an error occurs, %ERR_PTR(-ERROR) will be returned. The - * returned dentry must be passed to dput() when it is no longer needed. + * doesn't exist or an error occurs, %NULL will be returned. The returned + * dentry must be passed to dput() when it is no longer needed. * * If debugfs is not enabled in the kernel, the value -%ENODEV will be * returned. @@ -265,17 +265,17 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent) struct dentry *dentry; if (IS_ERR(parent)) - return parent; + return NULL; if (!parent) parent = debugfs_mount->mnt_root; dentry = lookup_one_len_unlocked(name, parent, strlen(name)); if (IS_ERR(dentry)) - return dentry; + return NULL; if (!d_really_is_positive(dentry)) { dput(dentry); - return ERR_PTR(-EINVAL); + return NULL; } return dentry; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 11:40 ` Greg Kroah-Hartman @ 2019-01-31 10:09 ` Kees Cook 2019-01-31 10:18 ` Tetsuo Handa 2019-01-31 10:53 ` Greg Kroah-Hartman 0 siblings, 2 replies; 10+ messages in thread From: Kees Cook @ 2019-01-31 10:09 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Tetsuo Handa, Omar Sandoval, syzbot, linux-fsdevel, LKML, syzkaller-bugs, Al Viro, Jens Axboe On Thu, Jan 31, 2019 at 12:41 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Wed, Jan 30, 2019 at 08:26:24PM +0900, Tetsuo Handa wrote: > > On 2019/01/30 20:11, Tetsuo Handa wrote: > > > Hello, Omar. > > > > > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > > > > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > > > if (!dir) > > > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > > > > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > > > > > Currently, Greg Kroah-Hartman is posting patches: > > > > > > When calling debugfs functions, there is no need to ever check the > > > return value. The function can work or not, but the code logic should > > > never do something different based on this. > > > > > > Omar, what do you want to do for this case? > > > > > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > > > > > > > The function which returned -EINVAL instead of NULL seems to be debugfs_lookup() > > modified by commit ff9fb72bc07705c0 ("debugfs: return error values, not NULL"). > > Ok, the patch below should fix this up. > > thanks, > > greg k-h > > ------------------------- > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Subject: [PATCH] debugfs: debugfs_lookup() should return NULL if not found > > Lots of callers of debugfs_lookup() were just checking NULL to see if > the file/directory was found or not. By changing this in ff9fb72bc077 > ("debugfs: return error values, not NULL") we caused some subsystems to > easily crash. > > Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL") > Reported-by: syzbot+b382ba6a802a3d242790@syzkaller.appspotmail.com > Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: Omar Sandoval <osandov@fb.com> > Cc: Jens Axboe <axboe@fb.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > fs/debugfs/inode.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c > index b16f8035b1af..29c68c5d44d5 100644 > --- a/fs/debugfs/inode.c > +++ b/fs/debugfs/inode.c > @@ -254,8 +254,8 @@ MODULE_ALIAS_FS("debugfs"); > * @parent: a pointer to the parent dentry of the file. > * > * This function will return a pointer to a dentry if it succeeds. If the file > - * doesn't exist or an error occurs, %ERR_PTR(-ERROR) will be returned. The > - * returned dentry must be passed to dput() when it is no longer needed. > + * doesn't exist or an error occurs, %NULL will be returned. The returned > + * dentry must be passed to dput() when it is no longer needed. > * > * If debugfs is not enabled in the kernel, the value -%ENODEV will be > * returned. > @@ -265,17 +265,17 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent) > struct dentry *dentry; > > if (IS_ERR(parent)) > - return parent; > + return NULL; > > if (!parent) > parent = debugfs_mount->mnt_root; > > dentry = lookup_one_len_unlocked(name, parent, strlen(name)); > if (IS_ERR(dentry)) > - return dentry; > + return NULL; > if (!d_really_is_positive(dentry)) { > dput(dentry); > - return ERR_PTR(-EINVAL); > + return NULL; > } > return dentry; > } > -- > 2.20.1 > FYI, this patch does not fix the relay.c crash I bisected... I think more clean-up is needed? -Kees ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-31 10:09 ` Kees Cook @ 2019-01-31 10:18 ` Tetsuo Handa 2019-01-31 10:53 ` Greg Kroah-Hartman 1 sibling, 0 replies; 10+ messages in thread From: Tetsuo Handa @ 2019-01-31 10:18 UTC (permalink / raw) To: Kees Cook, Greg Kroah-Hartman Cc: Omar Sandoval, syzbot, linux-fsdevel, LKML, syzkaller-bugs, Al Viro, Jens Axboe On 2019/01/31 19:09, Kees Cook wrote: > FYI, this patch does not fix the relay.c crash I bisected... I think > more clean-up is needed? You mean "general protection fault in relay_open_buf" at https://syzkaller.appspot.com/bug?id=080fbc3a5d10bd8ada100799168ebe1b70e2eefa ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-31 10:09 ` Kees Cook 2019-01-31 10:18 ` Tetsuo Handa @ 2019-01-31 10:53 ` Greg Kroah-Hartman 1 sibling, 0 replies; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-01-31 10:53 UTC (permalink / raw) To: Kees Cook Cc: Tetsuo Handa, Omar Sandoval, syzbot, linux-fsdevel, LKML, syzkaller-bugs, Al Viro, Jens Axboe On Thu, Jan 31, 2019 at 11:09:11PM +1300, Kees Cook wrote: > On Thu, Jan 31, 2019 at 12:41 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Jan 30, 2019 at 08:26:24PM +0900, Tetsuo Handa wrote: > > > On 2019/01/30 20:11, Tetsuo Handa wrote: > > > > Hello, Omar. > > > > > > > > syzbot is reporting a crash due to dput(-EINVAL) [1]. I think the location is > > > > > > > > dir = debugfs_lookup(buts->name, blk_debugfs_root); > > > > if (!dir) > > > > bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > > > > > > > > added by commit 6ac93117ab009d39 ("blktrace: use existing disk debugfs directory"). > > > > > > > > Currently, Greg Kroah-Hartman is posting patches: > > > > > > > > When calling debugfs functions, there is no need to ever check the > > > > return value. The function can work or not, but the code logic should > > > > never do something different based on this. > > > > > > > > Omar, what do you want to do for this case? > > > > > > > > [1] https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 > > > > > > > > > > The function which returned -EINVAL instead of NULL seems to be debugfs_lookup() > > > modified by commit ff9fb72bc07705c0 ("debugfs: return error values, not NULL"). > > > > Ok, the patch below should fix this up. > > > > thanks, > > > > greg k-h > > > > ------------------------- > > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Subject: [PATCH] debugfs: debugfs_lookup() should return NULL if not found > > > > Lots of callers of debugfs_lookup() were just checking NULL to see if > > the file/directory was found or not. By changing this in ff9fb72bc077 > > ("debugfs: return error values, not NULL") we caused some subsystems to > > easily crash. > > > > Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL") > > Reported-by: syzbot+b382ba6a802a3d242790@syzkaller.appspotmail.com > > Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Cc: Omar Sandoval <osandov@fb.com> > > Cc: Jens Axboe <axboe@fb.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > fs/debugfs/inode.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c > > index b16f8035b1af..29c68c5d44d5 100644 > > --- a/fs/debugfs/inode.c > > +++ b/fs/debugfs/inode.c > > @@ -254,8 +254,8 @@ MODULE_ALIAS_FS("debugfs"); > > * @parent: a pointer to the parent dentry of the file. > > * > > * This function will return a pointer to a dentry if it succeeds. If the file > > - * doesn't exist or an error occurs, %ERR_PTR(-ERROR) will be returned. The > > - * returned dentry must be passed to dput() when it is no longer needed. > > + * doesn't exist or an error occurs, %NULL will be returned. The returned > > + * dentry must be passed to dput() when it is no longer needed. > > * > > * If debugfs is not enabled in the kernel, the value -%ENODEV will be > > * returned. > > @@ -265,17 +265,17 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent) > > struct dentry *dentry; > > > > if (IS_ERR(parent)) > > - return parent; > > + return NULL; > > > > if (!parent) > > parent = debugfs_mount->mnt_root; > > > > dentry = lookup_one_len_unlocked(name, parent, strlen(name)); > > if (IS_ERR(dentry)) > > - return dentry; > > + return NULL; > > if (!d_really_is_positive(dentry)) { > > dput(dentry); > > - return ERR_PTR(-EINVAL); > > + return NULL; > > } > > return dentry; > > } > > -- > > 2.20.1 > > > > FYI, this patch does not fix the relay.c crash I bisected... I think > more clean-up is needed? Yes, you are right, I sent you a patch for that one, here it is as well. Note, this would be blowing up anyway if debugfs was not enabled, is relay.c not built if that is not the case? Ah, no, it isn't, that makes more sense now. 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] 10+ messages in thread
* Re: BUG: unable to handle kernel paging request in dput (2) 2019-01-30 10:35 BUG: unable to handle kernel paging request in dput (2) syzbot 2019-01-30 11:11 ` Tetsuo Handa @ 2019-01-30 11:20 ` syzbot 1 sibling, 0 replies; 10+ messages in thread From: syzbot @ 2019-01-30 11:20 UTC (permalink / raw) To: axboe, gregkh, linux-fsdevel, linux-kernel, osandov, penguin-kernel, syzkaller-bugs, viro syzbot has found a reproducer for 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=116209ef400000 kernel config: https://syzkaller.appspot.com/x/.config?x=a2b2e9c0bc43c14d dashboard link: https://syzkaller.appspot.com/bug?extid=b382ba6a802a3d242790 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10bdea98c00000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16390850c00000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+b382ba6a802a3d242790@syzkaller.appspotmail.com BUG: unable to handle kernel paging request at ffffffffffffffea #PF error: [normal kernel read fault] PGD 9874067 P4D 9874067 PUD 9876067 PMD 0 Oops: 0000 [#1] PREEMPT SMP KASAN CPU: 0 PID: 7985 Comm: syz-executor249 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:fast_dput fs/dcache.c:700 [inline] RIP: 0010:dput+0x11e/0x790 fs/dcache.c:819 Code: e8 97 2e a8 ff 45 84 ed 0f 84 e2 03 00 00 e8 49 2d a8 ff 4c 89 e0 48 c1 e8 03 0f b6 04 18 84 c0 74 08 3c 03 0f 8e f7 04 00 00 <45> 8b 2c 24 4d 8d bc 24 80 00 00 00 31 ff 41 83 e5 08 44 89 ee e8 RSP: 0018:ffff88808bc57740 EFLAGS: 00010246 RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81d9effa RDX: 0000000000000000 RSI: ffffffff81d9ec07 RDI: 0000000000000001 RBP: ffff88808bc577d8 R08: ffff888090abc500 R09: ffffed1015cc5b80 R10: ffffed1015cc5b7f R11: ffff8880ae62dbfb R12: ffffffffffffffea R13: 0000000000000001 R14: 1ffff1101178aeea R15: ffff88808bc578a0 FS: 0000000001144880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffffffffea CR3: 00000000946ec000 CR4: 00000000001406f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: do_blk_trace_setup+0x8e9/0xdb0 kernel/trace/blktrace.c:561 __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:0x440139 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 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fffa5cab9f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440139 RDX: 00000000200002c0 RSI: 00000000c0481273 RDI: 0000000000000003 RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8 R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004019c0 R13: 0000000000401a50 R14: 0000000000000000 R15: 0000000000000000 Modules linked in: CR2: ffffffffffffffea ---[ end trace 9fd433d20871e341 ]--- RIP: 0010:fast_dput fs/dcache.c:700 [inline] RIP: 0010:dput+0x11e/0x790 fs/dcache.c:819 Code: e8 97 2e a8 ff 45 84 ed 0f 84 e2 03 00 00 e8 49 2d a8 ff 4c 89 e0 48 c1 e8 03 0f b6 04 18 84 c0 74 08 3c 03 0f 8e f7 04 00 00 <45> 8b 2c 24 4d 8d bc 24 80 00 00 00 31 ff 41 83 e5 08 44 89 ee e8 RSP: 0018:ffff88808bc57740 EFLAGS: 00010246 RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81d9effa RDX: 0000000000000000 RSI: ffffffff81d9ec07 RDI: 0000000000000001 RBP: ffff88808bc577d8 R08: ffff888090abc500 R09: ffffed1015cc5b80 R10: ffffed1015cc5b7f R11: ffff8880ae62dbfb R12: ffffffffffffffea R13: 0000000000000001 R14: 1ffff1101178aeea R15: ffff88808bc578a0 FS: 0000000001144880(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffffffffea CR3: 00000000946ec000 CR4: 00000000001406f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-01-31 10:54 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-30 10:35 BUG: unable to handle kernel paging request in dput (2) syzbot 2019-01-30 11:11 ` Tetsuo Handa 2019-01-30 11:23 ` Greg Kroah-Hartman 2019-01-30 11:26 ` Tetsuo Handa 2019-01-30 11:34 ` Greg Kroah-Hartman 2019-01-30 11:40 ` Greg Kroah-Hartman 2019-01-31 10:09 ` Kees Cook 2019-01-31 10:18 ` Tetsuo Handa 2019-01-31 10:53 ` Greg Kroah-Hartman 2019-01-30 11:20 ` syzbot
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).