more virtio-net csum hardening

As reported first by Paulos Yibelo <[email protected]> (after which others, credited below, found more ways the bug can affect users) a virtio net header can place CHECKSUM_PARTIAL at any in-packet offset. If that state is retained, later checksum completion can modify the packet after protocol processing has made decisions from it.

Let's fix that: let's validate csum_start and csum_offset and only keep CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or UDP header, while handling VLAN headers, IPv4 options and IPv6 extension headers. Complete other non-GSO checksums immediately, before any protocol processing runs.

We can't easily "complete the checksum" for GSO we don't recognize - GSO assumes knowledge of the protocol. We could segment ignoring the checksum then do whatever csum_start and csum_offset say, but that seems like an overkill for something that shouldn't happen.

There's some risk here:
- non-gso packets which can't be parsed get checksum calculated in software, adding performance overhead
- gso packets which can't be parsed get dropped

Note that for zero copy, userspace can change the data after the checksum has been calculated. This is fundamental to zero copy by design, and the only outcome is a bad checksum and a packet drop.

To mitigate the risks, the following patch will add a sysctl knob to bypass the resolution. This is reasonable if the setup is such that there is no protocol processing at all.

UDP tunnel headers get the same handling on the inner header.

Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.

Note a fundamental limitation of this approach: an nf/BPF program that tweaks the payload of a packet in flight may rely on the checksum being fixed up later, assuming the packet carries CHECKSUM_PARTIAL.

For metadata that fails validation the checksum
is now completed at input, so such edits leave a stale checksum and the packet is dropped at the receiver. Protocols whose transport header we cannot parse (anything but TCP and UDP) are affected even when the metadata is self-consistent.

The sysctl added in the next patch is the escape hatch for these setups, but it is a very blunt tool.

Reported-by Paulos Yibelo <[email protected]>

Reported-by Chris Arges <[email protected]>

Tested-by Chris Arges <[email protected]>

Fixes f43798c27684 ("tun: Allow GSO using virtio_net_hdr")

Suggested-by Willem de Bruijn <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

include/linux/virtio_net.h +23 -0

+static inline bool
+virtio_net_l4_start(struct sk_buff *skb, unsigned int nhoff, __be16 proto,
+		    unsigned int *thoff, int *ip_proto)
+{
+	if (proto == htons(ETH_P_IP)) {
+		const struct iphdr *iph = virtio_net_ip4_parse(skb, nhoff, thoff);
+
+		if (!iph || ip_is_fragment(iph))
+			return false;
+		*ip_proto = iph->protocol;
+		return true;
+	}
+
+	if (proto == htons(ETH_P_IPV6)) {
+		const struct ipv6hdr *iph;
+		int flags = 0;
+
+		if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
+			return false;
+		iph = (const void *)(skb->data + nhoff);
+		if (iph->version != 6)
+			return false;
+		*thoff = nhoff + sizeof(*iph);

include/linux/virtio_net.h +10 -0

+		}
+		return true;
+	}
+
+	return false;
+}
+

@@ -368,6 +660,21 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	skb->inner_mac_header = inner_nh + skb_headroom(skb);
 	skb->transport_header = outer_th + skb_headroom(skb);
 	skb->encapsulation = 1;
+
+	if (!skb_is_gso(skb)) {
+		/* Too small to segment: complete the checksum in place

include/linux/virtio_net.h +2 -0

+	}
+
 	return 0;
 }
 

Signature · 1 line

MST

Quoted 78 lines · Michael S. Tsirkin

As reported first by Paulos Yibelo <[email protected]> (after which others, credited below, found more ways the bug can affect users) a virtio net header can place CHECKSUM_PARTIAL at any in-packet offset. If that state is retained, later checksum completion can modify the packet after protocol processing has made decisions from it.

Let's fix that: let's validate csum_start and csum_offset and only keep CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or UDP header, while handling VLAN headers, IPv4 options and IPv6 extension headers. Complete other non-GSO checksums immediately, before any protocol processing runs.

We can't easily "complete the checksum" for GSO we don't recognize - GSO assumes knowledge of the protocol. We could segment ignoring the checksum then do whatever csum_start and csum_offset say, but that seems like an overkill for something that shouldn't happen.

There's some risk here:
- non-gso packets which can't be parsed get checksum calculated in software, adding performance overhead
- gso packets which can't be parsed get dropped

Note that for zero copy, userspace can change the data after the checksum has been calculated. This is fundamental to zero copy by design, and the only outcome is a bad checksum and a packet drop.

To mitigate the risks, the following patch will add a sysctl knob to bypass the resolution. This is reasonable if the setup is such that there is no protocol processing at all.

UDP tunnel headers get the same handling on the inner header.

Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.

Note a fundamental limitation of this approach: an nf/BPF program that tweaks the payload of a packet in flight may rely on the checksum being fixed up later, assuming the packet carries CHECKSUM_PARTIAL.

For metadata that fails validation the checksum
is now completed at input, so such edits leave a stale checksum and the packet is dropped at the receiver. Protocols whose transport header we cannot parse (anything but TCP and UDP) are affected even when the metadata is self-consistent.

The sysctl added in the next patch is the escape hatch for these setups, but it is a very blunt tool.

Reported-by Paulos Yibelo <[email protected]>

Reported-by Chris Arges <[email protected]>

Tested-by Chris Arges <[email protected]>

Fixes f43798c27684 ("tun: Allow GSO using virtio_net_hdr")

Suggested-by Willem de Bruijn <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

include/linux/virtio_net.h +23 -0

+static inline bool
+virtio_net_l4_start(struct sk_buff *skb, unsigned int nhoff, __be16 proto,
+		    unsigned int *thoff, int *ip_proto)
+{
+	if (proto == htons(ETH_P_IP)) {
+		const struct iphdr *iph = virtio_net_ip4_parse(skb, nhoff, thoff);
+
+		if (!iph || ip_is_fragment(iph))
+			return false;
+		*ip_proto = iph->protocol;
+		return true;
+	}
+
+	if (proto == htons(ETH_P_IPV6)) {
+		const struct ipv6hdr *iph;
+		int flags = 0;
+
+		if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
+			return false;
+		iph = (const void *)(skb->data + nhoff);
+		if (iph->version != 6)
+			return false;
+		*thoff = nhoff + sizeof(*iph);

include/linux/virtio_net.h static inline int virtio_net_hdr_set_proto(struct sk_buff *skb, · line 187

Quoted 10 lines

include/linux/virtio_net.h +10 -0

+		*ip_proto = iph->nexthdr;
+		if (*ip_proto != IPPROTO_TCP && *ip_proto != IPPROTO_UDP) {
+			*thoff = nhoff;
+			/* Find the transport header behind any
+			 * extension headers, AH included.
+			 */
+			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
+						  &flags);
+			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
+				return false;

Minor, not cause for respinning on its own.

flags should include IP6_FH_F_AUTH so ipv6_find_hdr() stops at NEXTHDR_AUTH instead of skipping it:

- CHECKSUM_PARTIAL and GSO behind IPsec AH are invalid: AH authenticates the inner L4 header including its checksum field

Quoted 15 lines

include/linux/virtio_net.h +10 -0

+		}
+		return true;
+	}
+
+	return false;
+}
+

@@ -368,6 +660,21 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	skb->inner_mac_header = inner_nh + skb_headroom(skb);
 	skb->transport_header = outer_th + skb_headroom(skb);
 	skb->encapsulation = 1;
+
+	if (!skb_is_gso(skb)) {
+		/* Too small to segment: complete the checksum in place

include/linux/virtio_net.h virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, · line 675

Quoted 10 lines

include/linux/virtio_net.h +10 -0

+		 * instead of dropping - senders do tag small packets
+		 * with GSO headers, e.g. packetdrill sets gso_type and
+		 * gso_size with --mss, whatever the packet size.
+		 * The outer UDP checksum is not calculated, but a zero
+		 * one is fine: UDP over IPv4 allows a disabled
+		 * checksum, and so do tunnels over IPv6 where the
+		 * receiver opts in (RFC 6935, RFC 6936).
+		 */
+		skb_shinfo(skb)->gso_type &= ~(SKB_GSO_UDP_TUNNEL_CSUM |
+					       SKB_GSO_UDP_TUNNEL);

Maybe this carve-out for non-gso packet is best moved to its own patch, as that is where the remaining issues (to merging the rest) seem to be.

1. Moving !tnl_csum_negotiated below virtio_net_csum_partial_ok_at_offset() accepts packets with an unnegotiated VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM flag whenever inner validation falls back to __virtio_net_resolve_csum_partial_mismatch().
2. __virtio_net_resolve_csum_partial_mismatch() calls skb_checksum_help(), which completes only the inner checksum at hdr->csum_start and sets skb->ip_summed = CHECKSUM_NONE, leaving the outer UDP uh->check with an uncompleted pseudo-header checksum seed.

Despite the comment, this block neither completes any checksum in place nor is the outer UDP checksum zero when VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM is set:

- With VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM, the outer uh->check carries the non-zero pseudo-header checksum seed (as expected by __skb_udp_tunnel_segment()), not 0.

- Clearing SKB_GSO_UDP_TUNNEL_CSUM on a !skb_is_gso(skb) packet leaves skb->ip_summed == CHECKSUM_PARTIAL at inner_th with nothing completing the outer uh->check on transmit/forwarding (validate_xmit_skb() only completes/offloads the single inner checksum, emitting a corrupt outer UDP checksum). Even zeroing uh->check would be invalid over IPv6 unless the receiver explicitly opted into RFC 6936 (UDP_NO_CHECK6_RX).

Quoted 8 lines

include/linux/virtio_net.h +2 -0

+	}
+
 	return 0;
 }
 

Signature · 1 line

MST

As reported first by Paulos Yibelo <[email protected]> (after which others, credited below, found more ways the bug can affect users) a virtio net header can place CHECKSUM_PARTIAL at any in-packet offset. If that state is retained, later checksum completion can modify the packet after protocol processing has made decisions from it.

Let's fix that: let's validate csum_start and csum_offset and only keep CHECKSUM_PARTIAL when csum_start and csum_offset match a parsed TCP or UDP header, while handling VLAN headers, IPv4 options and IPv6 extension headers. Complete other non-GSO checksums immediately, before any protocol processing runs.

We can't easily "complete the checksum" for GSO we don't recognize - GSO assumes knowledge of the protocol. We could segment ignoring the checksum then do whatever csum_start and csum_offset say, but that seems like an overkill for something that shouldn't happen.

There's some risk here:
- non-gso packets which can't be parsed get checksum calculated in software, adding performance overhead
- gso packets which can't be parsed get dropped

Note that for zero copy, userspace can change the data after the checksum has been calculated. This is fundamental to zero copy by design, and the only outcome is a bad checksum and a packet drop.

To mitigate the risks, the following patch will add a sysctl knob to bypass the resolution. This is reasonable if the setup is such that there is no protocol processing at all.

UDP tunnel headers get the same handling on the inner header.

Packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are unaffected.

Note a fundamental limitation of this approach: an nf/BPF program that tweaks the payload of a packet in flight may rely on the checksum being fixed up later, assuming the packet carries CHECKSUM_PARTIAL.

For metadata that fails validation the checksum
is now completed at input, so such edits leave a stale checksum and the packet is dropped at the receiver. Protocols whose transport header we cannot parse (anything but TCP and UDP) are affected even when the metadata is self-consistent.

The sysctl added in the next patch is the escape hatch for these setups, but it is a very blunt tool.

Reported-by Paulos Yibelo <[email protected]>

Reported-by Chris Arges <[email protected]>

Tested-by Chris Arges <[email protected]>

Fixes f43798c27684 ("tun: Allow GSO using virtio_net_hdr")

Suggested-by Willem de Bruijn <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---

Notes (changelog):
v4->v5:
caller changes moved to "account for ingress-resolved virtio net hdr state" (Willem de Bruijn)
a packet too small to be GSO: ignore the tunnel GSO request, complete wild metadata in place (sashiko) document the zero copy interaction (sashiko) honor a preset skb->protocol on ethernet: the stack uses it, except on loopback which re-parses the frame (sashiko)
v5->v6:
read both skb->protocol and the frame ethertype: do not validate when they disagree, instead of preferring the preset protocol; same check after the IP version inference where there is no L2 header (Willem de Bruijn) pull the IPv4 options in the helper as introduced, with the transport header offset computed before the pull (Willem de Bruijn)
fold the trivial protocol calculation cleanup, formerly a separate patch, in (Willem de Bruijn)

 include/linux/virtio_net.h | 315 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 311 insertions(+), 4 deletions(-)

include/linux/virtio_net.h +311 -4

index 013c6c7e2dea..54048759cc6e 100644
@@ -5,7 +5,11 @@
 #include <linux/if_vlan.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/netdevice.h>
 #include <linux/udp.h>
+#include <net/gso.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
 #include <net/tcp.h>
 #include <uapi/linux/tcp.h>
 #include <uapi/linux/virtio_net.h>

@@ -48,6 +52,216 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
 	return 0;
 }
 
+/* TODO: nothing virtio specific here, move to ip.h */
+
+/*
+ * Pull and minimally validate the IPv4 header at @nhoff.
+ * Returns the header and stores the transport header offset in
+ * @thoff, or NULL if truncated or malformed.  The whole header,
+ * options included, is guaranteed linear on return.
+ */
+static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
+						  unsigned int nhoff,
+						  unsigned int *thoff)
+{
+	unsigned int ihl;
+	struct iphdr *iph;
+
+	if (!pskb_may_pull(skb, nhoff + sizeof(struct iphdr)))
+		return NULL;
+
+	iph = (struct iphdr *)(skb->data + nhoff);
+	if (iph->version != 4 ||
+	    iph->ihl < sizeof(struct iphdr) / sizeof(u32))
+		return NULL;
+
+	ihl = iph->ihl * sizeof(u32);
+	*thoff = nhoff + ihl;
+	/* Pull the options as well: the transport header offset
+	 * returned above must be within the linear data.  iph goes
+	 * stale across the pull.
+	 */
+	if (!pskb_may_pull(skb, nhoff + ihl))
+		return NULL;
+
+	return (struct iphdr *)(skb->data + nhoff);
+}
+
+/* Where does the L3 header start, and what protocol is it?
+ * Handles Ethernet and VLAN headers; other devices use the
+ * network header offset the caller set up.
+ */
+static inline bool
+virtio_net_l3_start(struct sk_buff *skb, int *nhoff, __be16 *proto)
+{
+	/* skb->dev should always be set here already,
+	 * but let's not crash when it is not.
+	 */
+	if (WARN_ON_ONCE(!skb->dev))
+		return false;
+
+	if (skb->dev->type == ARPHRD_ETHER ||
+	    skb->dev->type == ARPHRD_LOOPBACK) {
+		__be16 h_proto;
+
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			return false;
+		h_proto = eth_hdr(skb)->h_proto;
+		/* skb->protocol is a sender claim, and parts of the
+		 * stack parse the packet per it: when it disagrees with
+		 * the frame we do not know which view the packet will
+		 * be parsed as, so do not validate.
+		 */
+		if (skb->protocol && skb->protocol != h_proto)
+			return false;
+		*nhoff = ETH_HLEN;
+		*proto = h_proto;
+		if (eth_type_vlan(*proto)) {
+			*proto = __vlan_get_protocol(skb, *proto, nhoff);
+			if (!*proto)
+				return false;
+		}
+	} else {
+		const struct iphdr *iph;
+
+		*nhoff = skb_network_offset(skb);
+		/* An unset network header shows up as an offset
+		 * outside of the packet.
+		 */
+		if (*nhoff < 0 || (unsigned int)*nhoff >= skb->len)
+			return false;
+		/* No L2 header to cross-check with: take the protocol
+		 * from the IP version at the network header.
+		 */
+		if (!pskb_may_pull(skb, *nhoff + 1))
+			return false;
+		iph = (const struct iphdr *)(skb->data + *nhoff);
+		if (iph->version == 4)
+			*proto = htons(ETH_P_IP);
+		else if (iph->version == 6)
+			*proto = htons(ETH_P_IPV6);
+		else
+			return false;
+		if (skb->protocol && skb->protocol != *proto)
+			return false;
+	}
+	return true;
+}
+
+/* Walk the L3 header at @nhoff: where does its transport header
+ * start, and which protocol is it?  Returns false if the header
+ * is truncated, malformed or fragmented.
+ */
+static inline bool
+virtio_net_l4_start(struct sk_buff *skb, unsigned int nhoff, __be16 proto,
+		    unsigned int *thoff, int *ip_proto)
+{
+	if (proto == htons(ETH_P_IP)) {
+		const struct iphdr *iph = virtio_net_ip4_parse(skb, nhoff, thoff);
+
+		if (!iph || ip_is_fragment(iph))
+			return false;
+		*ip_proto = iph->protocol;
+		return true;
+	}
+
+	if (proto == htons(ETH_P_IPV6)) {
+		const struct ipv6hdr *iph;
+		int flags = 0;
+
+		if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
+			return false;
+		iph = (const void *)(skb->data + nhoff);
+		if (iph->version != 6)
+			return false;
+		*thoff = nhoff + sizeof(*iph);
+		*ip_proto = iph->nexthdr;
+		if (*ip_proto != IPPROTO_TCP && *ip_proto != IPPROTO_UDP) {
+			*thoff = nhoff;
+			/* Find the transport header behind any
+			 * extension headers, AH included.
+			 */
+			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
+						  &flags);
+			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
+				return false;
+		}
+		return true;
+	}
+
+	return false;
+}
+
+/* Does checksum metadata match a known TCP or UDP header? */
+static inline bool
+virtio_net_csum_partial_ok_at_offset(struct sk_buff *skb, u8 hdr_gso_type,
+				     unsigned int nhoff, __be16 proto)
+{
+	unsigned int thoff;
+	int ip_proto;
+
+	if (!virtio_net_l4_start(skb, nhoff, proto, &thoff, &ip_proto))
+		return false;
+
+	if (skb_checksum_start_offset(skb) != thoff)
+		return false;
+
+	switch (ip_proto) {
+	case IPPROTO_TCP:
+		if (skb->csum_offset != offsetof(struct tcphdr, check) ||
+		    !pskb_may_pull(skb, thoff + sizeof(struct tcphdr)))
+			return false;
+		break;
+	case IPPROTO_UDP:
+		if (skb->csum_offset != offsetof(struct udphdr, check) ||
+		    !pskb_may_pull(skb, thoff + sizeof(struct udphdr)))
+			return false;
+		break;
+	default:
+		return false;
+	}
+
+	switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+	case VIRTIO_NET_HDR_GSO_NONE:
+		return true;
+	case VIRTIO_NET_HDR_GSO_TCPV4:
+		return proto == htons(ETH_P_IP) && ip_proto == IPPROTO_TCP;
+	case VIRTIO_NET_HDR_GSO_TCPV6:
+		return proto == htons(ETH_P_IPV6) && ip_proto == IPPROTO_TCP;
+	case VIRTIO_NET_HDR_GSO_UDP:
+	case VIRTIO_NET_HDR_GSO_UDP_L4:
+		return ip_proto == IPPROTO_UDP;
+	default:
+		return false;
+	}
+}
+
+static inline bool
+virtio_net_csum_partial_ok(struct sk_buff *skb, u8 hdr_gso_type)
+{
+	int nhoff;
+	__be16 proto;
+
+	if (!virtio_net_l3_start(skb, &nhoff, &proto))
+		return false;
+
+	return virtio_net_csum_partial_ok_at_offset(skb, hdr_gso_type, nhoff,
+						   proto);
+}
+
+/*
+ * Handle a packet whose checksum metadata does not match the packet:
+ * GSO packets are dropped, as segmentation needs one checksum per
+ * segment; others are completed in software.
+ */
+static inline int __virtio_net_resolve_csum_partial_mismatch(struct sk_buff *skb)
+{
+	if (skb_is_gso(skb))
+		return -EINVAL;
+
+	return skb_checksum_help(skb) ?: 1;
+}
+
 static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
 					  const struct virtio_net_hdr *hdr,
 					  bool little_endian, u8 hdr_gso_type)

@@ -194,11 +408,35 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
 	return 0;
 }
 
+/*
+ * Return value:
+ * 0 - success
+ * >0 - checksum was completed in software (slow path)
+ * <0 - failure
+ */
 static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 					const struct virtio_net_hdr *hdr,
 					bool little_endian)
 {
-	return __virtio_net_hdr_to_skb(skb, hdr, little_endian, hdr->gso_type);
+	typeof(skb->transport_header) old_transport = skb->transport_header;
+	int ret = __virtio_net_hdr_to_skb(skb, hdr, little_endian,
+					  hdr->gso_type);
+
+	if (ret < 0)
+		return ret;
+
+	if (likely(skb->ip_summed != CHECKSUM_PARTIAL ||
+		   virtio_net_csum_partial_ok(skb, hdr->gso_type)))
+		return 0;
+
+	/*
+	 * Here we do not trust the checksum metadata.
+	 * __virtio_net_hdr_to_skb set the transport header to csum_start
+	 * from virtio hdr, restore it to a known good value.
+	 */
+	skb->transport_header = old_transport;
+
+	return __virtio_net_resolve_csum_partial_mismatch(skb);
 }
 
 /* This function must be called after virtio_net_hdr_from_skb(). */

@@ -282,6 +520,35 @@ static inline unsigned int virtio_l3min(bool is_ipv6)
 	return is_ipv6 ? sizeof(struct ipv6hdr) : sizeof(struct iphdr);
 }
 
+/* Do the tunnel offsets match the outer headers in the frame?
+ * The L3 header the tunnel claims must be the one the frame
+ * carries, with UDP following it at @outer_th.
+ */
+static inline bool
+virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool outer_isv6,
+			  unsigned int outer_th)
+{
+	int nhoff;
+	__be16 proto;
+	unsigned int thoff;
+	int ip_proto;
+
+	if (!virtio_net_l3_start(skb, &nhoff, &proto))
+		return false;
+	if (outer_isv6 != (proto == htons(ETH_P_IPV6)))
+		return false;
+	if (!virtio_net_l4_start(skb, nhoff, proto, &thoff, &ip_proto))
+		return false;
+
+	return ip_proto == IPPROTO_UDP && outer_th == thoff;
+}
+
+/*
+ * Return value:
+ * 0 - success
+ * >0 - checksum was completed in software (slow path)
+ * <0 - failure
+ */
 static inline int
 virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 			  const struct virtio_net_hdr_v1_hash_tunnel *vhdr,

@@ -290,9 +557,11 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 			  bool little_endian)
 {
 	const struct virtio_net_hdr *hdr = (const struct virtio_net_hdr *)vhdr;
+	typeof(skb->transport_header) old_transport = skb->transport_header;
 	unsigned int inner_nh, outer_th, inner_th;
 	unsigned int inner_l3min, outer_l3min;
 	u8 gso_inner_type, gso_tunnel_type;
+	__be16 inner_proto;
 	bool outer_isv6, inner_isv6;
 	int ret;
 

@@ -335,9 +604,12 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	/* Let the basic parsing deal with plain GSO features. */
 	ret = __virtio_net_hdr_to_skb(skb, hdr, true,
 				      hdr->gso_type & ~gso_tunnel_type);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
+	if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th))
+		return -EINVAL;
+
 	/* In case of USO, the inner protocol is still unknown and
 	 * `inner_isv6` is just a guess, additional parsing is needed.
 	 * The previous validation ensures that accessing an ipv4 inner

@@ -352,9 +624,29 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 			return -EINVAL;
 	}
 
-	skb_set_inner_protocol(skb, inner_isv6 ? htons(ETH_P_IPV6) :
-						 htons(ETH_P_IP));
+	inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP);
+
+	/*
+	 * We know VIRTIO_NET_HDR_F_NEEDS_CSUM was set so __virtio_net_hdr_to_skb
+	 * set CHECKSUM_PARTIAL.
+	 */
+	if (unlikely(!virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type,
+							  inner_nh,
+							  inner_proto))) {
+		/* __virtio_net_hdr_to_skb set the transport header to
+		 * csum_start, restore it to a known good value.
+		 */
+		skb->transport_header = old_transport;
+
+		return __virtio_net_resolve_csum_partial_mismatch(skb);
+	}
+
+	skb_set_inner_protocol(skb, inner_proto);
 	if (hdr->flags & VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM) {
+		/* Checked here, after the validation: a packet completed
+		 * in software never uses the feature, so an unnegotiated
+		 * request only matters for metadata good enough to act on.
+		 */
 		if (!tnl_csum_negotiated)
 			return -EINVAL;
 

@@ -368,6 +660,21 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
 	skb->inner_mac_header = inner_nh + skb_headroom(skb);
 	skb->transport_header = outer_th + skb_headroom(skb);
 	skb->encapsulation = 1;
+
+	if (!skb_is_gso(skb)) {
+		/* Too small to segment: complete the checksum in place
+		 * instead of dropping - senders do tag small packets
+		 * with GSO headers, e.g. packetdrill sets gso_type and
+		 * gso_size with --mss, whatever the packet size.
+		 * The outer UDP checksum is not calculated, but a zero
+		 * one is fine: UDP over IPv4 allows a disabled
+		 * checksum, and so do tunnels over IPv6 where the
+		 * receiver opts in (RFC 6935, RFC 6936).
+		 */
+		skb_shinfo(skb)->gso_type &= ~(SKB_GSO_UDP_TUNNEL_CSUM |
+					       SKB_GSO_UDP_TUNNEL);
+	}
+
 	return 0;
 }
 

Signature · 1 line

MST