linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
@ 2018-06-18 13:31 syzbot
  2018-07-06 17:52 ` syzbot
  0 siblings, 1 reply; 7+ messages in thread
From: syzbot @ 2018-06-18 13:31 UTC (permalink / raw)
  To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji

Hello,

syzbot found the following crash on:

HEAD commit:    f0dc7f9c6dd9 Merge git://git.kernel.org/pub/scm/linux/kern..
git tree:       net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1463121f800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=fa9c20c48788d1c1
dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x53e/0x5d0  
net/ipv6/ip6_offload.c:45
Read of size 1 at addr ffff8801cf0d7769 by task syz-executor0/21808

CPU: 1 PID: 21808 Comm: syz-executor0 Not tainted 4.17.0+ #84
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
  ipv6_gso_pull_exthdrs+0x53e/0x5d0 net/ipv6/ip6_offload.c:45
netlink: 48 bytes leftover after parsing attributes in process  
`syz-executor6'.
  ipv6_gso_segment+0x372/0x11c0 net/ipv6/ip6_offload.c:87
  skb_mac_gso_segment+0x3ad/0x720 net/core/dev.c:2792
  nsh_gso_segment+0x470/0xb40 net/nsh/nsh.c:111
  skb_mac_gso_segment+0x3ad/0x720 net/core/dev.c:2792
  __skb_gso_segment+0x3bb/0x870 net/core/dev.c:2865
  skb_gso_segment include/linux/netdevice.h:4079 [inline]
  validate_xmit_skb+0x638/0xf20 net/core/dev.c:3104
  __dev_queue_xmit+0xc0c/0x3900 net/core/dev.c:3561
  dev_queue_xmit+0x17/0x20 net/core/dev.c:3602
  packet_snd net/packet/af_packet.c:2921 [inline]
  packet_sendmsg+0x4275/0x6100 net/packet/af_packet.c:2946
  sock_sendmsg_nosec net/socket.c:645 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:655
  __sys_sendto+0x3d7/0x670 net/socket.c:1833
  __do_sys_sendto net/socket.c:1845 [inline]
  __se_sys_sendto net/socket.c:1841 [inline]
  __x64_sys_sendto+0xe1/0x1a0 net/socket.c:1841
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455b29
Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f1665cd0c68 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 00007f1665cd16d4 RCX: 0000000000455b29
RDX: 000000000000020b RSI: 00000000200016c0 RDI: 0000000000000013
RBP: 000000000072bea0 R08: 00000000200000c0 R09: 000000000000001c
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004c0eb8 R14: 00000000004d0960 R15: 0000000000000000

Allocated by task 21219:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
  kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
  getname_flags+0xd0/0x5a0 fs/namei.c:140
  user_path_at_empty+0x2d/0x50 fs/namei.c:2555
  user_path_at include/linux/namei.h:57 [inline]
  do_faccessat+0x24a/0x7c0 fs/open.c:389
  __do_sys_access fs/open.c:441 [inline]
  __se_sys_access fs/open.c:439 [inline]
  __x64_sys_access+0x59/0x80 fs/open.c:439
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 21219:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
  putname+0xf2/0x130 fs/namei.c:261
  filename_lookup+0x38b/0x4f0 fs/namei.c:2330
  user_path_at_empty+0x40/0x50 fs/namei.c:2555
  user_path_at include/linux/namei.h:57 [inline]
  do_faccessat+0x24a/0x7c0 fs/open.c:389
  __do_sys_access fs/open.c:441 [inline]
  __se_sys_access fs/open.c:439 [inline]
  __x64_sys_access+0x59/0x80 fs/open.c:439
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8801cf0d6d00
  which belongs to the cache names_cache of size 4096
The buggy address is located 2665 bytes inside of
  4096-byte region [ffff8801cf0d6d00, ffff8801cf0d7d00)
The buggy address belongs to the page:
page:ffffea00073c3580 count:1 mapcount:0 mapping:ffff8801da986dc0 index:0x0  
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea00073c3508 ffffea0006a10008 ffff8801da986dc0
raw: 0000000000000000 ffff8801cf0d6d00 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801cf0d7600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8801cf0d7680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801cf0d7700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                           ^
  ffff8801cf0d7780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8801cf0d7800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-06-18 13:31 KASAN: use-after-free Read in ipv6_gso_pull_exthdrs syzbot
@ 2018-07-06 17:52 ` syzbot
  2018-07-06 22:16   ` Willem de Bruijn
  0 siblings, 1 reply; 7+ messages in thread
From: syzbot @ 2018-07-06 17:52 UTC (permalink / raw)
  To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji

syzbot has found a reproducer for the following crash on:

HEAD commit:    70ba5b6db96f ipv4: Return EINVAL when ping_group_range sys..
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=13cd2970400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15dfb748400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12a1050c400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x57a/0x5f0  
net/ipv6/ip6_offload.c:45
Read of size 1 at addr ffff8801ce17f3a9 by task syz-executor655/4567

CPU: 1 PID: 4567 Comm: syz-executor655 Not tainted 4.18.0-rc3+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
  ipv6_gso_pull_exthdrs+0x57a/0x5f0 net/ipv6/ip6_offload.c:45
  ipv6_gso_segment+0x37a/0x11e0 net/ipv6/ip6_offload.c:87
  skb_mac_gso_segment+0x3b5/0x740 net/core/dev.c:2792
  nsh_gso_segment+0x470/0xb40 net/nsh/nsh.c:111
  skb_mac_gso_segment+0x3b5/0x740 net/core/dev.c:2792
  __skb_gso_segment+0x3c3/0x880 net/core/dev.c:2865
  skb_gso_segment include/linux/netdevice.h:4099 [inline]
  validate_xmit_skb+0x640/0xf30 net/core/dev.c:3104
  __dev_queue_xmit+0xc14/0x3910 net/core/dev.c:3561
  dev_queue_xmit+0x17/0x20 net/core/dev.c:3602
  packet_snd net/packet/af_packet.c:2919 [inline]
  packet_sendmsg+0x428e/0x6130 net/packet/af_packet.c:2944
  sock_sendmsg_nosec net/socket.c:641 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:651
  __sys_sendto+0x3d7/0x670 net/socket.c:1797
  __do_sys_sendto net/socket.c:1809 [inline]
  __se_sys_sendto net/socket.c:1805 [inline]
  __x64_sys_sendto+0xe1/0x1a0 net/socket.c:1805
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x441de9
Code: 25 02 00 85 c0 b8 00 00 00 00 48 0f 44 c3 5b c3 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 8b 0d fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00000000007dfe28 EFLAGS: 00000212 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000000021 RCX: 0000000000441de9
RDX: 0000000000000012 RSI: 0000000020000000 RDI: 0000000000000003
RBP: 00000000004a3f10 R08: 00000000200000c0 R09: 000000000000001c
R10: 0000000000000000 R11: 0000000000000212 R12: 00000000007dff68
R13: 0000000000403090 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 3516:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
  kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
  kmem_cache_zalloc include/linux/slab.h:697 [inline]
  get_empty_filp+0x12d/0x530 fs/file_table.c:122
  path_openat+0x11e/0x4e10 fs/namei.c:3516
  do_filp_open+0x255/0x380 fs/namei.c:3574
  do_sys_open+0x584/0x760 fs/open.c:1101
  __do_sys_open fs/open.c:1119 [inline]
  __se_sys_open fs/open.c:1114 [inline]
  __x64_sys_open+0x7e/0xc0 fs/open.c:1114
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 3519:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
  file_free_rcu+0x6f/0x90 fs/file_table.c:49
  __rcu_reclaim kernel/rcu/rcu.h:178 [inline]
  rcu_do_batch kernel/rcu/tree.c:2558 [inline]
  invoke_rcu_callbacks kernel/rcu/tree.c:2818 [inline]
  __rcu_process_callbacks kernel/rcu/tree.c:2785 [inline]
  rcu_process_callbacks+0xed5/0x1850 kernel/rcu/tree.c:2802
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:288

The buggy address belongs to the object at ffff8801ce17f280
  which belongs to the cache filp of size 456
The buggy address is located 297 bytes inside of
  456-byte region [ffff8801ce17f280, ffff8801ce17f448)
The buggy address belongs to the page:
page:ffffea0007385fc0 count:1 mapcount:0 mapping:ffff8801da987940  
index:0xffff8801ce17f780
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0007368048 ffffea0007368148 ffff8801da987940
raw: ffff8801ce17f780 ffff8801ce17f000 0000000100000005 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801ce17f280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8801ce17f300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801ce17f380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                   ^
  ffff8801ce17f400: fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc
  ffff8801ce17f480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-07-06 17:52 ` syzbot
@ 2018-07-06 22:16   ` Willem de Bruijn
  2018-07-08 22:58     ` Willem de Bruijn
  0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2018-07-06 22:16 UTC (permalink / raw)
  To: syzbot+7b9ed9872dab8c32305d
  Cc: David Miller, Alexey Kuznetsov, LKML, Network Development,
	syzkaller-bugs, Hideaki YOSHIFUJI

On Fri, Jul 6, 2018 at 1:55 PM syzbot
<syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com> wrote:
>
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit:    70ba5b6db96f ipv4: Return EINVAL when ping_group_range sys..
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=13cd2970400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15dfb748400000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12a1050c400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x57a/0x5f0
> net/ipv6/ip6_offload.c:45
> Read of size 1 at addr ffff8801ce17f3a9 by task syz-executor655/4567

This is an 8 byte packet with a virtio_net_hdr describing it as
VIRTIO_NET_HDR_GSO_TCPV4, but with protocol ETH_P_NSH. That
matches the occurrence of nsh_gso_segment in the stack trace.

This pulls the struct nshhdr of 8B, passing a packet with skb->len 0
to skb_mac_gso_segment. That is the only GRO function that
unconditionally calls _skb_pull without first checking pskb_may_pull.
Adding that check does catch this:

+       if (unlikely(!pskb_may_pull(skb, vlan_depth)))
+               return ERR_PTR(-EINVAL);

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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-07-06 22:16   ` Willem de Bruijn
@ 2018-07-08 22:58     ` Willem de Bruijn
  2018-07-08 23:18       ` Willem de Bruijn
  2018-07-11 10:07       ` Jiri Benc
  0 siblings, 2 replies; 7+ messages in thread
From: Willem de Bruijn @ 2018-07-08 22:58 UTC (permalink / raw)
  To: syzbot+7b9ed9872dab8c32305d
  Cc: David Miller, Alexey Kuznetsov, LKML, Network Development,
	syzkaller-bugs, Hideaki YOSHIFUJI, Jiri Benc

On Fri, Jul 6, 2018 at 6:16 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Fri, Jul 6, 2018 at 1:55 PM syzbot
> <syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com> wrote:
> >
> > syzbot has found a reproducer for the following crash on:
> >
> > HEAD commit:    70ba5b6db96f ipv4: Return EINVAL when ping_group_range sys..
> > git tree:       net
> > console output: https://syzkaller.appspot.com/x/log.txt?x=13cd2970400000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> > dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15dfb748400000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12a1050c400000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com
> >
> > IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
> > IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> > IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> > 8021q: adding VLAN 0 to HW filter on device team0
> > ==================================================================
> > BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x57a/0x5f0
> > net/ipv6/ip6_offload.c:45
> > Read of size 1 at addr ffff8801ce17f3a9 by task syz-executor655/4567
>
> This is an 8 byte packet with a virtio_net_hdr describing it as
> VIRTIO_NET_HDR_GSO_TCPV4, but with protocol ETH_P_NSH. That
> matches the occurrence of nsh_gso_segment in the stack trace.
>
> This pulls the struct nshhdr of 8B, passing a packet with skb->len 0
> to skb_mac_gso_segment. That is the only GRO function that
> unconditionally calls _skb_pull without first checking pskb_may_pull.
> Adding that check does catch this:
>
> +       if (unlikely(!pskb_may_pull(skb, vlan_depth)))
> +               return ERR_PTR(-EINVAL);

vlan_depth is 65528 when entering skb_mac_gso_segment due to
overflow of skb->mac_len in nsh_gso_segment. For this packet, the
outer mac len is zero, so skb->data == skb_mac_header(skb). Then

    skb_reset_network_header(skb);
    [...]
    __skb_pull(skb, nsh_len);
    skb_reset_mac_header(skb);    // now mac hdr starts nsh_len == 8B
after net hdr
    skb_reset_mac_len(skb);          // mac len = net hdr - mac hdr ==
(u16) -8 == 65528
    [..]
    skb_mac_gso_segment(skb, ..)

Setting skb->mac_len to 0, similar to mpls_gs_segment,
is sufficient if the encapsulated packet is not ETH_P_TEB.

If the packet is encapsulated at L2, __skb_pull(skb, vlan_depth)
has to pull the inner mac header before passing to l3 handlers like
inet_gso_segment.

If that header includes VLAN tags, skb_network_protocol will
parse then and update the mac length in vlan_depth. So
hardcoding to ETH_HLEN should be fine:

@@ -104,7 +95,7 @@ static struct sk_buff *nsh_gso_segment(struct sk_buff *skb,
        __skb_pull(skb, nsh_len);

        skb_reset_mac_header(skb);
-       skb_reset_mac_len(skb);
+       skb->mac_len = proto == ETH_P_TEB ? ETH_HLEN : 0;
        skb->protocol = proto;

        features &= NETIF_F_SG;

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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-07-08 22:58     ` Willem de Bruijn
@ 2018-07-08 23:18       ` Willem de Bruijn
  2018-07-11 10:07       ` Jiri Benc
  1 sibling, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2018-07-08 23:18 UTC (permalink / raw)
  To: syzbot+7b9ed9872dab8c32305d
  Cc: David Miller, Alexey Kuznetsov, LKML, Network Development,
	syzkaller-bugs, Hideaki YOSHIFUJI, Jiri Benc

On Sun, Jul 8, 2018 at 6:58 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Fri, Jul 6, 2018 at 6:16 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > On Fri, Jul 6, 2018 at 1:55 PM syzbot
> > <syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com> wrote:
> > >
> > > syzbot has found a reproducer for the following crash on:
> > >
> > > HEAD commit:    70ba5b6db96f ipv4: Return EINVAL when ping_group_range sys..
> > > git tree:       net
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=13cd2970400000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=7b9ed9872dab8c32305d
> > > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15dfb748400000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12a1050c400000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com
> > >
> > > IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
> > > IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> > > IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> > > 8021q: adding VLAN 0 to HW filter on device team0
> > > ==================================================================
> > > BUG: KASAN: use-after-free in ipv6_gso_pull_exthdrs+0x57a/0x5f0
> > > net/ipv6/ip6_offload.c:45
> > > Read of size 1 at addr ffff8801ce17f3a9 by task syz-executor655/4567
> >
> > This is an 8 byte packet with a virtio_net_hdr describing it as
> > VIRTIO_NET_HDR_GSO_TCPV4, but with protocol ETH_P_NSH. That
> > matches the occurrence of nsh_gso_segment in the stack trace.
> >
> > This pulls the struct nshhdr of 8B, passing a packet with skb->len 0
> > to skb_mac_gso_segment. That is the only GRO function that
> > unconditionally calls _skb_pull without first checking pskb_may_pull.
> > Adding that check does catch this:
> >
> > +       if (unlikely(!pskb_may_pull(skb, vlan_depth)))
> > +               return ERR_PTR(-EINVAL);
>
> vlan_depth is 65528 when entering skb_mac_gso_segment due to
> overflow of skb->mac_len in nsh_gso_segment. For this packet, the
> outer mac len is zero, so skb->data == skb_mac_header(skb). Then
>
>     skb_reset_network_header(skb);
>     [...]
>     __skb_pull(skb, nsh_len);
>     skb_reset_mac_header(skb);    // now mac hdr starts nsh_len == 8B
> after net hdr
>     skb_reset_mac_len(skb);          // mac len = net hdr - mac hdr ==
> (u16) -8 == 65528
>     [..]
>     skb_mac_gso_segment(skb, ..)
>
> Setting skb->mac_len to 0, similar to mpls_gs_segment,
> is sufficient if the encapsulated packet is not ETH_P_TEB.
>
> If the packet is encapsulated at L2, __skb_pull(skb, vlan_depth)
> has to pull the inner mac header before passing to l3 handlers like
> inet_gso_segment.
>
> If that header includes VLAN tags, skb_network_protocol will
> parse then and update the mac length in vlan_depth. So
> hardcoding to ETH_HLEN should be fine:
>
> @@ -104,7 +95,7 @@ static struct sk_buff *nsh_gso_segment(struct sk_buff *skb,
>         __skb_pull(skb, nsh_len);
>
>         skb_reset_mac_header(skb);
> -       skb_reset_mac_len(skb);
> +       skb->mac_len = proto == ETH_P_TEB ? ETH_HLEN : 0;

Well, htons(ETH_P_TEB)

but after this patch the reproducer no longer triggers.
ipv6_gso_segment will catch the zero length inner packet with
pskb_may_pull.

skb_mac_gso_segment itself does not strictly need a
pskb_may_pull, as skb_network_protocol calls pskb_may_pull
when processing ETH_P_TEB and VLAN headers.

Still, adding this would help catch bugs in callers sooner:

-       if (unlikely(!type))
+       if (unlikely(!type || !pskb_may_pull(skb, vlan_depth)))
                return ERR_PTR(-EINVAL);

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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-07-08 22:58     ` Willem de Bruijn
  2018-07-08 23:18       ` Willem de Bruijn
@ 2018-07-11 10:07       ` Jiri Benc
  2018-07-11 16:08         ` Willem de Bruijn
  1 sibling, 1 reply; 7+ messages in thread
From: Jiri Benc @ 2018-07-11 10:07 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: syzbot+7b9ed9872dab8c32305d, David Miller, Alexey Kuznetsov,
	LKML, Network Development, syzkaller-bugs, Hideaki YOSHIFUJI

Sorry for the delayed reply, I'm working through a pile of stuff after
being off.

On Sun, 8 Jul 2018 18:58:14 -0400, Willem de Bruijn wrote:
> Setting skb->mac_len to 0, similar to mpls_gs_segment,
> is sufficient if the encapsulated packet is not ETH_P_TEB.
> 
> If the packet is encapsulated at L2, __skb_pull(skb, vlan_depth)
> has to pull the inner mac header before passing to l3 handlers like
> inet_gso_segment.
> 
> If that header includes VLAN tags, skb_network_protocol will
> parse then and update the mac length in vlan_depth. So
> hardcoding to ETH_HLEN should be fine:
> 
> @@ -104,7 +95,7 @@ static struct sk_buff *nsh_gso_segment(struct sk_buff *skb,
>         __skb_pull(skb, nsh_len);
> 
>         skb_reset_mac_header(skb);
> -       skb_reset_mac_len(skb);
> +       skb->mac_len = proto == ETH_P_TEB ? ETH_HLEN : 0;
>         skb->protocol = proto;
> 
>         features &= NETIF_F_SG;

I agree. I think my original intention was to set mac_len to 0. Which
is obviously not done by calling skb_reset_mac_len...

Strangely, skb_network_protocol does not set *depth to ETH_HLEN if it
is 0 and the type is ETH_P_TEB, which is something I would expect it to
do. Thus we indeed have to differentiate between the two cases before
calling skb_mac_gso_segment.

Willem, will you send the patch formally (with the htons fix)? Thanks a
lot for the analysis and the patch!

 Jiri

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

* Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
  2018-07-11 10:07       ` Jiri Benc
@ 2018-07-11 16:08         ` Willem de Bruijn
  0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2018-07-11 16:08 UTC (permalink / raw)
  To: Jiri Benc
  Cc: syzbot+7b9ed9872dab8c32305d, David Miller, Alexey Kuznetsov,
	LKML, Network Development, syzkaller-bugs, Hideaki YOSHIFUJI

On Wed, Jul 11, 2018 at 6:07 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> Sorry for the delayed reply, I'm working through a pile of stuff after
> being off.
>
> On Sun, 8 Jul 2018 18:58:14 -0400, Willem de Bruijn wrote:
> > Setting skb->mac_len to 0, similar to mpls_gs_segment,
> > is sufficient if the encapsulated packet is not ETH_P_TEB.
> >
> > If the packet is encapsulated at L2, __skb_pull(skb, vlan_depth)
> > has to pull the inner mac header before passing to l3 handlers like
> > inet_gso_segment.
> >
> > If that header includes VLAN tags, skb_network_protocol will
> > parse then and update the mac length in vlan_depth. So
> > hardcoding to ETH_HLEN should be fine:
> >
> > @@ -104,7 +95,7 @@ static struct sk_buff *nsh_gso_segment(struct sk_buff *skb,
> >         __skb_pull(skb, nsh_len);
> >
> >         skb_reset_mac_header(skb);
> > -       skb_reset_mac_len(skb);
> > +       skb->mac_len = proto == ETH_P_TEB ? ETH_HLEN : 0;
> >         skb->protocol = proto;
> >
> >         features &= NETIF_F_SG;
>
> I agree. I think my original intention was to set mac_len to 0. Which
> is obviously not done by calling skb_reset_mac_len...
>
> Strangely, skb_network_protocol does not set *depth to ETH_HLEN if it
> is 0 and the type is ETH_P_TEB, which is something I would expect it to
> do. Thus we indeed have to differentiate between the two cases before
> calling skb_mac_gso_segment.
>
> Willem, will you send the patch formally (with the htons fix)? Thanks a
> lot for the analysis and the patch!

Thanks for verifying, Jiri.

Posted as http://patchwork.ozlabs.org/patch/942588/. I forgot to cc:
you directly in git send-email, sorry.

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

end of thread, other threads:[~2018-07-11 16:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-18 13:31 KASAN: use-after-free Read in ipv6_gso_pull_exthdrs syzbot
2018-07-06 17:52 ` syzbot
2018-07-06 22:16   ` Willem de Bruijn
2018-07-08 22:58     ` Willem de Bruijn
2018-07-08 23:18       ` Willem de Bruijn
2018-07-11 10:07       ` Jiri Benc
2018-07-11 16:08         ` Willem de Bruijn

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