netdev - Re: [PATCH net] packet: on direct_xmit, limit tso and csum to supported devices

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-JP5re8oq9qLEAAnQW3SxHDLOt1PedPEKw8UsUJ6KNq6w@mail.gmail.com>
Date:   Wed, 26 Oct 2016 11:32:55 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] packet: on direct_xmit, limit tso and csum to
 supported devices

On Wed, Oct 26, 2016 at 6:47 AM, Daniel Borkmann <daniel@...earbox.net> wrote:
> On 10/26/2016 02:28 AM, Willem de Bruijn wrote:
>>
>> From: Willem de Bruijn <willemb@...gle.com>
>>
>> When transmitting on a packet socket with PACKET_VNET_HDR and
>> PACKET_QDISC_BYPASS, validate device support for features requested
>> in vnet_hdr.
>>
>> Drop TSO packets sent to devices that do not support TSO or have the
>> feature disabled. Note that the latter currently do process those
>> packets correctly, regardless of not advertising the feature.
>>
>> Because of SKB_GSO_DODGY, it is not sufficient to test device features
>> with netif_needs_gso. Full validate_xmit_skb is needed.
>>
>> Switch to software checksum for non-TSO packets that request checksum
>> offload if that device feature is unsupported or disabled. Note that
>> similar to the TSO case, device drivers may perform checksum offload
>> correctly even when not advertising it.
>>
>> When switching to software checksum, packets hit skb_checksum_help,
>> which has two BUG_ON checksum not in linear segment. Packet sockets
>> always allocate at least up to csum_start + csum_off + 2 as linear.
>
>
> Ok, for the tpacket_fill_skb() case with SOCK_RAW, it's guaranteed,
> because if we have a vnet header, then copylen would cover that here
> (as opposed to just dev->hard_header_len). With Eric's suggestion,
> looks good to me. Thanks, Willem!

Right. For skb_checksum_help, the important check is that it is only entered
when ip_summed == CHECKSUM_PARTIAL. Packet sockets only set that
field after ensuring hdr_len exceeds the checksum.

But in general, linear headers are not guaranteed. A particularly fun
edge case is a device with mtu > PAGE_SIZE, which allows
packet_alloc_skb to create a packet with any linear length > 0 as long
as it does not ask for checksum offload or gso.

I plan to send a patch to net-next to always allocate headers in the
linear segment. skb_probe_transport_header is called in the send path,
but after allocation, so we may have to settle for simpler MAX_HEADER.

Thanks for reviewing, Daniel!