WARNING in kmem_cache_create_usercopy

WARNING in kmem_cache_create_usercopy

[syzbot]

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

Re: WARNING in kmem_cache_create_usercopy

[Dominique Martinet]

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

[PATCH] 9p/net: put a lower bound on msize

[Dominique Martinet]

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

Re: WARNING in kmem_cache_create_usercopy

[Dmitry Vyukov]

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.

Re: [PATCH] 9p/net: put a lower bound on msize

[Dominique Martinet]

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