* KMSAN: uninit-value in capi_write @ 2019-02-11 22:03 syzbot 2019-09-01 14:32 ` [PATCH net] isdn/capi: check message length in capi_write() Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: syzbot @ 2019-02-11 22:03 UTC (permalink / raw) To: davem, glider, isdn, keescook, linux-kernel, netdev, rdunlap, syzkaller-bugs, viro Hello, syzbot found the following crash on: HEAD commit: 11587f6ee534 kmsan: remove pr_err git tree: kmsan console output: https://syzkaller.appspot.com/x/log.txt?x=120bd84f400000 kernel config: https://syzkaller.appspot.com/x/.config?x=c8a62a4eb8ea3e9f dashboard link: https://syzkaller.appspot.com/bug?extid=0849c524d9c634f5ae66 compiler: clang version 8.0.0 (trunk 350161) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14c53cd3400000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13e2794b400000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com ================================================================== BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2 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+0x173/0x1d0 lib/dump_stack.c:113 kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613 __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313 capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 do_loop_readv_writev fs/read_write.c:703 [inline] do_iter_write+0x83e/0xd80 fs/read_write.c:961 vfs_writev fs/read_write.c:1004 [inline] do_writev+0x397/0x840 fs/read_write.c:1039 __do_sys_writev fs/read_write.c:1112 [inline] __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 RIP: 0033:0x440079 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:00007ffd84911358 EFLAGS: 00000207 ORIG_RAX: 0000000000000014 RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440079 RDX: 0000000000000001 RSI: 0000000020000180 RDI: 0000000000000003 RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8 R10: 0000000000000000 R11: 0000000000000207 R12: 0000000000401900 R13: 0000000000401990 R14: 0000000000000000 R15: 0000000000000000 Uninit was created at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:204 [inline] kmsan_internal_poison_shadow+0x92/0x150 mm/kmsan/kmsan.c:158 kmsan_kmalloc+0xa6/0x130 mm/kmsan/kmsan_hooks.c:176 kmsan_slab_alloc+0xe/0x10 mm/kmsan/kmsan_hooks.c:185 slab_post_alloc_hook mm/slab.h:446 [inline] slab_alloc_node mm/slub.c:2759 [inline] __kmalloc_node_track_caller+0xe18/0x1030 mm/slub.c:4383 __kmalloc_reserve net/core/skbuff.c:137 [inline] __alloc_skb+0x309/0xa20 net/core/skbuff.c:205 alloc_skb include/linux/skbuff.h:998 [inline] capi_write+0x12f/0xa90 drivers/isdn/capi/capi.c:691 do_loop_readv_writev fs/read_write.c:703 [inline] do_iter_write+0x83e/0xd80 fs/read_write.c:961 vfs_writev fs/read_write.c:1004 [inline] do_writev+0x397/0x840 fs/read_write.c:1039 __do_sys_writev fs/read_write.c:1112 [inline] __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 ================================================================== --- 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
* [PATCH net] isdn/capi: check message length in capi_write() 2019-02-11 22:03 KMSAN: uninit-value in capi_write syzbot @ 2019-09-01 14:32 ` Eric Biggers 2019-09-01 18:44 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2019-09-01 14:32 UTC (permalink / raw) To: netdev, David S . Miller; +Cc: Karsten Keil, syzkaller-bugs From: Eric Biggers <ebiggers@google.com> syzbot reported: BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2 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+0x173/0x1d0 lib/dump_stack.c:113 kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613 __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313 capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 do_loop_readv_writev fs/read_write.c:703 [inline] do_iter_write+0x83e/0xd80 fs/read_write.c:961 vfs_writev fs/read_write.c:1004 [inline] do_writev+0x397/0x840 fs/read_write.c:1039 __do_sys_writev fs/read_write.c:1112 [inline] __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 [...] The problem is that capi_write() is reading past the end of the message. Fix it by checking the message's length in the needed places. Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com Signed-off-by: Eric Biggers <ebiggers@google.com> --- drivers/isdn/capi/capi.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c index 3c3ad42f22bf..a016d8c3c410 100644 --- a/drivers/isdn/capi/capi.c +++ b/drivers/isdn/capi/capi.c @@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos if (!cdev->ap.applid) return -ENODEV; + if (count < CAPIMSG_BASELEN) + return -EINVAL; + skb = alloc_skb(count, GFP_USER); if (!skb) return -ENOMEM; @@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos } mlen = CAPIMSG_LEN(skb->data); if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) { - if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) { + if (count < 18 || + (size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) { kfree_skb(skb); return -EINVAL; } @@ -711,6 +715,10 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos CAPIMSG_SETAPPID(skb->data, cdev->ap.applid); if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) { + if (count < 12) { + kfree_skb(skb); + return -EINVAL; + } mutex_lock(&cdev->lock); capincci_free(cdev, CAPIMSG_NCCI(skb->data)); mutex_unlock(&cdev->lock); -- 2.23.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] isdn/capi: check message length in capi_write() 2019-09-01 14:32 ` [PATCH net] isdn/capi: check message length in capi_write() Eric Biggers @ 2019-09-01 18:44 ` David Miller 2019-09-06 2:36 ` [PATCH net v2] " Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: David Miller @ 2019-09-01 18:44 UTC (permalink / raw) To: ebiggers; +Cc: netdev, isdn, syzkaller-bugs From: Eric Biggers <ebiggers@kernel.org> Date: Sun, 1 Sep 2019 09:32:39 -0500 > From: Eric Biggers <ebiggers@google.com> > > syzbot reported: > > BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 > CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2 > 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+0x173/0x1d0 lib/dump_stack.c:113 > kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613 > __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313 > capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 > do_loop_readv_writev fs/read_write.c:703 [inline] > do_iter_write+0x83e/0xd80 fs/read_write.c:961 > vfs_writev fs/read_write.c:1004 [inline] > do_writev+0x397/0x840 fs/read_write.c:1039 > __do_sys_writev fs/read_write.c:1112 [inline] > __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 > __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 > do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 > entry_SYSCALL_64_after_hwframe+0x63/0xe7 > [...] > > The problem is that capi_write() is reading past the end of the message. > Fix it by checking the message's length in the needed places. > > Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > drivers/isdn/capi/capi.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c > index 3c3ad42f22bf..a016d8c3c410 100644 > --- a/drivers/isdn/capi/capi.c > +++ b/drivers/isdn/capi/capi.c > @@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos > if (!cdev->ap.applid) > return -ENODEV; > > + if (count < CAPIMSG_BASELEN) > + return -EINVAL; > + > skb = alloc_skb(count, GFP_USER); > if (!skb) > return -ENOMEM; This is fine. > @@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos > } > mlen = CAPIMSG_LEN(skb->data); > if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) { > - if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) { > + if (count < 18 || ... > + if (count < 12) { These magic constants, on the other hand, are not. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v2] isdn/capi: check message length in capi_write() 2019-09-01 18:44 ` David Miller @ 2019-09-06 2:36 ` Eric Biggers 2019-09-07 15:47 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2019-09-06 2:36 UTC (permalink / raw) To: netdev, David Miller; +Cc: Karsten Keil, syzkaller-bugs From: Eric Biggers <ebiggers@google.com> syzbot reported: BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2 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+0x173/0x1d0 lib/dump_stack.c:113 kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613 __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313 capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 do_loop_readv_writev fs/read_write.c:703 [inline] do_iter_write+0x83e/0xd80 fs/read_write.c:961 vfs_writev fs/read_write.c:1004 [inline] do_writev+0x397/0x840 fs/read_write.c:1039 __do_sys_writev fs/read_write.c:1112 [inline] __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 entry_SYSCALL_64_after_hwframe+0x63/0xe7 [...] The problem is that capi_write() is reading past the end of the message. Fix it by checking the message's length in the needed places. Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com Signed-off-by: Eric Biggers <ebiggers@google.com> --- Changed v1 => v2: - Use CAPI_DATA_B3_REQ_LEN, and define and use a constant for CAPI_DISCONNECT_B3_RESP_LEN. drivers/isdn/capi/capi.c | 10 +++++++++- include/uapi/linux/isdn/capicmd.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c index 3c3ad42f22bf..c92b405b7646 100644 --- a/drivers/isdn/capi/capi.c +++ b/drivers/isdn/capi/capi.c @@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos if (!cdev->ap.applid) return -ENODEV; + if (count < CAPIMSG_BASELEN) + return -EINVAL; + skb = alloc_skb(count, GFP_USER); if (!skb) return -ENOMEM; @@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos } mlen = CAPIMSG_LEN(skb->data); if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) { - if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) { + if (count < CAPI_DATA_B3_REQ_LEN || + (size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) { kfree_skb(skb); return -EINVAL; } @@ -711,6 +715,10 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos CAPIMSG_SETAPPID(skb->data, cdev->ap.applid); if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) { + if (count < CAPI_DISCONNECT_B3_RESP_LEN) { + kfree_skb(skb); + return -EINVAL; + } mutex_lock(&cdev->lock); capincci_free(cdev, CAPIMSG_NCCI(skb->data)); mutex_unlock(&cdev->lock); diff --git a/include/uapi/linux/isdn/capicmd.h b/include/uapi/linux/isdn/capicmd.h index 4941628a4fb9..5ec88e7548a9 100644 --- a/include/uapi/linux/isdn/capicmd.h +++ b/include/uapi/linux/isdn/capicmd.h @@ -16,6 +16,7 @@ #define CAPI_MSG_BASELEN 8 #define CAPI_DATA_B3_REQ_LEN (CAPI_MSG_BASELEN+4+4+2+2+2) #define CAPI_DATA_B3_RESP_LEN (CAPI_MSG_BASELEN+4+2) +#define CAPI_DISCONNECT_B3_RESP_LEN (CAPI_MSG_BASELEN+4) /*----- CAPI commands -----*/ #define CAPI_ALERT 0x01 -- 2.23.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] isdn/capi: check message length in capi_write() 2019-09-06 2:36 ` [PATCH net v2] " Eric Biggers @ 2019-09-07 15:47 ` David Miller 0 siblings, 0 replies; 5+ messages in thread From: David Miller @ 2019-09-07 15:47 UTC (permalink / raw) To: ebiggers; +Cc: netdev, isdn, syzkaller-bugs From: Eric Biggers <ebiggers@kernel.org> Date: Thu, 5 Sep 2019 19:36:37 -0700 > From: Eric Biggers <ebiggers@google.com> > > syzbot reported: > > BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 > CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2 > 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+0x173/0x1d0 lib/dump_stack.c:113 > kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613 > __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313 > capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700 > do_loop_readv_writev fs/read_write.c:703 [inline] > do_iter_write+0x83e/0xd80 fs/read_write.c:961 > vfs_writev fs/read_write.c:1004 [inline] > do_writev+0x397/0x840 fs/read_write.c:1039 > __do_sys_writev fs/read_write.c:1112 [inline] > __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109 > __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109 > do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291 > entry_SYSCALL_64_after_hwframe+0x63/0xe7 > [...] > > The problem is that capi_write() is reading past the end of the message. > Fix it by checking the message's length in the needed places. > > Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com > Signed-off-by: Eric Biggers <ebiggers@google.com> Applied and queued up for -stable. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-07 15:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-11 22:03 KMSAN: uninit-value in capi_write syzbot 2019-09-01 14:32 ` [PATCH net] isdn/capi: check message length in capi_write() Eric Biggers 2019-09-01 18:44 ` David Miller 2019-09-06 2:36 ` [PATCH net v2] " Eric Biggers 2019-09-07 15:47 ` David Miller
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).