* net: GPF in eth_header @ 2016-11-26 17:30 Dmitry Vyukov 2016-11-26 18:28 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Dmitry Vyukov @ 2016-11-26 17:30 UTC (permalink / raw) To: David Miller, Tom Herbert, aduyck, Hannes Frederic Sowa, jbenc, Sabrina Dubroca, netdev, LKML, Eric Dumazet Cc: syzkaller Hello, The following program triggers GPF in eth_header: https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). BUG: unable to handle kernel paging request at ffffed002d14d74a IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 PGD 7fff6067 [ 50.787819] PUD 7fff5067 PMD 0 [ 50.787819] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN Modules linked in: CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 task: ffff88003a1841c0 task.stack: ffff880034d08000 RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 Stack: 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 Call Trace: [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 [< inline >] dst_neigh_output ./include/net/dst.h:464 [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 [< inline >] dst_output ./include/net/dst.h:501 [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 net/ipv6/ip6_output.c:1732 [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 [< inline >] sock_sendmsg_nosec net/socket.c:621 [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 [< inline >] SYSC_writev fs/read_write.c:1017 [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 arch/x86/entry/entry_64.S:209 Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 RSP <ffff880034d0eb68> CR2: ffffed002d14d74a ---[ end trace a73fedfdc11bd60c ]--- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 17:30 net: GPF in eth_header Dmitry Vyukov @ 2016-11-26 18:28 ` Eric Dumazet 2016-11-26 19:07 ` Andrey Konovalov 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-26 18:28 UTC (permalink / raw) To: Dmitry Vyukov Cc: David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML, syzkaller On Sat, Nov 26, 2016 at 9:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > Hello, > > The following program triggers GPF in eth_header: > > https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt > > On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). > > BUG: unable to handle kernel paging request at ffffed002d14d74a > IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 > PGD 7fff6067 [ 50.787819] PUD 7fff5067 > PMD 0 [ 50.787819] > Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN > Modules linked in: > CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > task: ffff88003a1841c0 task.stack: ffff880034d08000 > RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] > eth_header+0x75/0x260 net/ethernet/eth.c:88 > RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 > RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 > RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 > RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 > R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 > R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 > FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 > Stack: > 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 > 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 > ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 > Call Trace: > [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 > [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 > [< inline >] dst_neigh_output ./include/net/dst.h:464 > [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 > [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 > [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 > [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 > [< inline >] dst_output ./include/net/dst.h:501 > [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 > [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 > [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 > net/ipv6/ip6_output.c:1732 > [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 > [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 > [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 > [< inline >] sock_sendmsg_nosec net/socket.c:621 > [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 > [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 > [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 > [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 > [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 > [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 > [< inline >] SYSC_writev fs/read_write.c:1017 > [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 > [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 > arch/x86/entry/entry_64.S:209 > Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be > 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> > b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 > RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 > RSP <ffff880034d0eb68> > CR2: ffffed002d14d74a > ---[ end trace a73fedfdc11bd60c ]--- Hi Dmitry I could not reproduce the issue. Might need some specific configuration... loopback device has proper ethernet header (all 0) Fault happens in : 0f b6 0c 30 movzbl (%rax,%rsi,1),%ecx RAX=1ffff1002d14d74a which is RDI>>3, and RSI=dffffc0000000000 Could this be a KASAN problem ? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 18:28 ` Eric Dumazet @ 2016-11-26 19:07 ` Andrey Konovalov 2016-11-26 20:05 ` Eric Dumazet 2016-11-28 18:50 ` Eric Dumazet 0 siblings, 2 replies; 19+ messages in thread From: Andrey Konovalov @ 2016-11-26 19:07 UTC (permalink / raw) To: syzkaller Cc: Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML [-- Attachment #1: Type: text/plain, Size: 5343 bytes --] On Sat, Nov 26, 2016 at 7:28 PM, 'Eric Dumazet' via syzkaller <syzkaller@googlegroups.com> wrote: > On Sat, Nov 26, 2016 at 9:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote: >> Hello, >> >> The following program triggers GPF in eth_header: >> >> https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt >> >> On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). >> >> BUG: unable to handle kernel paging request at ffffed002d14d74a >> IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >> PGD 7fff6067 [ 50.787819] PUD 7fff5067 >> PMD 0 [ 50.787819] >> Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN >> Modules linked in: >> CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> task: ffff88003a1841c0 task.stack: ffff880034d08000 >> RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] >> eth_header+0x75/0x260 net/ethernet/eth.c:88 >> RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 >> RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 >> RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 >> RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 >> R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 >> R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 >> FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 >> Stack: >> 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 >> 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 >> ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 >> Call Trace: >> [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 >> [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 >> [< inline >] dst_neigh_output ./include/net/dst.h:464 >> [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 >> [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 >> [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 >> [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 >> [< inline >] dst_output ./include/net/dst.h:501 >> [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 >> [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 >> [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 >> net/ipv6/ip6_output.c:1732 >> [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 >> [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 >> [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 >> [< inline >] sock_sendmsg_nosec net/socket.c:621 >> [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 >> [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 >> [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 >> [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 >> [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 >> [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 >> [< inline >] SYSC_writev fs/read_write.c:1017 >> [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 >> [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 >> arch/x86/entry/entry_64.S:209 >> Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be >> 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> >> b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 >> RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >> RSP <ffff880034d0eb68> >> CR2: ffffed002d14d74a >> ---[ end trace a73fedfdc11bd60c ]--- > > > Hi Dmitry > > I could not reproduce the issue. Might need some specific configuration... > > loopback device has proper ethernet header (all 0) > > Fault happens in : > > 0f b6 0c 30 movzbl (%rax,%rsi,1),%ecx > > RAX=1ffff1002d14d74a which is RDI>>3, and RSI=dffffc0000000000 > > Could this be a KASAN problem ? Hi Eric, The crash happens when the kernel tries to access shadow for nonmapped memory. The issue here is an integer overflow which happens in neigh_resolve_output(). skb_network_offset(skb) can return negative number, but __skb_pull() accepts unsigned int as len. As a result, the least significat bit in higher 32 bits of skb->data gets set and we get an out-of-bounds with offset of 4 GB. I've attached a short reproducer, but you either need KASAN or to add a BUG_ON to see the crash. In this reproducer skb_network_offset() becomes negative after merging two ipv6 fragments. I actually see multiple places where skb_network_offset() is used as an argument to skb_pull(). So I guess every place can potentially be buggy. Thanks! > > -- > You received this message because you are subscribed to the Google Groups "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: ipv6-poc1.c --] [-- Type: application/octet-stream, Size: 1020 bytes --] #include <sys/types.h> #include <sys/socket.h> #include <netinet/in.h> #include <string.h> int main() { mmap(0x20000000ul, 0x28000ul, 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul); long sock = socket(0xaul, 0x3ul, 0x2cul); struct sockaddr_in6 addr; memset(&addr, 0, sizeof(addr)); addr.sin6_family = AF_INET6; addr.sin6_port = htons(0x4242); addr.sin6_addr = in6addr_loopback; // addr.sin6_addr.s6_addr[1] = 42; connect(sock, (const struct sockaddr *)&addr, sizeof(addr)); memcpy((void*)0x20025000, "\x06\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 72); sendto(sock, (const void *)0x20025000ul, 0x48ul, 0x4ul, 0x0ul, 0x0ul); sendto(sock, (const void *)0x20025000ul, 0x48ul, 0x4ul, 0x0ul, 0x0ul); return 0; } ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 19:07 ` Andrey Konovalov @ 2016-11-26 20:05 ` Eric Dumazet 2016-11-26 20:34 ` Eric Dumazet 2016-11-29 10:26 ` Andrey Konovalov 2016-11-28 18:50 ` Eric Dumazet 1 sibling, 2 replies; 19+ messages in thread From: Eric Dumazet @ 2016-11-26 20:05 UTC (permalink / raw) To: Andrey Konovalov Cc: syzkaller, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML > Hi Eric, > > The crash happens when the kernel tries to access shadow for nonmapped memory. > > The issue here is an integer overflow which happens in neigh_resolve_output(). > skb_network_offset(skb) can return negative number, but __skb_pull() > accepts unsigned int as len. > As a result, the least significat bit in higher 32 bits of skb->data > gets set and we get an out-of-bounds with offset of 4 GB. > > I've attached a short reproducer, but you either need KASAN or to add > a BUG_ON to see the crash. > In this reproducer skb_network_offset() becomes negative after merging > two ipv6 fragments. > > I actually see multiple places where skb_network_offset() is used as > an argument to skb_pull(). > So I guess every place can potentially be buggy. Well, I think the intent is to accept a negative number. This definitely was assumed by commit e1f165032c8bade authors ! I guess they were using a 32bit kernel for their tests. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 20:05 ` Eric Dumazet @ 2016-11-26 20:34 ` Eric Dumazet 2016-11-29 10:26 ` Andrey Konovalov 1 sibling, 0 replies; 19+ messages in thread From: Eric Dumazet @ 2016-11-26 20:34 UTC (permalink / raw) To: Andrey Konovalov Cc: syzkaller, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML 2016-11-26 12:05 GMT-08:00 Eric Dumazet <erdlkml@gmail.com>: >> Hi Eric, >> >> The crash happens when the kernel tries to access shadow for nonmapped memory. >> >> The issue here is an integer overflow which happens in neigh_resolve_output(). >> skb_network_offset(skb) can return negative number, but __skb_pull() >> accepts unsigned int as len. >> As a result, the least significat bit in higher 32 bits of skb->data >> gets set and we get an out-of-bounds with offset of 4 GB. >> >> I've attached a short reproducer, but you either need KASAN or to add >> a BUG_ON to see the crash. >> In this reproducer skb_network_offset() becomes negative after merging >> two ipv6 fragments. >> >> I actually see multiple places where skb_network_offset() is used as >> an argument to skb_pull(). >> So I guess every place can potentially be buggy. > > Well, I think the intent is to accept a negative number. > > This definitely was assumed by commit e1f165032c8bade authors ! > > I guess they were using a 32bit kernel for their tests. Correct fix would be to use skb_push(skb, -skb_network_offset(skb)); As done in other locations... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 20:05 ` Eric Dumazet 2016-11-26 20:34 ` Eric Dumazet @ 2016-11-29 10:26 ` Andrey Konovalov 2016-11-29 14:58 ` Eric Dumazet 1 sibling, 1 reply; 19+ messages in thread From: Andrey Konovalov @ 2016-11-29 10:26 UTC (permalink / raw) To: syzkaller Cc: Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML On Sat, Nov 26, 2016 at 9:05 PM, Eric Dumazet <erdlkml@gmail.com> wrote: >> I actually see multiple places where skb_network_offset() is used as >> an argument to skb_pull(). >> So I guess every place can potentially be buggy. > > Well, I think the intent is to accept a negative number. I'm not sure that was the intent since it results in a signedness issue which leads to an out-of-bounds. A quick grep shows that the same issue can potentially happen in multiple places across the kernel: net/ipv6/ip6_output.c:1655: __skb_pull(skb, skb_network_offset(skb)); net/packet/af_packet.c:2043: skb_pull(skb, skb_network_offset(skb)); net/packet/af_packet.c:2165: skb_pull(skb, skb_network_offset(skb)); net/core/neighbour.c:1301: __skb_pull(skb, skb_network_offset(skb)); net/core/neighbour.c:1331: __skb_pull(skb, skb_network_offset(skb)); net/core/dev.c:3157: __skb_pull(skb, skb_network_offset(skb)); net/sched/sch_teql.c:337: __skb_pull(skb, skb_network_offset(skb)); net/sched/sch_atm.c:479: skb_pull(skb, skb_network_offset(skb)); net/ipv4/ip_output.c:1385: __skb_pull(skb, skb_network_offset(skb)); net/ipv4/ip_fragment.c:391: if (!pskb_pull(skb, skb_network_offset(skb) + ihl)) drivers/net/vxlan.c:1440: __skb_pull(reply, skb_network_offset(reply)); drivers/net/vxlan.c:1902: __skb_pull(skb, skb_network_offset(skb)); drivers/net/vrf.c:220: __skb_pull(skb, skb_network_offset(skb)); drivers/net/vrf.c:314: __skb_pull(skb, skb_network_offset(skb)); A similar thing also happened to somebody else (on a receive path!): https://forums.grsecurity.net/viewtopic.php?f=3&t=4550 Does it make sense to check skb_network_offset() before passing it to skb_pull() everywhere? > > This definitely was assumed by commit e1f165032c8bade authors ! > > I guess they were using a 32bit kernel for their tests. > > -- > You received this message because you are subscribed to the Google Groups "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-29 10:26 ` Andrey Konovalov @ 2016-11-29 14:58 ` Eric Dumazet 2016-11-29 15:31 ` Andrey Konovalov 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-29 14:58 UTC (permalink / raw) To: Andrey Konovalov Cc: syzkaller, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML On Tue, 2016-11-29 at 11:26 +0100, Andrey Konovalov wrote: > On Sat, Nov 26, 2016 at 9:05 PM, Eric Dumazet <erdlkml@gmail.com> wrote: > >> I actually see multiple places where skb_network_offset() is used as > >> an argument to skb_pull(). > >> So I guess every place can potentially be buggy. > > > > Well, I think the intent is to accept a negative number. > > I'm not sure that was the intent since it results in a signedness > issue which leads to an out-of-bounds. > Hey, I already mentioned where was the bug. You missed the investigation where I pointed it to FLorian ? > A quick grep shows that the same issue can potentially happen in > multiple places across the kernel: > > net/ipv6/ip6_output.c:1655: __skb_pull(skb, skb_network_offset(skb)); > net/packet/af_packet.c:2043: skb_pull(skb, skb_network_offset(skb)); > net/packet/af_packet.c:2165: skb_pull(skb, skb_network_offset(skb)); > net/core/neighbour.c:1301: __skb_pull(skb, skb_network_offset(skb)); > net/core/neighbour.c:1331: __skb_pull(skb, skb_network_offset(skb)); > net/core/dev.c:3157: __skb_pull(skb, skb_network_offset(skb)); > net/sched/sch_teql.c:337: __skb_pull(skb, skb_network_offset(skb)); > net/sched/sch_atm.c:479: skb_pull(skb, skb_network_offset(skb)); > net/ipv4/ip_output.c:1385: __skb_pull(skb, skb_network_offset(skb)); > net/ipv4/ip_fragment.c:391: if (!pskb_pull(skb, skb_network_offset(skb) + ihl)) > drivers/net/vxlan.c:1440: __skb_pull(reply, skb_network_offset(reply)); > drivers/net/vxlan.c:1902: __skb_pull(skb, skb_network_offset(skb)); > drivers/net/vrf.c:220: __skb_pull(skb, skb_network_offset(skb)); > drivers/net/vrf.c:314: __skb_pull(skb, skb_network_offset(skb)); > > A similar thing also happened to somebody else (on a receive path!): > https://forums.grsecurity.net/viewtopic.php?f=3&t=4550 > > Does it make sense to check skb_network_offset() before passing it to > skb_pull() everywhere? Well, sure, we could add safety checks everywhere and slow the kernel when debugging is requested. But skb_network_offset() is not the problem here. Why are you focusing on it ? The real problem is in __skb_pull() or __skb_push() and all similar helpers. Lots of added checks and slowdowns. diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 9c535fbccf2c7dbfae04cee393460e86d588c26b..d6116e37d054fc1536114347ed3c41fc7dc7a882 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1923,6 +1923,7 @@ static inline unsigned char *__skb_put(struct sk_buff *skb, unsigned int len) unsigned char *skb_push(struct sk_buff *skb, unsigned int len); static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) { + BUG_ON((int)len < 0); skb->data -= len; skb->len += len; return skb->data; @@ -1931,6 +1932,7 @@ static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) unsigned char *skb_pull(struct sk_buff *skb, unsigned int len); static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len) { + BUG_ON((int)len < 0); skb->len -= len; BUG_ON(skb->len < skb->data_len); return skb->data += len; @@ -1938,6 +1940,7 @@ static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len) static inline unsigned char *skb_pull_inline(struct sk_buff *skb, unsigned int len) { + BUG_ON((int)len < 0); return unlikely(len > skb->len) ? NULL : __skb_pull(skb, len); } diff --git a/net/core/skbuff.c b/net/core/skbuff.c index d1d1a5a5ad24ded523fc12ffba8c602b03bd0830..7bf098c848fd857ba5d287fc91d43f62f381bd55 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1450,6 +1450,7 @@ EXPORT_SYMBOL(skb_put); */ unsigned char *skb_push(struct sk_buff *skb, unsigned int len) { + BUG_ON((int)len < 0); skb->data -= len; skb->len += len; if (unlikely(skb->data<skb->head)) ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-29 14:58 ` Eric Dumazet @ 2016-11-29 15:31 ` Andrey Konovalov 2016-11-29 16:15 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Andrey Konovalov @ 2016-11-29 15:31 UTC (permalink / raw) To: syzkaller Cc: Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML On Tue, Nov 29, 2016 at 3:58 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2016-11-29 at 11:26 +0100, Andrey Konovalov wrote: >> On Sat, Nov 26, 2016 at 9:05 PM, Eric Dumazet <erdlkml@gmail.com> wrote: >> >> I actually see multiple places where skb_network_offset() is used as >> >> an argument to skb_pull(). >> >> So I guess every place can potentially be buggy. >> > >> > Well, I think the intent is to accept a negative number. >> >> I'm not sure that was the intent since it results in a signedness >> issue which leads to an out-of-bounds. >> > > Hey, I already mentioned where was the bug. > > You missed the investigation where I pointed it to FLorian ? > >> A quick grep shows that the same issue can potentially happen in >> multiple places across the kernel: >> >> net/ipv6/ip6_output.c:1655: __skb_pull(skb, skb_network_offset(skb)); >> net/packet/af_packet.c:2043: skb_pull(skb, skb_network_offset(skb)); >> net/packet/af_packet.c:2165: skb_pull(skb, skb_network_offset(skb)); >> net/core/neighbour.c:1301: __skb_pull(skb, skb_network_offset(skb)); >> net/core/neighbour.c:1331: __skb_pull(skb, skb_network_offset(skb)); >> net/core/dev.c:3157: __skb_pull(skb, skb_network_offset(skb)); >> net/sched/sch_teql.c:337: __skb_pull(skb, skb_network_offset(skb)); >> net/sched/sch_atm.c:479: skb_pull(skb, skb_network_offset(skb)); >> net/ipv4/ip_output.c:1385: __skb_pull(skb, skb_network_offset(skb)); >> net/ipv4/ip_fragment.c:391: if (!pskb_pull(skb, skb_network_offset(skb) + ihl)) >> drivers/net/vxlan.c:1440: __skb_pull(reply, skb_network_offset(reply)); >> drivers/net/vxlan.c:1902: __skb_pull(skb, skb_network_offset(skb)); >> drivers/net/vrf.c:220: __skb_pull(skb, skb_network_offset(skb)); >> drivers/net/vrf.c:314: __skb_pull(skb, skb_network_offset(skb)); >> >> A similar thing also happened to somebody else (on a receive path!): >> https://forums.grsecurity.net/viewtopic.php?f=3&t=4550 >> >> Does it make sense to check skb_network_offset() before passing it to >> skb_pull() everywhere? > > Well, sure, we could add safety checks everywhere and slow the kernel > when debugging is requested. > > But skb_network_offset() is not the problem here. Why are you focusing > on it ? > > The real problem is in __skb_pull() or __skb_push() and all similar > helpers. Lots of added checks and slowdowns. The issue is not with skb_network_offset(), but with __skb_pull() using skb_network_offset() as an argument. I'm not sure what would be the beast way to fix this, to add a check before every __skb_pull(skb_network_offset()), to fix __skb_pull() to work with signed ints, to add BUG_ON()'s in __skb_pull, or something else. What I meant is that you fixed this very instance of the bug, and I'm pointing out that a similar one might hit us again. > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 9c535fbccf2c7dbfae04cee393460e86d588c26b..d6116e37d054fc1536114347ed3c41fc7dc7a882 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -1923,6 +1923,7 @@ static inline unsigned char *__skb_put(struct sk_buff *skb, unsigned int len) > unsigned char *skb_push(struct sk_buff *skb, unsigned int len); > static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) > { > + BUG_ON((int)len < 0); > skb->data -= len; > skb->len += len; > return skb->data; > @@ -1931,6 +1932,7 @@ static inline unsigned char *__skb_push(struct sk_buff *skb, unsigned int len) > unsigned char *skb_pull(struct sk_buff *skb, unsigned int len); > static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len) > { > + BUG_ON((int)len < 0); > skb->len -= len; > BUG_ON(skb->len < skb->data_len); > return skb->data += len; > @@ -1938,6 +1940,7 @@ static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len) > > static inline unsigned char *skb_pull_inline(struct sk_buff *skb, unsigned int len) > { > + BUG_ON((int)len < 0); > return unlikely(len > skb->len) ? NULL : __skb_pull(skb, len); > } > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index d1d1a5a5ad24ded523fc12ffba8c602b03bd0830..7bf098c848fd857ba5d287fc91d43f62f381bd55 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -1450,6 +1450,7 @@ EXPORT_SYMBOL(skb_put); > */ > unsigned char *skb_push(struct sk_buff *skb, unsigned int len) > { > + BUG_ON((int)len < 0); > skb->data -= len; > skb->len += len; > if (unlikely(skb->data<skb->head)) > > > > > > > > -- > You received this message because you are subscribed to the Google Groups "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-29 15:31 ` Andrey Konovalov @ 2016-11-29 16:15 ` Eric Dumazet 0 siblings, 0 replies; 19+ messages in thread From: Eric Dumazet @ 2016-11-29 16:15 UTC (permalink / raw) To: Andrey Konovalov Cc: syzkaller, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML On Tue, 2016-11-29 at 16:31 +0100, Andrey Konovalov wrote: = > The issue is not with skb_network_offset(), but with __skb_pull() > using skb_network_offset() as an argument. > No. The issue can happen with _any_ __skb_pull() with a 'negative' argument, on 64bit arches. skb_network_offset() is only one of the many cases this could happen if a bug is added at some random place, including memory corruption from a different kernel layer, or buggy hardware. > I'm not sure what would be the beast way to fix this, to add a check > before every __skb_pull(skb_network_offset()), to fix __skb_pull() to > work with signed ints, to add BUG_ON()'s in __skb_pull, or something > else. > > What I meant is that you fixed this very instance of the bug, and I'm > pointing out that a similar one might hit us again. As I said, adding a check in skb_network_offset() would not be generic enough. Sure, we can be proactive and add tests everywhere in the kernel, but we also want to keep it reasonably fast. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-26 19:07 ` Andrey Konovalov 2016-11-26 20:05 ` Eric Dumazet @ 2016-11-28 18:50 ` Eric Dumazet 2016-11-28 19:04 ` Andrey Konovalov 1 sibling, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 18:50 UTC (permalink / raw) To: Andrey Konovalov, Hannes Frederic Sowa Cc: syzkaller, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Hannes Frederic Sowa, Jiri Benc, Sabrina Dubroca, netdev, LKML On Sat, 2016-11-26 at 20:07 +0100, Andrey Konovalov wrote: > On Sat, Nov 26, 2016 at 7:28 PM, 'Eric Dumazet' via syzkaller > <syzkaller@googlegroups.com> wrote: > > On Sat, Nov 26, 2016 at 9:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > >> Hello, > >> > >> The following program triggers GPF in eth_header: > >> > >> https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt > >> > >> On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). > >> > >> BUG: unable to handle kernel paging request at ffffed002d14d74a > >> IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 > >> PGD 7fff6067 [ 50.787819] PUD 7fff5067 > >> PMD 0 [ 50.787819] > >> Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN > >> Modules linked in: > >> CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > >> task: ffff88003a1841c0 task.stack: ffff880034d08000 > >> RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] > >> eth_header+0x75/0x260 net/ethernet/eth.c:88 > >> RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 > >> RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 > >> RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 > >> RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 > >> R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 > >> R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 > >> FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 > >> Stack: > >> 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 > >> 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 > >> ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 > >> Call Trace: > >> [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 > >> [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 > >> [< inline >] dst_neigh_output ./include/net/dst.h:464 > >> [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 > >> [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 > >> [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 > >> [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 > >> [< inline >] dst_output ./include/net/dst.h:501 > >> [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 > >> [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 > >> [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 > >> net/ipv6/ip6_output.c:1732 > >> [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 > >> [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 > >> [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 > >> [< inline >] sock_sendmsg_nosec net/socket.c:621 > >> [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 > >> [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 > >> [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 > >> [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 > >> [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 > >> [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 > >> [< inline >] SYSC_writev fs/read_write.c:1017 > >> [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 > >> [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 > >> arch/x86/entry/entry_64.S:209 > >> Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be > >> 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> > >> b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 > >> RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 > >> RSP <ffff880034d0eb68> > >> CR2: ffffed002d14d74a > >> ---[ end trace a73fedfdc11bd60c ]--- > > > > > > Hi Dmitry > > > > I could not reproduce the issue. Might need some specific configuration... > > > > loopback device has proper ethernet header (all 0) > > > > Fault happens in : > > > > 0f b6 0c 30 movzbl (%rax,%rsi,1),%ecx > > > > RAX=1ffff1002d14d74a which is RDI>>3, and RSI=dffffc0000000000 > > > > Could this be a KASAN problem ? > > Hi Eric, > > The crash happens when the kernel tries to access shadow for nonmapped memory. > > The issue here is an integer overflow which happens in neigh_resolve_output(). > skb_network_offset(skb) can return negative number, but __skb_pull() > accepts unsigned int as len. > As a result, the least significat bit in higher 32 bits of skb->data > gets set and we get an out-of-bounds with offset of 4 GB. > > I've attached a short reproducer, but you either need KASAN or to add > a BUG_ON to see the crash. > In this reproducer skb_network_offset() becomes negative after merging > two ipv6 fragments. > > I actually see multiple places where skb_network_offset() is used as > an argument to skb_pull(). > So I guess every place can potentially be buggy. > > Thanks! I can not reproduce the bug on my hosts. Quite hard to debug for me. skb_network_offset() can not be negative at this point, unless there is a bug upper in the stack. Hannes, do you have an idea of what could be wrong in IPv6 stack ? Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 18:50 ` Eric Dumazet @ 2016-11-28 19:04 ` Andrey Konovalov 2016-11-28 19:34 ` Dmitry Vyukov 0 siblings, 1 reply; 19+ messages in thread From: Andrey Konovalov @ 2016-11-28 19:04 UTC (permalink / raw) To: syzkaller Cc: Hannes Frederic Sowa, Dmitry Vyukov, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, Nov 28, 2016 at 7:50 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Sat, 2016-11-26 at 20:07 +0100, Andrey Konovalov wrote: >> On Sat, Nov 26, 2016 at 7:28 PM, 'Eric Dumazet' via syzkaller >> <syzkaller@googlegroups.com> wrote: >> > On Sat, Nov 26, 2016 at 9:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote: >> >> Hello, >> >> >> >> The following program triggers GPF in eth_header: >> >> >> >> https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt >> >> >> >> On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). >> >> >> >> BUG: unable to handle kernel paging request at ffffed002d14d74a >> >> IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >> >> PGD 7fff6067 [ 50.787819] PUD 7fff5067 >> >> PMD 0 [ 50.787819] >> >> Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN >> >> Modules linked in: >> >> CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 >> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> >> task: ffff88003a1841c0 task.stack: ffff880034d08000 >> >> RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] >> >> eth_header+0x75/0x260 net/ethernet/eth.c:88 >> >> RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 >> >> RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 >> >> RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 >> >> RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 >> >> R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 >> >> R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 >> >> FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 >> >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> >> CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 >> >> Stack: >> >> 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 >> >> 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 >> >> ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 >> >> Call Trace: >> >> [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 >> >> [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 >> >> [< inline >] dst_neigh_output ./include/net/dst.h:464 >> >> [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 >> >> [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 >> >> [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 >> >> [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 >> >> [< inline >] dst_output ./include/net/dst.h:501 >> >> [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 >> >> [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 >> >> [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 >> >> net/ipv6/ip6_output.c:1732 >> >> [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 >> >> [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 >> >> [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 >> >> [< inline >] sock_sendmsg_nosec net/socket.c:621 >> >> [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 >> >> [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 >> >> [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 >> >> [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 >> >> [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 >> >> [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 >> >> [< inline >] SYSC_writev fs/read_write.c:1017 >> >> [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 >> >> [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 >> >> arch/x86/entry/entry_64.S:209 >> >> Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be >> >> 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> >> >> b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 >> >> RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >> >> RSP <ffff880034d0eb68> >> >> CR2: ffffed002d14d74a >> >> ---[ end trace a73fedfdc11bd60c ]--- >> > >> > >> > Hi Dmitry >> > >> > I could not reproduce the issue. Might need some specific configuration... >> > >> > loopback device has proper ethernet header (all 0) >> > >> > Fault happens in : >> > >> > 0f b6 0c 30 movzbl (%rax,%rsi,1),%ecx >> > >> > RAX=1ffff1002d14d74a which is RDI>>3, and RSI=dffffc0000000000 >> > >> > Could this be a KASAN problem ? >> >> Hi Eric, >> >> The crash happens when the kernel tries to access shadow for nonmapped memory. >> >> The issue here is an integer overflow which happens in neigh_resolve_output(). >> skb_network_offset(skb) can return negative number, but __skb_pull() >> accepts unsigned int as len. >> As a result, the least significat bit in higher 32 bits of skb->data >> gets set and we get an out-of-bounds with offset of 4 GB. >> >> I've attached a short reproducer, but you either need KASAN or to add >> a BUG_ON to see the crash. >> In this reproducer skb_network_offset() becomes negative after merging >> two ipv6 fragments. >> >> I actually see multiple places where skb_network_offset() is used as >> an argument to skb_pull(). >> So I guess every place can potentially be buggy. >> >> Thanks! > > I can not reproduce the bug on my hosts. > Quite hard to debug for me. > > skb_network_offset() can not be negative at this point, unless there is > a bug upper in the stack. Hi Eric, As far as I can see, skb_network_offset() becomes negative after pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data) in nf_ct_frag6_queue(). At least I'm able to detect that with a BUG_ON(). Also it seems that the issue is only reproducible (at least with the poc I provided) for a short time after boot. I hope that helps. > > Hannes, do you have an idea of what could be wrong in IPv6 stack ? > > Thanks. > > > -- > You received this message because you are subscribed to the Google Groups "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 19:04 ` Andrey Konovalov @ 2016-11-28 19:34 ` Dmitry Vyukov 2016-11-28 19:47 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Dmitry Vyukov @ 2016-11-28 19:34 UTC (permalink / raw) To: syzkaller Cc: Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, Nov 28, 2016 at 8:04 PM, 'Andrey Konovalov' via syzkaller <syzkaller@googlegroups.com> wrote: > On Mon, Nov 28, 2016 at 7:50 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> On Sat, 2016-11-26 at 20:07 +0100, Andrey Konovalov wrote: >>> On Sat, Nov 26, 2016 at 7:28 PM, 'Eric Dumazet' via syzkaller >>> <syzkaller@googlegroups.com> wrote: >>> > On Sat, Nov 26, 2016 at 9:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote: >>> >> Hello, >>> >> >>> >> The following program triggers GPF in eth_header: >>> >> >>> >> https://gist.githubusercontent.com/dvyukov/613cadf05543b55a419f237e419cd495/raw/5471231523d1a07c3de55f11f87472c2816ee06c/gistfile1.txt >>> >> >>> >> On commit 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24). >>> >> >>> >> BUG: unable to handle kernel paging request at ffffed002d14d74a >>> >> IP: [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >>> >> PGD 7fff6067 [ 50.787819] PUD 7fff5067 >>> >> PMD 0 [ 50.787819] >>> >> Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN >>> >> Modules linked in: >>> >> CPU: 2 PID: 6712 Comm: a.out Not tainted 4.9.0-rc6+ #55 >>> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >>> >> task: ffff88003a1841c0 task.stack: ffff880034d08000 >>> >> RIP: 0010:[<ffffffff86be3295>] [<ffffffff86be3295>] >>> >> eth_header+0x75/0x260 net/ethernet/eth.c:88 >>> >> RSP: 0018:ffff880034d0eb68 EFLAGS: 00010a03 >>> >> RAX: 1ffff1002d14d74a RBX: ffff880168a6ba4a RCX: ffff88006a9c7858 >>> >> RDX: 000000000000dd86 RSI: dffffc0000000000 RDI: ffff880168a6ba56 >>> >> RBP: ffff880034d0eb98 R08: 0000000000000000 R09: 0000000000000031 >>> >> R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000000 >>> >> R13: ffff88006c208d80 R14: 00000000000086dd R15: ffff88006a9c7858 >>> >> FS: 0000000001a02940(0000) GS:ffff88006d000000(0000) knlGS:0000000000000000 >>> >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> >> CR2: ffffed002d14d74a CR3: 0000000037373000 CR4: 00000000000006e0 >>> >> Stack: >>> >> 000000316881ab40 ffff88006a9c76c0 ffff88006881ab40 ffff88006a9c77f8 >>> >> 0000000000000000 dffffc0000000000 ffff880034d0ee98 ffffffff86b31af9 >>> >> ffffffff8719605c ffff880034d0f0f8 ffffffff000086dd ffffffff86be3220 >>> >> Call Trace: >>> >> [< inline >] dev_hard_header ./include/linux/netdevice.h:2762 >>> >> [<ffffffff86b31af9>] neigh_resolve_output+0x659/0xb20 net/core/neighbour.c:1302 >>> >> [< inline >] dst_neigh_output ./include/net/dst.h:464 >>> >> [<ffffffff8719605c>] ip6_finish_output2+0xb3c/0x2500 net/ipv6/ip6_output.c:121 >>> >> [<ffffffff871a0b0b>] ip6_finish_output+0x2eb/0x760 net/ipv6/ip6_output.c:139 >>> >> [< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246 >>> >> [<ffffffff871a1157>] ip6_output+0x1d7/0x9a0 net/ipv6/ip6_output.c:153 >>> >> [< inline >] dst_output ./include/net/dst.h:501 >>> >> [<ffffffff873312ea>] ip6_local_out+0x9a/0x180 net/ipv6/output_core.c:170 >>> >> [<ffffffff871a3886>] ip6_send_skb+0xa6/0x340 net/ipv6/ip6_output.c:1712 >>> >> [<ffffffff871a3bd8>] ip6_push_pending_frames+0xb8/0xe0 >>> >> net/ipv6/ip6_output.c:1732 >>> >> [< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607 >>> >> [<ffffffff8722acfb>] rawv6_sendmsg+0x250b/0x2c20 net/ipv6/raw.c:920 >>> >> [<ffffffff8701c4f5>] inet_sendmsg+0x385/0x590 net/ipv4/af_inet.c:734 >>> >> [< inline >] sock_sendmsg_nosec net/socket.c:621 >>> >> [<ffffffff86a6ea9f>] sock_sendmsg+0xcf/0x110 net/socket.c:631 >>> >> [<ffffffff86a6ee0b>] sock_write_iter+0x32b/0x620 net/socket.c:829 >>> >> [<ffffffff81a6f153>] do_iter_readv_writev+0x363/0x670 fs/read_write.c:695 >>> >> [<ffffffff81a71ba1>] do_readv_writev+0x431/0x9b0 fs/read_write.c:872 >>> >> [<ffffffff81a726dc>] vfs_writev+0x8c/0xc0 fs/read_write.c:911 >>> >> [<ffffffff81a72825>] do_writev+0x115/0x2d0 fs/read_write.c:944 >>> >> [< inline >] SYSC_writev fs/read_write.c:1017 >>> >> [<ffffffff81a75fdc>] SyS_writev+0x2c/0x40 fs/read_write.c:1014 >>> >> [<ffffffff8814cf85>] entry_SYSCALL_64_fastpath+0x23/0xc6 >>> >> arch/x86/entry/entry_64.S:209 >>> >> Code: 41 83 fe 04 0f 84 aa 00 00 00 e8 17 4e b0 fa 48 8d 7b 0c 48 be >>> >> 00 00 00 00 00 fc ff df 44 89 f2 66 c1 c2 08 48 89 f8 48 c1 e8 03 <0f> >>> >> b6 0c 30 48 8d 43 0d 49 89 c0 49 c1 e8 03 41 0f b6 34 30 49 >>> >> RIP [<ffffffff86be3295>] eth_header+0x75/0x260 net/ethernet/eth.c:88 >>> >> RSP <ffff880034d0eb68> >>> >> CR2: ffffed002d14d74a >>> >> ---[ end trace a73fedfdc11bd60c ]--- >>> > >>> > >>> > Hi Dmitry >>> > >>> > I could not reproduce the issue. Might need some specific configuration... >>> > >>> > loopback device has proper ethernet header (all 0) >>> > >>> > Fault happens in : >>> > >>> > 0f b6 0c 30 movzbl (%rax,%rsi,1),%ecx >>> > >>> > RAX=1ffff1002d14d74a which is RDI>>3, and RSI=dffffc0000000000 >>> > >>> > Could this be a KASAN problem ? >>> >>> Hi Eric, >>> >>> The crash happens when the kernel tries to access shadow for nonmapped memory. >>> >>> The issue here is an integer overflow which happens in neigh_resolve_output(). >>> skb_network_offset(skb) can return negative number, but __skb_pull() >>> accepts unsigned int as len. >>> As a result, the least significat bit in higher 32 bits of skb->data >>> gets set and we get an out-of-bounds with offset of 4 GB. >>> >>> I've attached a short reproducer, but you either need KASAN or to add >>> a BUG_ON to see the crash. >>> In this reproducer skb_network_offset() becomes negative after merging >>> two ipv6 fragments. >>> >>> I actually see multiple places where skb_network_offset() is used as >>> an argument to skb_pull(). >>> So I guess every place can potentially be buggy. >>> >>> Thanks! >> >> I can not reproduce the bug on my hosts. >> Quite hard to debug for me. >> >> skb_network_offset() can not be negative at this point, unless there is >> a bug upper in the stack. > > Hi Eric, > > As far as I can see, skb_network_offset() becomes negative after > pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data) in nf_ct_frag6_queue(). > At least I'm able to detect that with a BUG_ON(). > > Also it seems that the issue is only reproducible (at least with the > poc I provided) for a short time after boot. Eric, Is it enough to debug? Or maybe Andrey can trace some values for you. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 19:34 ` Dmitry Vyukov @ 2016-11-28 19:47 ` Eric Dumazet 2016-11-28 21:05 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 19:47 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, 2016-11-28 at 20:34 +0100, Dmitry Vyukov wrote: > On Mon, Nov 28, 2016 at 8:04 PM, 'Andrey Konovalov' via syzkaller > > Hi Eric, > > > > As far as I can see, skb_network_offset() becomes negative after > > pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data) in nf_ct_frag6_queue(). > > At least I'm able to detect that with a BUG_ON(). > > > > Also it seems that the issue is only reproducible (at least with the > > poc I provided) for a short time after boot. > > > Eric, > > Is it enough to debug? Or maybe Andrey can trace some values for you. Well, now we are talking, if you tell me how many modules you load, it might help ;) nf_ct_frag6_queue is nowhere to be seen in my kernels, that might explain why I could not reproduce the bug. Let me try ;) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 19:47 ` Eric Dumazet @ 2016-11-28 21:05 ` Eric Dumazet 2016-11-28 21:18 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 21:05 UTC (permalink / raw) To: Dmitry Vyukov, Florian Westphal Cc: syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, 2016-11-28 at 11:47 -0800, Eric Dumazet wrote: > On Mon, 2016-11-28 at 20:34 +0100, Dmitry Vyukov wrote: > > On Mon, Nov 28, 2016 at 8:04 PM, 'Andrey Konovalov' via syzkaller > > > > Hi Eric, > > > > > > As far as I can see, skb_network_offset() becomes negative after > > > pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data) in nf_ct_frag6_queue(). > > > At least I'm able to detect that with a BUG_ON(). > > > > > > Also it seems that the issue is only reproducible (at least with the > > > poc I provided) for a short time after boot. > > > > > > Eric, > > > > Is it enough to debug? Or maybe Andrey can trace some values for you. > > Well, now we are talking, if you tell me how many modules you load, it > might help ;) > > nf_ct_frag6_queue is nowhere to be seen in my kernels, that might > explain why I could not reproduce the bug. > > Let me try ;) > Might be a bug added in commit daaa7d647f81f3 ("netfilter: ipv6: avoid nf_iterate recursion") Florian, what do you think of dropping a packet that presumably was mangled badly by nf_ct_frag6_queue() ? (Like about 48 byte pulled :(, and/or skb->csum changed ) diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c index f7aab5ab93a5..31aa947332d8 100644 --- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c +++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c @@ -65,8 +65,8 @@ static unsigned int ipv6_defrag(void *priv, err = nf_ct_frag6_gather(state->net, skb, nf_ct6_defrag_user(state->hook, skb)); - /* queued */ - if (err == -EINPROGRESS) + /* queued or mangled ... */ + if (err) return NF_STOLEN; return NF_ACCEPT; ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 21:05 ` Eric Dumazet @ 2016-11-28 21:18 ` Eric Dumazet 2016-11-28 21:34 ` Florian Westphal 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 21:18 UTC (permalink / raw) To: Dmitry Vyukov Cc: Florian Westphal, syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, 2016-11-28 at 13:05 -0800, Eric Dumazet wrote: > On Mon, 2016-11-28 at 11:47 -0800, Eric Dumazet wrote: > > On Mon, 2016-11-28 at 20:34 +0100, Dmitry Vyukov wrote: > > > On Mon, Nov 28, 2016 at 8:04 PM, 'Andrey Konovalov' via syzkaller > > > > > > Hi Eric, > > > > > > > > As far as I can see, skb_network_offset() becomes negative after > > > > pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data) in nf_ct_frag6_queue(). > > > > At least I'm able to detect that with a BUG_ON(). > > > > > > > > Also it seems that the issue is only reproducible (at least with the > > > > poc I provided) for a short time after boot. > > > > > > > > > Eric, > > > > > > Is it enough to debug? Or maybe Andrey can trace some values for you. > > > > Well, now we are talking, if you tell me how many modules you load, it > > might help ;) > > > > nf_ct_frag6_queue is nowhere to be seen in my kernels, that might > > explain why I could not reproduce the bug. > > > > Let me try ;) > > > > Might be a bug added in commit daaa7d647f81f3 > ("netfilter: ipv6: avoid nf_iterate recursion") > > Florian, what do you think of dropping a packet that presumably was > mangled badly by nf_ct_frag6_queue() ? > > (Like about 48 byte pulled :(, and/or skb->csum changed ) > > diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > index f7aab5ab93a5..31aa947332d8 100644 > --- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > +++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > @@ -65,8 +65,8 @@ static unsigned int ipv6_defrag(void *priv, > > err = nf_ct_frag6_gather(state->net, skb, > nf_ct6_defrag_user(state->hook, skb)); > - /* queued */ > - if (err == -EINPROGRESS) > + /* queued or mangled ... */ > + if (err) > return NF_STOLEN; Or more exactly , returning NF_DROP so that skb is really freed ;) diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c index f7aab5ab93a5..508739a3ca2a 100644 --- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c +++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c @@ -65,9 +65,9 @@ static unsigned int ipv6_defrag(void *priv, err = nf_ct_frag6_gather(state->net, skb, nf_ct6_defrag_user(state->hook, skb)); - /* queued */ - if (err == -EINPROGRESS) - return NF_STOLEN; + /* queued or mangled ... */ + if (err) + return (err == -EINPROGRESS) ? NF_STOLEN : NF_DROP; return NF_ACCEPT; } ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 21:18 ` Eric Dumazet @ 2016-11-28 21:34 ` Florian Westphal 2016-11-28 22:14 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Florian Westphal @ 2016-11-28 21:34 UTC (permalink / raw) To: Eric Dumazet Cc: Dmitry Vyukov, Florian Westphal, syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML Eric Dumazet <eric.dumazet@gmail.com> wrote: > > Might be a bug added in commit daaa7d647f81f3 > > ("netfilter: ipv6: avoid nf_iterate recursion") > > > > Florian, what do you think of dropping a packet that presumably was > > mangled badly by nf_ct_frag6_queue() ? ipv4 definitely frees malformed packets. In general, I think netfilter should avoid 'silent' drops if possible and let skb continue, but of course such skbs should not be made worse as what we ate to begin with... > > (Like about 48 byte pulled :(, and/or skb->csum changed ) I think this warrants a review of ipv6 reassembly too, bug reported here is because ipv6 nf defrag is also done on output. > diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > index f7aab5ab93a5..508739a3ca2a 100644 > --- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > +++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > @@ -65,9 +65,9 @@ static unsigned int ipv6_defrag(void *priv, > > err = nf_ct_frag6_gather(state->net, skb, > nf_ct6_defrag_user(state->hook, skb)); > - /* queued */ > - if (err == -EINPROGRESS) > - return NF_STOLEN; > + /* queued or mangled ... */ > + if (err) > + return (err == -EINPROGRESS) ? NF_STOLEN : NF_DROP; > > return NF_ACCEPT; Looks good, we'll need to change some of the errno return codes in nf_ct_frag6_gather to 0 though for this to work, which should not be too hard ;) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 21:34 ` Florian Westphal @ 2016-11-28 22:14 ` Eric Dumazet 2016-11-28 22:19 ` Florian Westphal 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 22:14 UTC (permalink / raw) To: Florian Westphal Cc: Dmitry Vyukov, syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, 2016-11-28 at 22:34 +0100, Florian Westphal wrote: > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > Might be a bug added in commit daaa7d647f81f3 > > > ("netfilter: ipv6: avoid nf_iterate recursion") > > > > > > Florian, what do you think of dropping a packet that presumably was > > > mangled badly by nf_ct_frag6_queue() ? > > ipv4 definitely frees malformed packets. > In general, I think netfilter should avoid 'silent' drops if possible > and let skb continue, but of course such skbs should not be made worse > as what we ate to begin with... > > > > (Like about 48 byte pulled :(, and/or skb->csum changed ) > > I think this warrants a review of ipv6 reassembly too, bug reported here > is because ipv6 nf defrag is also done on output. ip6_frag_queue() definitely frees bad/mangled skbs() > Looks good, we'll need to change some of the errno return codes in > nf_ct_frag6_gather to 0 though for this to work, which should not be too > hard ;) If the goal is to let buggy packets pass, then we might need to undo changes in nf_ct_frag6_queue() ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 22:14 ` Eric Dumazet @ 2016-11-28 22:19 ` Florian Westphal 2016-11-28 23:16 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Florian Westphal @ 2016-11-28 22:19 UTC (permalink / raw) To: Eric Dumazet Cc: Florian Westphal, Dmitry Vyukov, syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Mon, 2016-11-28 at 22:34 +0100, Florian Westphal wrote: > > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > Might be a bug added in commit daaa7d647f81f3 > > > > ("netfilter: ipv6: avoid nf_iterate recursion") > > > > > > > > Florian, what do you think of dropping a packet that presumably was > > > > mangled badly by nf_ct_frag6_queue() ? > > > > ipv4 definitely frees malformed packets. > > In general, I think netfilter should avoid 'silent' drops if possible > > and let skb continue, but of course such skbs should not be made worse > > as what we ate to begin with... > > > > > > (Like about 48 byte pulled :(, and/or skb->csum changed ) > > > > I think this warrants a review of ipv6 reassembly too, bug reported here > > is because ipv6 nf defrag is also done on output. > > > ip6_frag_queue() definitely frees bad/mangled skbs() Yes, sorry. nf_ct_frag6_queue is mostly derived from ip6_frag_queue so any bugs in one might also exist in other. Thats all I wanted to say here. I'll check this tomorrow. > > Looks good, we'll need to change some of the errno return codes in > > nf_ct_frag6_gather to 0 though for this to work, which should not be too > > hard ;) > > If the goal is to let buggy packets pass, then we might need to undo > changes in nf_ct_frag6_queue() It currently returns -EINVAL in cases where skb wasn't changed/altered (e.g. because it doesn't have a fragment header), so we should ACCEPT in that case. As for 'buggy' packet, I think its ok to mimic ip6_frag_queue, i.e. if it tosses returning NF_DROP under same circumstance seems ok. (Passing however will -- on ingress side -- cause snmp stat increments in ipv6 reassembly, this still might be desireable). I'll check where undo might be possible/not too hard. Thanks Eric for debugging this! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: net: GPF in eth_header 2016-11-28 22:19 ` Florian Westphal @ 2016-11-28 23:16 ` Eric Dumazet 0 siblings, 0 replies; 19+ messages in thread From: Eric Dumazet @ 2016-11-28 23:16 UTC (permalink / raw) To: Florian Westphal Cc: Dmitry Vyukov, syzkaller, Hannes Frederic Sowa, David Miller, Tom Herbert, Alexander Duyck, Jiri Benc, Sabrina Dubroca, netdev, LKML On Mon, 2016-11-28 at 23:19 +0100, Florian Westphal wrote: > It currently returns -EINVAL in cases where skb wasn't changed/altered > (e.g. because it doesn't have a fragment header), so we should ACCEPT in > that case. > Maybe nf_ct_frag6_queue() should return direct NF_ codes then ... ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-11-29 16:18 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-26 17:30 net: GPF in eth_header Dmitry Vyukov 2016-11-26 18:28 ` Eric Dumazet 2016-11-26 19:07 ` Andrey Konovalov 2016-11-26 20:05 ` Eric Dumazet 2016-11-26 20:34 ` Eric Dumazet 2016-11-29 10:26 ` Andrey Konovalov 2016-11-29 14:58 ` Eric Dumazet 2016-11-29 15:31 ` Andrey Konovalov 2016-11-29 16:15 ` Eric Dumazet 2016-11-28 18:50 ` Eric Dumazet 2016-11-28 19:04 ` Andrey Konovalov 2016-11-28 19:34 ` Dmitry Vyukov 2016-11-28 19:47 ` Eric Dumazet 2016-11-28 21:05 ` Eric Dumazet 2016-11-28 21:18 ` Eric Dumazet 2016-11-28 21:34 ` Florian Westphal 2016-11-28 22:14 ` Eric Dumazet 2016-11-28 22:19 ` Florian Westphal 2016-11-28 23:16 ` Eric Dumazet
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).