Re: [PATCH] fs: fs_parser: avoid NULL param->string to kstrtouint

2019-07-19 Thread YinFengwei
On Fri, Jul 19, 2019 at 07:38:11PM +0200, Greg KH wrote:
> On Fri, Jul 19, 2019 at 08:43:29PM +0800, Yin Fengwei wrote:
> > syzbot reported general protection fault in kstrtouint:
> > https://lkml.org/lkml/2019/7/18/328
> > 
> > >From the log, if the mount option is something like:
> >fd,
> > 
> > The default parameter (which has NULL param->string) will be
> > passed to vfs_parse_fs_param. Finally, this NULL param->string
> > is passed to kstrtouint and trigger NULL pointer access.
> > 
> > Reported-by: syzbot+398343b7c1b1b9892...@syzkaller.appspotmail.com
> > Fixes: 71cbb7570a9a ("vfs: Move the subtype parameter into fuse")
> > 
> > Signed-off-by: Yin Fengwei 
> > ---
> >  fs/fs_parser.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> > index d13fe7d797c2..578e6880ac67 100644
> > --- a/fs/fs_parser.c
> > +++ b/fs/fs_parser.c
> > @@ -210,6 +210,10 @@ int fs_parse(struct fs_context *fc,
> > case fs_param_is_fd: {
> > switch (param->type) {
> > case fs_value_is_string:
> > +   if (result->has_value) {
> > +   goto bad_value;
> > +   }
> 
> Always run checkpatch.pl so grumpy maintainers do not tell you to go run
> checkpatch.pl :)

Thanks a lot for kindly reminder. Will be careful for future patch. :)

Regards
Yin, Fengwei


Re: [PATCH] fs: fs_parser: avoid NULL param->string to kstrtouint

2019-07-19 Thread YinFengwei
On Fri, Jul 19, 2019 at 03:37:37PM +0200, Dmitry Vyukov wrote:
> On Fri, Jul 19, 2019 at 2:44 PM Yin Fengwei  wrote:
> >
> > syzbot reported general protection fault in kstrtouint:
> > https://lkml.org/lkml/2019/7/18/328
> >
> > From the log, if the mount option is something like:
> >fd,
> >
> > The default parameter (which has NULL param->string) will be
> > passed to vfs_parse_fs_param. Finally, this NULL param->string
> > is passed to kstrtouint and trigger NULL pointer access.
> >
> > Reported-by: syzbot+398343b7c1b1b9892...@syzkaller.appspotmail.com
> > Fixes: 71cbb7570a9a ("vfs: Move the subtype parameter into fuse")
> >
> > Signed-off-by: Yin Fengwei 
> > ---
> >  fs/fs_parser.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> > index d13fe7d797c2..578e6880ac67 100644
> > --- a/fs/fs_parser.c
> > +++ b/fs/fs_parser.c
> > @@ -210,6 +210,10 @@ int fs_parse(struct fs_context *fc,
> > case fs_param_is_fd: {
> > switch (param->type) {
> > case fs_value_is_string:
> > +   if (result->has_value) {
> 
> !result->has_value ?
Yes. Should have ! in condition for NULL param->string. Will fix in v2.

Regards
Yin, Fengwei

> 
> > +   goto bad_value;
> > +   }
> > +
> > ret = kstrtouint(param->string, 0, 
> > >uint_32);
> > break;
> > case fs_value_is_file:
> > --
> > 2.17.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups 
> > "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an 
> > email to syzkaller-bugs+unsubscr...@googlegroups.com.
> > To view this discussion on the web visit 
> > https://groups.google.com/d/msgid/syzkaller-bugs/20190719124329.23207-1-nh26223.lmm%40gmail.com.


Re: general protection fault in kstrtouint (2)

2019-07-19 Thread YinFengwei
Hi,

On Thu, Jul 18, 2019 at 05:18:07AM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:e40115c0 Add linux-next specific files for 20190717
> git tree:   linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=11d51b7060
> kernel config:  https://syzkaller.appspot.com/x/.config?x=3430a151e1452331
> dashboard link: https://syzkaller.appspot.com/bug?extid=398343b7c1b1b989228d
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=164e743460
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1399554c60
> 
> The bug was bisected to:
> 
> commit 71cbb7570a9a0b830125163c20125a8b5e65ac45
> Author: David Howells 
> Date:   Mon Mar 25 16:38:31 2019 +
> 
> vfs: Move the subtype parameter into fuse
After this patch, if mount option is something like fd, (no "=" in ),
the default parameter which has NULL param->string is passed to
vfs_parse_fs_param. And this NULL param->string is passed to kstrtouint and
trigger null pointer access finally.


Regards
Yin, Fengwei

> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1432307860
> final crash:https://syzkaller.appspot.com/x/report.txt?x=1632307860
> console output: https://syzkaller.appspot.com/x/log.txt?x=1232307860
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+398343b7c1b1b9892...@syzkaller.appspotmail.com
> Fixes: 71cbb7570a9a ("vfs: Move the subtype parameter into fuse")
> 
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault:  [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 9017 Comm: syz-executor410 Not tainted 5.2.0-next-20190717 #40
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:kstrtoull lib/kstrtox.c:123 [inline]
> RIP: 0010:kstrtouint+0x85/0x1a0 lib/kstrtox.c:222
> Code: 04 00 f3 f3 f3 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 e8 6c 35
> 35 fe 4c 89 e2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <0f> b6 04 02 4c 89
> e2 83 e2 07 38 d0 7f 08 84 c0 0f 85 db 00 00 00
> RSP: 0018:8880997a79e0 EFLAGS: 00010246
> RAX: dc00 RBX: 8880997a7b38 RCX: 81c482dc
> RDX:  RSI: 833d4f84 RDI: 
> RBP: 8880997a7a70 R08: 8880a17ce100 R09: ed1015d06c84
> R10: ed1015d06c83 R11: 8880ae83641b R12: 
> R13: 1110132f4f3d R14: 8880997a7a48 R15: 
> FS:  56585880() GS:8880ae80() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 00455340 CR3: 95a49000 CR4: 001406f0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> Call Trace:
>  fs_parse+0xde1/0x1080 fs/fs_parser.c:209
>  fuse_parse_param+0xac/0x750 fs/fuse/inode.c:491
>  vfs_parse_fs_param+0x2ca/0x540 fs/fs_context.c:145
>  vfs_parse_fs_string+0x105/0x170 fs/fs_context.c:188
>  generic_parse_monolithic+0x181/0x200 fs/fs_context.c:228
>  parse_monolithic_mount_data+0x69/0x90 fs/fs_context.c:708
>  do_new_mount fs/namespace.c:2779 [inline]
>  do_mount+0x1369/0x1c30 fs/namespace.c:3103
>  ksys_mount+0xdb/0x150 fs/namespace.c:3312
>  __do_sys_mount fs/namespace.c:3326 [inline]
>  __se_sys_mount fs/namespace.c:3323 [inline]
>  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3323
>  do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x440299
> 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:7fff19b997e8 EFLAGS: 0246 ORIG_RAX: 00a5
> RAX: ffda RBX: 7fff19b997f0 RCX: 00440299
> RDX: 2080 RSI: 20c0 RDI: 
> RBP: 006cb018 R08: 22c0 R09: 65732f636f72702f
> R10:  R11: 0246 R12: 00401b80
> R13: 00401c10 R14:  R15: 
> Modules linked in:
> ---[ end trace 8c219e63b0160ea4 ]---
> RIP: 0010:kstrtoull lib/kstrtox.c:123 [inline]
> RIP: 0010:kstrtouint+0x85/0x1a0 lib/kstrtox.c:222
> Code: 04 00 f3 f3 f3 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 e8 6c 35
> 35 fe 4c 89 e2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <0f> b6 04 02 4c 89
> e2 83 e2 07 38 d0 7f 08 84 c0 0f 85 db 00 00 00
> RSP: 0018:8880997a79e0 EFLAGS: 00010246
> RAX: dc00 RBX: 8880997a7b38 RCX: 81c482dc
> RDX:  RSI: 833d4f84 RDI: 
> RBP: 8880997a7a70 R08: 8880a17ce100 R09: ed1015d06c84
> R10: ed1015d06c83 R11: 8880ae83641b R12: 
>