* WARNING in kmem_cache_create_usercopy @ 2018-11-02 15:37 syzbot 2018-11-02 22:39 ` Dominique Martinet 0 siblings, 1 reply; 5+ messages in thread From: syzbot @ 2018-11-02 15:37 UTC (permalink / raw) To: asmadeus, davem, ericvh, linux-kernel, lucho, netdev, syzkaller-bugs, v9fs-developer Hello, syzbot found the following crash on: HEAD commit: 4794a36bf08d Add linux-next specific files for 20180928 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=124f814e400000 kernel config: https://syzkaller.appspot.com/x/.config?x=b0ba9bb377f8ae91 dashboard link: https://syzkaller.appspot.com/bug?extid=0c1d61e4db7db94102ca compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1511532a400000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1701f831400000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+0c1d61e4db7db94102ca@syzkaller.appspotmail.com WARNING: CPU: 0 PID: 6065 at mm/slab_common.c:473 kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 6065 Comm: syz-executor140 Not tainted 4.19.0-rc5-next-20180928+ #84 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c4 lib/dump_stack.c:113 panic+0x238/0x4e7 kernel/panic.c:184 __warn.cold.8+0x163/0x1ba kernel/panic.c:536 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969 RIP: 0010:kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473 Code: 44 89 f0 25 00 60 de 04 45 85 ed 89 45 cc 75 0b 8b 45 d0 85 c0 0f 85 8e 01 00 00 44 39 eb 72 0a 89 d8 44 29 e8 3b 45 d0 73 7e <0f> 0b c7 45 d0 00 00 00 00 4c 8b 45 10 44 89 fa 89 de 4c 89 e7 8b RSP: 0018:ffff8801bc23f5d0 EFLAGS: 00010213 RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000006 RDX: 0000000000000000 RSI: 0000000000000020 RDI: ffffffff88b04b20 RBP: ffff8801bc23f608 R08: fffffbfff1283a2d R09: fffffbfff1283a2c R10: ffff8801bc23f5c0 R11: ffffffff8941d167 R12: ffffffff88b04b20 R13: 00000000fffffffd R14: 0000000000000000 R15: 0000000000000000 p9_client_create+0xa58/0x1769 net/9p/client.c:1054 v9fs_session_init+0x217/0x1bb0 fs/9p/v9fs.c:421 v9fs_mount+0x7c/0x8f0 fs/9p/vfs_super.c:135 legacy_get_tree+0x131/0x460 fs/fs_context.c:718 vfs_get_tree+0x1cb/0x5c0 fs/super.c:1795 do_new_mount fs/namespace.c:2648 [inline] do_mount+0x70c/0x1d90 fs/namespace.c:2974 ksys_mount+0x12d/0x140 fs/namespace.c:3190 __do_sys_mount fs/namespace.c:3204 [inline] __se_sys_mount fs/namespace.c:3201 [inline] __x64_sys_mount+0xbe/0x150 fs/namespace.c:3201 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x440189 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:00007ffdd30ec3c8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 0000000000440189 RDX: 00000000200008c0 RSI: 0000000020000000 RDI: 0000000000000000 RBP: 00000000006ca018 R08: 0000000020000a80 R09: 00000000004002c8 R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000401a10 R13: 0000000000401aa0 R14: 0000000000000000 R15: 0000000000000000 Kernel Offset: disabled Rebooting in 86400 seconds.. --- 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] 5+ messages in thread
* Re: WARNING in kmem_cache_create_usercopy 2018-11-02 15:37 WARNING in kmem_cache_create_usercopy syzbot @ 2018-11-02 22:39 ` Dominique Martinet 2018-11-05 8:52 ` [PATCH] 9p/net: put a lower bound on msize Dominique Martinet 2018-11-07 2:29 ` WARNING in kmem_cache_create_usercopy Dmitry Vyukov 0 siblings, 2 replies; 5+ messages in thread From: Dominique Martinet @ 2018-11-02 22:39 UTC (permalink / raw) To: syzbot Cc: davem, ericvh, linux-kernel, lucho, netdev, syzkaller-bugs, v9fs-developer syzbot wrote on Fri, Nov 02, 2018: > RIP: 0010:kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473 > Code: 44 89 f0 25 00 60 de 04 45 85 ed 89 45 cc 75 0b 8b 45 d0 85 c0 > 0f 85 8e 01 00 00 44 39 eb 72 0a 89 d8 44 29 e8 3b 45 d0 73 7e <0f> > 0b c7 45 d0 00 00 00 00 4c 8b 45 10 44 89 fa 89 de 4c 89 e7 8b > RSP: 0018:ffff8801bc23f5d0 EFLAGS: 00010213 > RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000006 > RDX: 0000000000000000 RSI: 0000000000000020 RDI: ffffffff88b04b20 > RBP: ffff8801bc23f608 R08: fffffbfff1283a2d R09: fffffbfff1283a2c > R10: ffff8801bc23f5c0 R11: ffffffff8941d167 R12: ffffffff88b04b20 > R13: 00000000fffffffd R14: 0000000000000000 R15: 0000000000000000 > p9_client_create+0xa58/0x1769 net/9p/client.c:1054 No lower bound on msize, the reproducer gives a reply from the pseudo-server with msize=8 and we happily take it, underflowing the msize - 11 (hdr+4) argument to kmem_cache_create_usercopy... This probably never worked anyway, but we now get a warning :) We need to add a sane lower bound to msize as well as the current upper bound set in the transport. We have some header sizes in 9p.h for IO header overhead (P9_IOHDRSZ to 24 for example) but I think that's too low in practice; stuff like readdir will require more than this to get a single entry... We can request the server to ask for at least 4k? 9p would probably work with less (e.g. 1k; I'd rather not have to figgure the minimum length we need to get each messages to work in its minimal form) but honestly even with 4k the perforamnces will be terrible, so tempted to go with that... I'll send a patch imposing 4k next week unless someone else does first, or replies indicate different preferences. @Dmitry: semi-related, the C reproducer ( https://syzkaller.appspot.com/x/repro.c?x=1701f831400000 ) has a lot of "readable" letters spelled out as "\x63..." or chars as 0x3d - it's fine for generated code and that might be easier for the intermediate representation syzkaller works with, but do you know something handy that would help convert these to readable strings? e.g. "\x63\x61\x63\x68\x65\x3d\xc0\x6d\x61\x70" could be written "cache=\xc0map", and 0x3d as '=' (hm I guess the later would not always make sense to convert so probably best left as is, but it gets annoying pretty fast with longer strings) -- Dominique ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] 9p/net: put a lower bound on msize 2018-11-02 22:39 ` Dominique Martinet @ 2018-11-05 8:52 ` Dominique Martinet 2018-11-20 6:03 ` Dominique Martinet 2018-11-07 2:29 ` WARNING in kmem_cache_create_usercopy Dmitry Vyukov 1 sibling, 1 reply; 5+ messages in thread From: Dominique Martinet @ 2018-11-05 8:52 UTC (permalink / raw) Cc: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov, syzkaller-bugs, v9fs-developer, linux-kernel, netdev From: Dominique Martinet <dominique.martinet@cea.fr> If the requested msize is too small (either from command line argument or from the server version reply), we won't get any work done. If it's *really* too small, nothing will work, and this got caught by syzbot recently (on a new kmem_cache_create_usercopy() call) Just set a minimum msize to 4k in both code paths, until someone complains they have a use-case for a smaller msize. We need to check in both mount option and server reply individually because the msize for the first version request would be unchecked with just a global check on clnt->msize. Reported-by: syzbot+0c1d61e4db7db94102ca@syzkaller.appspotmail.com Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr> Cc: Eric Van Hensbergen <ericvh@gmail.com> Cc: Latchesar Ionkov <lucho@ionkov.net> --- net/9p/client.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/net/9p/client.c b/net/9p/client.c index 2c9a17b9b46b..b1163ebdc622 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -181,6 +181,12 @@ static int parse_opts(char *opts, struct p9_client *clnt) ret = r; continue; } + if (r < 4096) { + p9_debug(P9_DEBUG_ERROR, + "msize should be at least 4k\n"); + ret = -EINVAL; + continue; + } clnt->msize = option; break; case Opt_trans: @@ -983,10 +989,18 @@ static int p9_client_version(struct p9_client *c) else if (!strncmp(version, "9P2000", 6)) c->proto_version = p9_proto_legacy; else { + p9_debug(P9_DEBUG_ERROR, + "server returned an unknown version: %s\n", version); err = -EREMOTEIO; goto error; } + if (msize < 4096) { + p9_debug(P9_DEBUG_ERROR, + "server returned a msize < 4096: %d\n", msize); + err = -EREMOTEIO; + goto error; + } if (msize < c->msize) c->msize = msize; @@ -1043,6 +1057,13 @@ struct p9_client *p9_client_create(const char *dev_name, char *options) if (clnt->msize > clnt->trans_mod->maxsize) clnt->msize = clnt->trans_mod->maxsize; + if (clnt->msize < 4096) { + p9_debug(P9_DEBUG_ERROR, + "Please specify a msize of at least 4k\n"); + err = -EINVAL; + goto free_client; + } + err = p9_client_version(clnt); if (err) goto close_trans; -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] 9p/net: put a lower bound on msize 2018-11-05 8:52 ` [PATCH] 9p/net: put a lower bound on msize Dominique Martinet @ 2018-11-20 6:03 ` Dominique Martinet 0 siblings, 0 replies; 5+ messages in thread From: Dominique Martinet @ 2018-11-20 6:03 UTC (permalink / raw) To: Dominique Martinet Cc: Eric Van Hensbergen, Latchesar Ionkov, syzkaller-bugs, v9fs-developer, linux-kernel, netdev Dominique Martinet wrote on Mon, Nov 05, 2018: > From: Dominique Martinet <dominique.martinet@cea.fr> > > If the requested msize is too small (either from command line argument > or from the server version reply), we won't get any work done. > If it's *really* too small, nothing will work, and this got caught by > syzbot recently (on a new kmem_cache_create_usercopy() call) > > Just set a minimum msize to 4k in both code paths, until someone > complains they have a use-case for a smaller msize. > > We need to check in both mount option and server reply individually > because the msize for the first version request would be unchecked > with just a global check on clnt->msize. > > Reported-by: syzbot+0c1d61e4db7db94102ca@syzkaller.appspotmail.com > Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr> > Cc: Eric Van Hensbergen <ericvh@gmail.com> > Cc: Latchesar Ionkov <lucho@ionkov.net> > --- > net/9p/client.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/net/9p/client.c b/net/9p/client.c > index 2c9a17b9b46b..b1163ebdc622 100644 > --- a/net/9p/client.c > +++ b/net/9p/client.c > @@ -181,6 +181,12 @@ static int parse_opts(char *opts, struct p9_client *clnt) > ret = r; > continue; > } > + if (r < 4096) { This should read if (option < 4096), amending what I've put in for -next last week :( Reviews wanted! > + p9_debug(P9_DEBUG_ERROR, > + "msize should be at least 4k\n"); > + ret = -EINVAL; > + continue; > + } > clnt->msize = option; > break; > case Opt_trans: > @@ -983,10 +989,18 @@ static int p9_client_version(struct p9_client *c) > else if (!strncmp(version, "9P2000", 6)) > c->proto_version = p9_proto_legacy; > else { > + p9_debug(P9_DEBUG_ERROR, > + "server returned an unknown version: %s\n", version); > err = -EREMOTEIO; > goto error; > } > > + if (msize < 4096) { > + p9_debug(P9_DEBUG_ERROR, > + "server returned a msize < 4096: %d\n", msize); > + err = -EREMOTEIO; > + goto error; > + } > if (msize < c->msize) > c->msize = msize; > > @@ -1043,6 +1057,13 @@ struct p9_client *p9_client_create(const char *dev_name, char *options) > if (clnt->msize > clnt->trans_mod->maxsize) > clnt->msize = clnt->trans_mod->maxsize; > > + if (clnt->msize < 4096) { > + p9_debug(P9_DEBUG_ERROR, > + "Please specify a msize of at least 4k\n"); > + err = -EINVAL; > + goto free_client; > + } > + > err = p9_client_version(clnt); > if (err) > goto close_trans; -- Dominique ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING in kmem_cache_create_usercopy 2018-11-02 22:39 ` Dominique Martinet 2018-11-05 8:52 ` [PATCH] 9p/net: put a lower bound on msize Dominique Martinet @ 2018-11-07 2:29 ` Dmitry Vyukov 1 sibling, 0 replies; 5+ messages in thread From: Dmitry Vyukov @ 2018-11-07 2:29 UTC (permalink / raw) To: Dominique Martinet Cc: syzbot, David Miller, Eric Van Hensbergen, LKML, Latchesar Ionkov, netdev, syzkaller-bugs, v9fs-developer On Fri, Nov 2, 2018 at 3:39 PM, Dominique Martinet <asmadeus@codewreck.org> wrote: > syzbot wrote on Fri, Nov 02, 2018: >> RIP: 0010:kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473 >> Code: 44 89 f0 25 00 60 de 04 45 85 ed 89 45 cc 75 0b 8b 45 d0 85 c0 >> 0f 85 8e 01 00 00 44 39 eb 72 0a 89 d8 44 29 e8 3b 45 d0 73 7e <0f> >> 0b c7 45 d0 00 00 00 00 4c 8b 45 10 44 89 fa 89 de 4c 89 e7 8b >> RSP: 0018:ffff8801bc23f5d0 EFLAGS: 00010213 >> RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000006 >> RDX: 0000000000000000 RSI: 0000000000000020 RDI: ffffffff88b04b20 >> RBP: ffff8801bc23f608 R08: fffffbfff1283a2d R09: fffffbfff1283a2c >> R10: ffff8801bc23f5c0 R11: ffffffff8941d167 R12: ffffffff88b04b20 >> R13: 00000000fffffffd R14: 0000000000000000 R15: 0000000000000000 >> p9_client_create+0xa58/0x1769 net/9p/client.c:1054 > > No lower bound on msize, the reproducer gives a reply from the > pseudo-server with msize=8 and we happily take it, underflowing the > msize - 11 (hdr+4) argument to kmem_cache_create_usercopy... > This probably never worked anyway, but we now get a warning :) > > > We need to add a sane lower bound to msize as well as the current upper > bound set in the transport. > We have some header sizes in 9p.h for IO header overhead (P9_IOHDRSZ to > 24 for example) but I think that's too low in practice; stuff like > readdir will require more than this to get a single entry... We can > request the server to ask for at least 4k? > 9p would probably work with less (e.g. 1k; I'd rather not have to > figgure the minimum length we need to get each messages to work in its > minimal form) but honestly even with 4k the perforamnces will be > terrible, so tempted to go with that... > > I'll send a patch imposing 4k next week unless someone else does first, > or replies indicate different preferences. > > > @Dmitry: semi-related, the C reproducer ( > https://syzkaller.appspot.com/x/repro.c?x=1701f831400000 ) has a lot of > "readable" letters spelled out as "\x63..." or chars as 0x3d - it's fine > for generated code and that might be easier for the intermediate > representation syzkaller works with, but do you know something handy > that would help convert these to readable strings? > e.g. "\x63\x61\x63\x68\x65\x3d\xc0\x6d\x61\x70" could be written > "cache=\xc0map", and 0x3d as '=' (hm I guess the later would not always > make sense to convert so probably best left as is, but it gets annoying > pretty fast with longer strings) Hi Dominique, I've filed https://github.com/google/syzkaller/issues/792 for this. Thanks for the feedback. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-20 6:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-02 15:37 WARNING in kmem_cache_create_usercopy syzbot 2018-11-02 22:39 ` Dominique Martinet 2018-11-05 8:52 ` [PATCH] 9p/net: put a lower bound on msize Dominique Martinet 2018-11-20 6:03 ` Dominique Martinet 2018-11-07 2:29 ` WARNING in kmem_cache_create_usercopy Dmitry Vyukov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.