Message ID | 20170425131827.66498-1-glider@google.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
From: Alexander Potapenko <glider@google.com> Date: Tue, 25 Apr 2017 15:18:27 +0200 > rawv6_send_hdrinc() expects that the buffer copied from the userspace > contains the IPv6 header, so if too few bytes are copied parts of the > header may remain uninitialized. > > This bug has been detected with KMSAN. > > Signed-off-by: Alexander Potapenko <glider@google.com> Hmmm, ipv4 seems to lack this check as well. I think we need to be careful here and fully understand why KMSAN doesn't seem to be triggering in the ipv4 case but for ipv6 it is before I apply this. Thanks.
On Tue, Apr 25, 2017 at 10:55 AM, David Miller <davem@davemloft.net> wrote: > From: Alexander Potapenko <glider@google.com> > Date: Tue, 25 Apr 2017 15:18:27 +0200 > >> rawv6_send_hdrinc() expects that the buffer copied from the userspace >> contains the IPv6 header, so if too few bytes are copied parts of the >> header may remain uninitialized. >> >> This bug has been detected with KMSAN. >> >> Signed-off-by: Alexander Potapenko <glider@google.com> > > Hmmm, ipv4 seems to lack this check as well. > > I think we need to be careful here and fully understand why KMSAN doesn't > seem to be triggering in the ipv4 case but for ipv6 it is before I apply > this. This could be a bug in nf_ct_frag6_gather() missing one pskb_may_pull()
On Tue, Apr 25, 2017 at 7:55 PM, David Miller <davem@davemloft.net> wrote: > From: Alexander Potapenko <glider@google.com> > Date: Tue, 25 Apr 2017 15:18:27 +0200 > >> rawv6_send_hdrinc() expects that the buffer copied from the userspace >> contains the IPv6 header, so if too few bytes are copied parts of the >> header may remain uninitialized. >> >> This bug has been detected with KMSAN. >> >> Signed-off-by: Alexander Potapenko <glider@google.com> > > Hmmm, ipv4 seems to lack this check as well. > > I think we need to be careful here and fully understand why KMSAN doesn't > seem to be triggering in the ipv4 case but for ipv6 it is before I apply > this. Maybe I just couldn't come up with a decent test case for ipv4 yet. syzkaller generated the equivalent of the following program for ipv6: ======================================= #define _GNU_SOURCE #include <netinet/in.h> #include <string.h> #include <sys/socket.h> #include <error.h> int main() { int sock = socket(PF_INET6, SOCK_RAW, IPPROTO_RAW); struct sockaddr_in6 dest_addr; memset(&dest_addr, 0, sizeof(dest_addr)); dest_addr.sin6_family = AF_INET6; inet_pton(AF_INET6, "ff00::", &dest_addr.sin6_addr); int err = sendto(sock, 0, 0, 0, &dest_addr, sizeof(dest_addr)); if (err == -1) perror("sendto"); return 0; } ======================================= I attempted to replace INET6 and such with INET and provide a legal IPv4 address to inet_pton(), but couldn't trigger the warning. > Thanks.
On Wed, Apr 26, 2017 at 10:54 AM, Alexander Potapenko <glider@google.com> wrote: > On Tue, Apr 25, 2017 at 7:55 PM, David Miller <davem@davemloft.net> wrote: >> From: Alexander Potapenko <glider@google.com> >> Date: Tue, 25 Apr 2017 15:18:27 +0200 >> >>> rawv6_send_hdrinc() expects that the buffer copied from the userspace >>> contains the IPv6 header, so if too few bytes are copied parts of the >>> header may remain uninitialized. >>> >>> This bug has been detected with KMSAN. >>> >>> Signed-off-by: Alexander Potapenko <glider@google.com> >> >> Hmmm, ipv4 seems to lack this check as well. >> >> I think we need to be careful here and fully understand why KMSAN doesn't >> seem to be triggering in the ipv4 case but for ipv6 it is before I apply >> this. > Maybe I just couldn't come up with a decent test case for ipv4 yet. > syzkaller generated the equivalent of the following program for ipv6: > > ======================================= > #define _GNU_SOURCE > > #include <netinet/in.h> > #include <string.h> > #include <sys/socket.h> > #include <error.h> > > int main() > { > int sock = socket(PF_INET6, SOCK_RAW, IPPROTO_RAW); > struct sockaddr_in6 dest_addr; > memset(&dest_addr, 0, sizeof(dest_addr)); > dest_addr.sin6_family = AF_INET6; > inet_pton(AF_INET6, "ff00::", &dest_addr.sin6_addr); > int err = sendto(sock, 0, 0, 0, &dest_addr, sizeof(dest_addr)); > if (err == -1) > perror("sendto"); > return 0; > } > ======================================= > > I attempted to replace INET6 and such with INET and provide a legal > IPv4 address to inet_pton(), but couldn't trigger the warning. syzkaller reports the following errors in the wild (although there's no stable repro): ================================================================== BUG: KMSAN: use of unitialized memory in raw_send_hdrinc net/ipv4/raw.c:407 [inline] BUG: KMSAN: use of unitialized memory in raw_sendmsg+0x2c8b/0x3400 net/ipv4/raw.c:640 inter: 0 CPU: 3 PID: 2853 Comm: syz-executor1 Not tainted 4.11.0-rc5+ #2445 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 [inline] dump_stack+0x143/0x1b0 lib/dump_stack.c:52 kmsan_report+0x16b/0x1e0 mm/kmsan/kmsan.c:1078 __kmsan_warning_32+0x5c/0xa0 mm/kmsan/kmsan_instr.c:510 raw_send_hdrinc net/ipv4/raw.c:407 [inline] raw_sendmsg+0x2c8b/0x3400 net/ipv4/raw.c:640 inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 [inline] sock_sendmsg net/socket.c:643 [inline] ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 __sys_sendmsg net/socket.c:2031 [inline] SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 SyS_sendmsg+0x87/0xb0 net/socket.c:2038 entry_SYSCALL_64_fastpath+0x13/0x94 RIP: 0033:0x44a669 RSP: 002b:00007f08d06edb58 EFLAGS: 00000286 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 0000000000708000 RCX: 000000000044a669 RDX: 0000000000000000 RSI: 0000000020007fc8 RDI: 0000000000000017 RBP: 0000000000005080 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000286 R12: 00000000006e4140 R13: 0000000020b22000 R14: 0000000000000000 R15: 0000000000000000 origin: 00000000cb200085 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:362 [inline] kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:257 kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:270 slab_alloc_node mm/slub.c:2735 [inline] __kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4341 __kmalloc_reserve net/core/skbuff.c:138 [inline] __alloc_skb+0x2cd/0x740 net/core/skbuff.c:231 alloc_skb include/linux/skbuff.h:933 [inline] alloc_skb_with_frags+0x209/0xbc0 net/core/skbuff.c:4678 sock_alloc_send_pskb+0x9ff/0xe00 net/core/sock.c:1903 sock_alloc_send_skb+0xe4/0x100 net/core/sock.c:1920 raw_send_hdrinc net/ipv4/raw.c:366 [inline] raw_sendmsg+0x1db4/0x3400 net/ipv4/raw.c:640 inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 [inline] sock_sendmsg net/socket.c:643 [inline] ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 __sys_sendmsg net/socket.c:2031 [inline] SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 SyS_sendmsg+0x87/0xb0 net/socket.c:2038 entry_SYSCALL_64_fastpath+0x13/0x94 ================================================================== > > > > -- > Alexander Potapenko > Software Engineer > > Google Germany GmbH > Erika-Mann-Straße, 33 > 80636 München > > Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg
On Wed, Apr 26, 2017 at 3:00 PM, Alexander Potapenko <glider@google.com> wrote: > On Wed, Apr 26, 2017 at 10:54 AM, Alexander Potapenko <glider@google.com> wrote: >> On Tue, Apr 25, 2017 at 7:55 PM, David Miller <davem@davemloft.net> wrote: >>> From: Alexander Potapenko <glider@google.com> >>> Date: Tue, 25 Apr 2017 15:18:27 +0200 >>> >>>> rawv6_send_hdrinc() expects that the buffer copied from the userspace >>>> contains the IPv6 header, so if too few bytes are copied parts of the >>>> header may remain uninitialized. >>>> >>>> This bug has been detected with KMSAN. >>>> >>>> Signed-off-by: Alexander Potapenko <glider@google.com> >>> >>> Hmmm, ipv4 seems to lack this check as well. >>> >>> I think we need to be careful here and fully understand why KMSAN doesn't >>> seem to be triggering in the ipv4 case but for ipv6 it is before I apply >>> this. >> Maybe I just couldn't come up with a decent test case for ipv4 yet. >> syzkaller generated the equivalent of the following program for ipv6: >> >> ======================================= >> #define _GNU_SOURCE >> >> #include <netinet/in.h> >> #include <string.h> >> #include <sys/socket.h> >> #include <error.h> >> >> int main() >> { >> int sock = socket(PF_INET6, SOCK_RAW, IPPROTO_RAW); >> struct sockaddr_in6 dest_addr; >> memset(&dest_addr, 0, sizeof(dest_addr)); >> dest_addr.sin6_family = AF_INET6; >> inet_pton(AF_INET6, "ff00::", &dest_addr.sin6_addr); >> int err = sendto(sock, 0, 0, 0, &dest_addr, sizeof(dest_addr)); >> if (err == -1) >> perror("sendto"); >> return 0; >> } >> ======================================= >> >> I attempted to replace INET6 and such with INET and provide a legal >> IPv4 address to inet_pton(), but couldn't trigger the warning. > syzkaller reports the following errors in the wild (although there's > no stable repro): > > ================================================================== > BUG: KMSAN: use of unitialized memory in raw_send_hdrinc > net/ipv4/raw.c:407 [inline] > BUG: KMSAN: use of unitialized memory in raw_sendmsg+0x2c8b/0x3400 > net/ipv4/raw.c:640 > inter: 0 > CPU: 3 PID: 2853 Comm: syz-executor1 Not tainted 4.11.0-rc5+ #2445 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:16 [inline] > dump_stack+0x143/0x1b0 lib/dump_stack.c:52 > kmsan_report+0x16b/0x1e0 mm/kmsan/kmsan.c:1078 > __kmsan_warning_32+0x5c/0xa0 mm/kmsan/kmsan_instr.c:510 > raw_send_hdrinc net/ipv4/raw.c:407 [inline] > raw_sendmsg+0x2c8b/0x3400 net/ipv4/raw.c:640 > inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 > sock_sendmsg_nosec net/socket.c:633 [inline] > sock_sendmsg net/socket.c:643 [inline] > ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 > __sys_sendmsg net/socket.c:2031 [inline] > SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 > SyS_sendmsg+0x87/0xb0 net/socket.c:2038 > entry_SYSCALL_64_fastpath+0x13/0x94 > RIP: 0033:0x44a669 > RSP: 002b:00007f08d06edb58 EFLAGS: 00000286 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 0000000000708000 RCX: 000000000044a669 > RDX: 0000000000000000 RSI: 0000000020007fc8 RDI: 0000000000000017 > RBP: 0000000000005080 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000286 R12: 00000000006e4140 > R13: 0000000020b22000 R14: 0000000000000000 R15: 0000000000000000 > origin: 00000000cb200085 > save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59 > kmsan_save_stack_with_flags mm/kmsan/kmsan.c:362 [inline] > kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:257 > kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:270 > slab_alloc_node mm/slub.c:2735 [inline] > __kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4341 > __kmalloc_reserve net/core/skbuff.c:138 [inline] > __alloc_skb+0x2cd/0x740 net/core/skbuff.c:231 > alloc_skb include/linux/skbuff.h:933 [inline] > alloc_skb_with_frags+0x209/0xbc0 net/core/skbuff.c:4678 > sock_alloc_send_pskb+0x9ff/0xe00 net/core/sock.c:1903 > sock_alloc_send_skb+0xe4/0x100 net/core/sock.c:1920 > raw_send_hdrinc net/ipv4/raw.c:366 [inline] > raw_sendmsg+0x1db4/0x3400 net/ipv4/raw.c:640 > inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 > sock_sendmsg_nosec net/socket.c:633 [inline] > sock_sendmsg net/socket.c:643 [inline] > ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 > __sys_sendmsg net/socket.c:2031 [inline] > SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 > SyS_sendmsg+0x87/0xb0 net/socket.c:2038 > entry_SYSCALL_64_fastpath+0x13/0x94 > ================================================================== Ok, there was a subtle bug in KMSAN instrumentation that made the IPv4 case extremely hard to discover (http://bugs.llvm.org/show_bug.cgi?id=32842) After fixing it, the following syscalls: socket(PF_INET, SOCK_RAW, IPPROTO_RAW) = 3 sendto(3, NULL, 0, MSG_DONTWAIT, {sa_family=AF_INET, sin_port=htons(36895), sin_addr=inet_addr("10.0.0.0")}, 16) = -1 EINVAL (Invalid argument) reliably trigger the bug at line 404 here: 342 static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4, 343 struct msghdr *msg, size_t length, 344 struct rtable **rtp, unsigned int flags, 345 const struct sockcm_cookie *sockc) 346 { ... 391 if (memcpy_from_msg(iph, msg, length)) 392 goto error_free; 393 394 iphlen = iph->ihl * 4; 395 396 /* 397 * We don't want to modify the ip header, but we do need to 398 * be sure that it won't cause problems later along the network 399 * stack. Specifically we want to make sure that iph->ihl is a 400 * sane value. If ihl points beyond the length of the buffer passed 401 * in, reject the frame as invalid 402 */ 403 err = -EINVAL; 404 if (iphlen > length) 405 goto error_free; I'll send the updated patch next week. > >> >> >> >> -- >> Alexander Potapenko >> Software Engineer >> >> Google Germany GmbH >> Erika-Mann-Straße, 33 >> 80636 München >> >> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle >> Registergericht und -nummer: Hamburg, HRB 86891 >> Sitz der Gesellschaft: Hamburg > > > > -- > Alexander Potapenko > Software Engineer > > Google Germany GmbH > Erika-Mann-Straße, 33 > 80636 München > > Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg
From: Alexander Potapenko <glider@google.com> Date: Fri, 28 Apr 2017 16:28:12 +0200 > I'll send the updated patch next week. Great, thanks for doing all of this work.
================================================================== BUG: KMSAN: use of unitialized memory in nf_ct_frag6_gather+0xf5a/0x44a0 inter: 0 CPU: 0 PID: 1036 Comm: probe Not tainted 4.11.0-rc5+ #2455 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 dump_stack+0x143/0x1b0 lib/dump_stack.c:52 kmsan_report+0x16b/0x1e0 mm/kmsan/kmsan.c:1078 __kmsan_warning_32+0x5c/0xa0 mm/kmsan/kmsan_instr.c:510 nf_ct_frag6_gather+0xf5a/0x44a0 net/ipv6/netfilter/nf_conntrack_reasm.c:577 ipv6_defrag+0x1d9/0x280 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68 nf_hook_entry_hookfn ./include/linux/netfilter.h:102 nf_hook_slow+0x13f/0x3c0 net/netfilter/core.c:310 nf_hook ./include/linux/netfilter.h:212 NF_HOOK ./include/linux/netfilter.h:255 rawv6_send_hdrinc net/ipv6/raw.c:673 rawv6_sendmsg+0x2fcb/0x41a0 net/ipv6/raw.c:919 inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 SYSC_sendto+0x6a5/0x7c0 net/socket.c:1696 SyS_sendto+0xbc/0xe0 net/socket.c:1664 do_syscall_64+0x72/0xa0 arch/x86/entry/common.c:285 entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 RIP: 0033:0x436e03 RSP: 002b:00007ffce48baf38 EFLAGS: 00000246 ORIG_RAX: 000000000000002c RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000436e03 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003 RBP: 00007ffce48baf90 R08: 00007ffce48baf50 R09: 000000000000001c R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 0000000000401790 R14: 0000000000401820 R15: 0000000000000000 origin: 00000000d9400053 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:362 kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:257 kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:270 slab_alloc_node mm/slub.c:2735 __kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4341 __kmalloc_reserve net/core/skbuff.c:138 __alloc_skb+0x2cd/0x740 net/core/skbuff.c:231 alloc_skb ./include/linux/skbuff.h:933 alloc_skb_with_frags+0x209/0xbc0 net/core/skbuff.c:4678 sock_alloc_send_pskb+0x9ff/0xe00 net/core/sock.c:1903 sock_alloc_send_skb+0xe4/0x100 net/core/sock.c:1920 rawv6_send_hdrinc net/ipv6/raw.c:638 rawv6_sendmsg+0x2918/0x41a0 net/ipv6/raw.c:919 inet_sendmsg+0x3f8/0x6d0 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 SYSC_sendto+0x6a5/0x7c0 net/socket.c:1696 SyS_sendto+0xbc/0xe0 net/socket.c:1664 do_syscall_64+0x72/0xa0 arch/x86/entry/common.c:285 return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:246 ================================================================== , triggered by the following syscalls: socket(PF_INET6, SOCK_RAW, IPPROTO_RAW) = 3 sendto(3, NULL, 0, 0, {sa_family=AF_INET6, sin6_port=htons(0), inet_pton(AF_INET6, "ff00::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EPERM --- net/ipv6/raw.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c index f174e76e6505..805464668bd8 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -632,6 +632,8 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length, ipv6_local_error(sk, EMSGSIZE, fl6, rt->dst.dev->mtu); return -EMSGSIZE; } + if (length < sizeof(struct ipv6hdr)) + return -EINVAL; if (flags&MSG_PROBE) goto out;
rawv6_send_hdrinc() expects that the buffer copied from the userspace contains the IPv6 header, so if too few bytes are copied parts of the header may remain uninitialized. This bug has been detected with KMSAN. Signed-off-by: Alexander Potapenko <glider@google.com> --- For the record, the KMSAN report: