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

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

>>>>> 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. */
>
> No dodgy source can legitimately generate sctp code, so it should not
> even get there. Also, a packet can just as easily spoof an esp packet.
> See also the discussion in the Link above.
>
> We can address this specific issue in segmentation by placing a check
> analogous to skb_validate_dodgy_gso in the network layer. Something
> like this (but with ECN option support):
>
> @@ -1258,6 +1258,22 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>
>         skb_reset_transport_header(skb);
>
> +       gso_type = skb_shinfo(skb)->gso_type;
> +       if (gso_type & SKB_GSO_DODGY) {
> +               switch (gso_type & (SKB_GSO_TCPV4 | SKB_GSO_UDP)) {
> +               case SKB_GSO_TCPV4:
> +                       if (proto != IPPROTO_TCP)
> +                               goto out;
> +                       break;
> +               case SKB_GSO_UDP:
> +                       if (proto != IPPROTO_UDP)
> +                               goto out;
> +                       break;
> +               default:
> +                       goto out;
> +               }
> +       }

Okay, I sent this instead: http://patchwork.ozlabs.org/patch/862643/