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

[Jason Wang <jasowang@redhat.com>]

On 2018年01月17日 12:33, Willem de Bruijn 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.

So it looks to me we should do it in here in sctp_gso_segment().

if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
         /* Packet is from an untrusted source, reset gso_segs. */


And we probably need to recover what has been removed since 
5c7cdf339af560f980b12eb6b0b5aa5f68ac6658 ("gso: Remove arbitrary checks 
for unsupported GSO").

>
>>> 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.

We try hard to avoid flow dissector since its overhead is obvious. But 
looks like this patch did it unconditionally, and even for non gso packet.

> 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.

For performance reason. I think we should delay the check or 
segmentation as much as possible until it was really needed.

>
> As a matter of fact, it incurs a cost on all packets, including the common
> case generated by the protocol stack.

Btw, this looks could be triggered from guest. So it looks at least a 
DOS form guest to host which should be treated as CVE?

Thanks