On Thu, Aug 29, 2019 at 8:22 AM Shmulik Ladkani <shmulik.ladk...@gmail.com> wrote: > > On Tue, 27 Aug 2019 14:10:35 +0200 > Daniel Borkmann <dan...@iogearbox.net> wrote: > > > Given first point above wrt hitting rarely, it would be good to first get a > > better understanding for writing a reproducer. Back then Yonghong added one > > to the BPF kernel test suite [0], so it would be desirable to extend it for > > the case you're hitting. Given NAT64 use-case is needed and used by multiple > > parties, we should try to (fully) fix it generically. > > > > Thanks Daniel. > > Managed to write a reproducer which mimics the skb we see on prodction, > that hits the exact same BUG_ON. > > Submitted as a separate RFC PATCH to bpf-next.
Thanks for the reproducer. One quick fix is to disable sg and thus revert to copying in this case. Not ideal, but better than a kernel splat: @@ -3714,6 +3714,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, sg = !!(features & NETIF_F_SG); csum = !!can_checksum_protocol(features, proto); + if (list_skb && skb_headlen(list_skb) && !list_skb->head_frag) + sg = false; + It could perhaps be refined to avoid in the special case where skb_headlen(list_skb) == len and nskb aligned to start of list_skb. And needs looking into effect on GSO_BY_FRAGS. I also looked into trying to convert a kmalloc'ed skb->head into a headfrag. But even if possible, that conversion is non-trivial and easy to have bugs of its own. @@ -3849,8 +3885,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, if (!skb_headlen(list_skb)) { BUG_ON(!nfrags); } else { - BUG_ON(!list_skb->head_frag); - + BUG_ON(!list_skb->head_frag && + !skb_to_headfrag(list_skb, GFP_ATOMIC));