From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Benc Subject: Re: KASAN: use-after-free Read in ipv6_gso_pull_exthdrs Date: Wed, 11 Jul 2018 12:07:53 +0200 Message-ID: <20180711120753.6940b66d@redhat.com> References: <000000000000b0ee7a056eea93a7@google.com> <00000000000033f8440570585207@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com, David Miller , Alexey Kuznetsov , LKML , Network Development , syzkaller-bugs@googlegroups.com, Hideaki YOSHIFUJI To: Willem de Bruijn Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org 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