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 <[email protected]>
---
For the record, the KMSAN report:
==================================================================
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;
--
2.13.0.rc0.306.g87b477812d-goog
From: Alexander Potapenko <[email protected]>
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 <[email protected]>
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 <[email protected]> wrote:
> From: Alexander Potapenko <[email protected]>
> 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 <[email protected]>
>
> 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 <[email protected]> wrote:
> From: Alexander Potapenko <[email protected]>
> 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 <[email protected]>
>
> 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.
--
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 10:54 AM, Alexander Potapenko <[email protected]> wrote:
> On Tue, Apr 25, 2017 at 7:55 PM, David Miller <[email protected]> wrote:
>> From: Alexander Potapenko <[email protected]>
>> 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 <[email protected]>
>>
>> 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
--
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 <[email protected]>
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.
On Wed, Apr 26, 2017 at 3:00 PM, Alexander Potapenko <[email protected]> wrote:
> On Wed, Apr 26, 2017 at 10:54 AM, Alexander Potapenko <[email protected]> wrote:
>> On Tue, Apr 25, 2017 at 7:55 PM, David Miller <[email protected]> wrote:
>>> From: Alexander Potapenko <[email protected]>
>>> 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 <[email protected]>
>>>
>>> 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
--
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