Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2018-07-08 23:19:29
Also in:
lkml
On Sun, Jul 8, 2018 at 6:58 PM Willem de Bruijn [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Fri, Jul 6, 2018 at 6:16 PM Willem de Bruijn [off-list ref] wrote:quoted
On Fri, Jul 6, 2018 at 1:55 PM syzbot [off-list ref] wrote:quoted
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/4567This 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);