On Wed, May 2, 2018 at 9:05 AM, Michal Kubecek [off-list ref] wrote:

On Mon, Apr 30, 2018 at 03:58:36PM -0400, Willem de Bruijn wrote:

quoted

From: Willem de Bruijn <willemb@google.com>

Syzbot managed to send a udp gso packet without checksum offload into
the gso stack by disabling tx checksum (UDP_NO_CHECK6_TX). This
triggered the skb_warn_bad_offload.

  RIP: 0010:skb_warn_bad_offload+0x2bc/0x600 net/core/dev.c:2658
   skb_gso_segment include/linux/netdevice.h:4038 [inline]
   validate_xmit_skb+0x54d/0xd90 net/core/dev.c:3120
   __dev_queue_xmit+0xbf8/0x34c0 net/core/dev.c:3577
   dev_queue_xmit+0x17/0x20 net/core/dev.c:3618

UDP_NO_CHECK6_TX sets skb->ip_summed to CHECKSUM_NONE just after the
udp gso integrity checks in udp_(v6_)send_skb. Extend those checks to
catch and fail in this case.

Sounds rather familiar... perhaps we might want to check other
exceptions added to UFO over the years, some might apply here as well.

Good point. Of the conditions in that infamous branch

        if ((((length + (skb ? skb->len : fragheaderlen)) > mtu) ||
             (skb && skb_is_gso(skb))) &&
            (sk->sk_protocol == IPPROTO_UDP) &&
            (rt->dst.dev->features & NETIF_F_UFO) && !dst_xfrm(&rt->dst) &&
            (sk->sk_type == SOCK_DGRAM) && !sk->sk_no_check_tx) {

the fragmentation related ones do not apply. The new condition is also simpler
as it is evaluated once when the entire datagram is built, as opposed to on each
append.

That leaves the xfrm test. Indeed, those need to be blocked as well.