[PATCH 4.19 18/48] net: stricter validation of untrusted gso packets

Greg Kroah-Hartman
2020-05-13T09:47:08Z

From: Willem de Bruijn <willemb@google.com>

[ Upstream commit 9274124f023b5c56dc4326637d4f787968b03607 ]

Syzkaller again found a path to a kernel crash through bad gso input:
a packet with transport header extending beyond skb_headlen(skb).

Tighten validation at kernel entry:

- Verify that the transport header lies within the linear section.

To avoid pulling linux/tcp.h, verify just sizeof tcphdr.
tcp_gso_segment will call pskb_may_pull (th->doff * 4) before use.

- Match the gso_type against the ip_proto found by the flow dissector.

Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/virtio_net.h | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -3,6 +3,8 @@
#define _LINUX_VIRTIO_NET_H

#include <linux/if_vlan.h>
+#include <uapi/linux/tcp.h>
+#include <uapi/linux/udp.h>
#include <uapi/linux/virtio_net.h>

static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
@@ -28,17 +30,25 @@ static inline int virtio_net_hdr_to_skb(
bool little_endian)
{
unsigned int gso_type = 0;
+ unsigned int thlen = 0;
+ unsigned int ip_proto;

if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
case VIRTIO_NET_HDR_GSO_TCPV4:
gso_type = SKB_GSO_TCPV4;
+ ip_proto = IPPROTO_TCP;
+ thlen = sizeof(struct tcphdr);
break;
case VIRTIO_NET_HDR_GSO_TCPV6:
gso_type = SKB_GSO_TCPV6;
+ ip_proto = IPPROTO_TCP;
+ thlen = sizeof(struct tcphdr);
break;
case VIRTIO_NET_HDR_GSO_UDP:
gso_type = SKB_GSO_UDP;
+ ip_proto = IPPROTO_UDP;
+ thlen = sizeof(struct udphdr);
break;
default:
return -EINVAL;
@@ -57,16 +67,22 @@ static inline int virtio_net_hdr_to_skb(

if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
+
+ if (skb_transport_offset(skb) + thlen > skb_headlen(skb))
+ return -EINVAL;
} else {
/* gso packets without NEEDS_CSUM do not set transport_offset.
* probe and drop if does not match one of the above types.
*/
if (gso_type && skb->network_header) {
+ struct flow_keys_basic keys;
+
if (!skb->protocol)
virtio_net_hdr_set_proto(skb, hdr);
retry:
- skb_probe_transport_header(skb, -1);
- if (!skb_transport_header_was_set(skb)) {
+ if (!skb_flow_dissect_flow_keys_basic(skb, &keys,
+ NULL, 0, 0, 0,
+ 0)) {
/* UFO does not specify ipv4 or 6: try both */
if (gso_type & SKB_GSO_UDP &&
skb->protocol == htons(ETH_P_IP)) {
@@ -75,6 +91,12 @@ retry:
}
return -EINVAL;
}
+
+ if (keys.control.thoff + thlen > skb_headlen(skb) ||
+ keys.basic.ip_proto != ip_proto)
+ return -EINVAL;
+
+ skb_set_transport_header(skb, keys.control.thoff);
}
}