linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
@ 2017-04-25 13:18 Alexander Potapenko
  2017-04-25 17:55 ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Potapenko @ 2017-04-25 13:18 UTC (permalink / raw)
  To: dvyukov, kcc, edumazet, davem, kuznet; +Cc: linux-kernel, netdev

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:

==================================================================
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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-25 13:18 [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr) Alexander Potapenko
@ 2017-04-25 17:55 ` David Miller
  2017-04-25 18:10   ` Eric Dumazet
  2017-04-26  8:54   ` Alexander Potapenko
  0 siblings, 2 replies; 7+ messages in thread
From: David Miller @ 2017-04-25 17:55 UTC (permalink / raw)
  To: glider; +Cc: dvyukov, kcc, edumazet, kuznet, linux-kernel, netdev

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-25 17:55 ` David Miller
@ 2017-04-25 18:10   ` Eric Dumazet
  2017-04-26  8:54   ` Alexander Potapenko
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2017-04-25 18:10 UTC (permalink / raw)
  To: David Miller
  Cc: Alexander Potapenko, Dmitry Vyukov, Kostya Serebryany,
	Alexey Kuznetsov, LKML, netdev

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()

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-25 17:55 ` David Miller
  2017-04-25 18:10   ` Eric Dumazet
@ 2017-04-26  8:54   ` Alexander Potapenko
  2017-04-26 13:00     ` Alexander Potapenko
  1 sibling, 1 reply; 7+ messages in thread
From: Alexander Potapenko @ 2017-04-26  8:54 UTC (permalink / raw)
  To: David Miller
  Cc: Dmitriy Vyukov, Kostya Serebryany, Eric Dumazet,
	Alexey Kuznetsov, LKML, Networking

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.



-- 
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-26  8:54   ` Alexander Potapenko
@ 2017-04-26 13:00     ` Alexander Potapenko
  2017-04-28 14:28       ` Alexander Potapenko
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Potapenko @ 2017-04-26 13:00 UTC (permalink / raw)
  To: David Miller
  Cc: Dmitriy Vyukov, Kostya Serebryany, Eric Dumazet,
	Alexey Kuznetsov, LKML, Networking

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



-- 
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-26 13:00     ` Alexander Potapenko
@ 2017-04-28 14:28       ` Alexander Potapenko
  2017-04-28 14:32         ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Potapenko @ 2017-04-28 14:28 UTC (permalink / raw)
  To: David Miller
  Cc: Dmitriy Vyukov, Kostya Serebryany, Eric Dumazet,
	Alexey Kuznetsov, LKML, Networking

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



-- 
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr)
  2017-04-28 14:28       ` Alexander Potapenko
@ 2017-04-28 14:32         ` David Miller
  0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2017-04-28 14:32 UTC (permalink / raw)
  To: glider; +Cc: dvyukov, kcc, edumazet, kuznet, linux-kernel, netdev

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-28 14:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-25 13:18 [PATCH] ipv6: ensure message length for raw socket is at least sizeof(ipv6hdr) Alexander Potapenko
2017-04-25 17:55 ` David Miller
2017-04-25 18:10   ` Eric Dumazet
2017-04-26  8:54   ` Alexander Potapenko
2017-04-26 13:00     ` Alexander Potapenko
2017-04-28 14:28       ` Alexander Potapenko
2017-04-28 14:32         ` 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).