netdev - Re: [PATCH net] net: accept UFO datagrams from tuntap and packet

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF=yD-+QO6+y_frUfWREc8_OQQmThdy+-rMAkRXGB69h=KAxzQ@mail.gmail.com>
Date:   Sat, 18 Nov 2017 19:58:16 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Network Development <netdev@...r.kernel.org>,
        Jason Wang <jasowang@...hat.com>,
        Michal Kubecek <mkubecek@...e.cz>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] net: accept UFO datagrams from tuntap and packet

On Fri, Nov 17, 2017 at 8:42 PM, David Miller <davem@...emloft.net> wrote:
> From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> Date: Fri, 17 Nov 2017 17:59:13 -0500
>
>> Tuntap and similar devices can inject GSO packets. Accept type
>> VIRTIO_NET_HDR_GSO_UDP, even though not generating UFO natively.
>>
>> Processes are expected to use feature negotiation such as TUNSETOFFLOAD
>> to detect supported offload types and refrain from injecting other
>> packets. This process breaks down with live migration: guest kernels
>> do not renegotiate flags, so destination hosts need to expose all
>> features that the source host does.
>>
>> Partially revert the UFO removal from 182e0b6b5846~1..d9d30adf5677.
>> This patch introduces nearly(*) no new code to simplify verification.
>> It brings back verbatim tuntap UFO negotiation, VIRTIO_NET_HDR_GSO_UDP
>> insertion and software UFO segmentation.
>
> This looks good, one minor nit:
>
>> @@ -2369,6 +2369,10 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)
>>                               features |= NETIF_F_TSO6;
>>                       arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
>>               }
>> +
>> +             if (arg & TUN_F_UFO) {
>> +                     arg &= ~TUN_F_UFO;
>> +             }
>
> This can be just simply "arg &= ~TUN_F_UFO;"?  If anything the curly braces
> should be removed for a single statement basic block.

Okay, I'll fix that up in v2. At least for the initial version I wanted
to make as few changes as possible to the original code.