Re: [PATCH net] net: validate untrusted gso packets

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Tue, Jan 16, 2018 at 11:33 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Tue, Jan 16, 2018 at 11:04 PM, Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>> On 2018年01月17日 04:29, Willem de Bruijn wrote:
>>>
>>> From: Willem de Bruijn<willemb@google.com>
>>>
>>> Validate gso packet type and headers on kernel entry. Reuse the info
>>> gathered by skb_probe_transport_header.
>>>
>>> Syzbot found two bugs by passing bad gso packets in packet sockets.
>>> Untrusted user packets are limited to a small set of gso types in
>>> virtio_net_hdr_to_skb. But segmentation occurs on packet contents.
>>> Syzkaller was able to enter gso callbacks that are not hardened
>>> against untrusted user input.
>>
>>
>> Do this mean there's something missed in exist header check for dodgy
>> packets?
>
> virtio_net_hdr_to_skb checks gso_type, but it does not verify that this
> type correctly describes the actual packet. Segmentation happens based
> on packet contents. So a packet was crafted to enter sctp gso, even
> though no such gso_type exists. This issue is not specific to sctp.
>
>>>
>>> User packets can also have corrupted headers, tripping up segmentation
>>> logic that expects sane packets from the trusted protocol stack.
>>> Hardening all segmentation paths against all bad packets is error
>>> prone and slows down the common path, so validate on kernel entry.
>>
>>
>> I think evil packets should be rare in common case, so I'm not sure validate
>> it on kernel entry is a good choice especially consider we've already had
>> header check.
>
> This just makes that check more strict. Frequency of malicious packets is
> not really relevant if a single bad packet can cause damage.
>
> The alternative to validate on kernel entry is to harden the entire segmentation
> layer and lower part of the stack. That is much harder to get right and not
> necessarily cheaper.
>
> As a matter of fact, it incurs a cost on all packets, including the common
> case generated by the protocol stack.

If packets can be fully validated at the source, we can eventually also
get rid of the entire SKB_GSO_DODGY and NETIF_F_GSO_ROBUST
logic. Then virtio packets won't have to enter the segmentation layer
at all for TSO capable devices.