[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAF=yD-LHwi94CLpsQ42mFu6WEzAr-jrd+ZczAio2HhDtQMq0Ug@mail.gmail.com>
Date: Wed, 17 Jan 2018 18:17:22 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jason Wang <jasowang@...hat.com>
Cc: Network Development <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] net: validate untrusted gso packets
>>>>> From: Willem de Bruijn<willemb@...gle.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/
Powered by blists - more mailing lists