Skip to content

Commit

Permalink
net: stricter validation of untrusted gso packets
Browse files Browse the repository at this point in the history
[ Upstream commit 9274124 ]

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: bfd5f4a ("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>
  • Loading branch information
wdebruij authored and gregkh committed May 14, 2020
1 parent 2d79fe5 commit f12aa51
Showing 1 changed file with 24 additions and 2 deletions.
26 changes: 24 additions & 2 deletions include/linux/virtio_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -28,17 +30,25 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *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;
Expand All @@ -57,16 +67,22 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *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)) {
Expand All @@ -75,6 +91,12 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
}
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);
}
}

Expand Down

0 comments on commit f12aa51

Please sign in to comment.