netdev - Re: [PATCH net-next] udp: disable gso with no_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAF=yD-JkAsZCZozbMczK7wDrargquBdG0inV-DZe_47FcB3zHg@mail.gmail.com>
Date:   Wed, 2 May 2018 12:01:28 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next] udp: disable gso with no_check_tx

On Wed, May 2, 2018 at 9:05 AM, Michal Kubecek <mkubecek@...e.cz> wrote:
> On Mon, Apr 30, 2018 at 03:58:36PM -0400, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@...gle.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.