[PATCH net] net: extend IPv6 exthdr detection of tunneled packets

[PATCH net] net: extend IPv6 exthdr detection of tunneled packets

[Willem de Bruijn]

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

Commit c4336a07eb6b ("net: correctly handle tunneled traffic on IPV6_CSUM
GSO fallback") split skb_gso_has_extension_hdr() into mutually exclusive
branches on skb->encapsulation. This did not yet address all paths:

1. With skb->encapsulation set, the outer header is not checked. IPv6
   tunnels such as ip6_gre and ip6_tunnel add an outer Destination
   Options header by default (encap_limit). Their GSO packets skip
   software GSO, then skb_csum_hwoffload_help() sees the outer extension
   header and calls skb_checksum_help() on the GSO skb, which warns and
   drops it.

2. Tunnels over IPv6 without an outer transport header, such as
   ip6_tunnel, leave skb->transport_header at the inner transport
   header. skb_network_header_len() then spans the outer IPv6, tunnel
   and inner IP headers, a false positive.

3. UDP tunnels without an inner network header, such as SCTP-in-UDP or
   PSP, have no inner IPv6 header to check. Decide on the outer header
   alone.

4. Directly dereferencing inner_ip_hdr(skb)->version without
   skb_header_pointer() is unsafe.

Instead, check ipv6_ext_hdr(nexthdr) on the outer IPv6 header and, if
set, on the inner IPv6 header. Read the headers with skb_header_pointer().

Use the same helper in skb_csum_hwoffload_help(). Its open coded test
has the false positive of (2) and ignores the inner header.

Background: checksum offload of tunneled packets invariants:

Non-GSO skb:
- If the inner packet is CHECKSUM_PARTIAL, Local Checksum Offload computes
  the outer checksum in software and the device offloads only the inner L4
  checksum.
- If the inner packet is CHECKSUM_NONE (e.g., SCTP-in-UDP, ESP-in-UDP,
  Remote Checksum Offload), the device offloads the outer UDP or GRE
  checksum instead.

GSO skb:
- A device with NETIF_F_GSO_UDP_TUNNEL_CSUM or NETIF_F_GSO_GRE_CSUM
  computes both inner and outer checksums per segment.
  The outer checksum is seeded with only the pseudo-header checksum.

A NETIF_F_IPV6_CSUM device must parse through the outer headers to
reach the inner ones.

Fixes: c4336a07eb6b ("net: correctly handle tunneled traffic on IPV6_CSUM GSO fallback")
Cc: stable@vger.kernel.org
Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Stable: trees before commit 1676ebba391d ("net/ipv6: Remove jumbo_remove
step from TX path") (v7.0) must keep the BIG TCP jumbo exemption on the
outer check, or BIG TCP loses TSO (see 68e068cabd2c):

	if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
	    !ipv6_has_hopopt_jumbo(skb) &&
	    __skb_has_ipv6_ext_hdr(skb, skb_network_offset(skb)))
		return true;

Tested with gre_gso.sh over veth with NETIF_F_IPV6_CSUM:
- GREv6 without extension headers
- GREv6 with inner dstopts
- GREv6 with outer encaplimit, and
- SCTP-in-UDPv6.

That needs a test-only ethtool feature to veth to advertise
NETIF_F_IPV6_CSUM (mutually exclusive with NETIF_F_HW_CSUM).

If no one objects (to test-only veth code), I can follow up with those
veth and selftest patches to net-next later.
---
 net/core/dev.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 0292a16e16c2..404ea7437b41 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3818,20 +3818,29 @@ static netdev_features_t dflt_features_check(struct sk_buff *skb,
 	return vlan_features_check(skb, features);
 }
 
-static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
-{
-	if (!skb->encapsulation)
-		return ((skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
-			 (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
-			  vlan_get_protocol(skb) == htons(ETH_P_IPV6))) &&
-			skb_transport_header_was_set(skb) &&
-			skb_network_header_len(skb) != sizeof(struct ipv6hdr));
-	else
-		return (!skb_inner_network_header_was_set(skb) ||
-			((skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
-			  (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
-			   inner_ip_hdr(skb)->version == 6)) &&
-			 skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr)));
+static bool __skb_has_ipv6_ext_hdr(const struct sk_buff *skb, int nhoff)
+{
+	const struct ipv6hdr *ip6h;
+	struct ipv6hdr _ip6h;
+
+	ip6h = skb_header_pointer(skb, nhoff, sizeof(_ip6h), &_ip6h);
+	return ip6h && ip6h->version == 6 && ipv6_ext_hdr(ip6h->nexthdr);
+}
+
+static bool skb_has_ipv6_extension_hdr(const struct sk_buff *skb)
+{
+	if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
+	    __skb_has_ipv6_ext_hdr(skb, skb_network_offset(skb)))
+		return true;
+
+	/* Tunnels without an inner network header, such as SCTP-in-UDP or
+	 * PSP, have no inner IP header and thus no inner extension header.
+	 */
+	if (skb->encapsulation && skb_inner_network_header_was_set(skb) &&
+	    __skb_has_ipv6_ext_hdr(skb, skb_inner_network_offset(skb)))
+		return true;
+
+	return false;
 }
 
 static netdev_features_t gso_features_check(const struct sk_buff *skb,
@@ -3886,7 +3895,7 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
 	 * so neither does TSO that depends on it.
 	 */
 	if (features & NETIF_F_IPV6_CSUM &&
-	    skb_gso_has_extension_hdr(skb))
+	    skb_has_ipv6_extension_hdr(skb))
 		features &= ~(NETIF_F_IPV6_CSUM | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4);
 
 	return features;
@@ -3988,8 +3997,7 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
 		return 0;
 
 	if (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) {
-		if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
-		    skb_network_header_len(skb) != sizeof(struct ipv6hdr))
+		if (skb_has_ipv6_extension_hdr(skb))
 			goto sw_checksum;
 
 		switch (skb->csum_offset) {
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] net: extend IPv6 exthdr detection of tunneled packets

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Regression for non-encapsulated IPv6 CHECKSUM_PARTIAL skbs in
  skb_csum_hwoffload_help(), plus a matching gap in…

--

net: extend IPv6 exthdr detection of tunneled packets

This patch replaces skb_gso_has_extension_hdr() with
skb_has_ipv6_extension_hdr(). The new helper runs ipv6_ext_hdr() on the
outer IPv6 nexthdr. For encapsulated skbs that have an inner network
header, it also runs it on the inner IPv6 nexthdr. skb_csum_hwoffload_help()
now uses the same helper instead of its open coded skb_network_header_len()
test.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0dbc..992b78308c13f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3818,20 +3818,29 @@ static netdev_features_t dflt_features_check(struct sk_buff *skb,
>  	return vlan_features_check(skb, features);
>  }
>  
> -static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
> -{
> -	if (!skb->encapsulation)
> -		return ((skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
> -			 (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
> -			  vlan_get_protocol(skb) == htons(ETH_P_IPV6))) &&
> -			skb_transport_header_was_set(skb) &&
> -			skb_network_header_len(skb) != sizeof(struct ipv6hdr));

[ ... ]

> +static bool __skb_has_ipv6_ext_hdr(const struct sk_buff *skb, int nhoff)
> +{
> +	const struct ipv6hdr *ip6h;
> +	struct ipv6hdr _ip6h;
> +
> +	ip6h = skb_header_pointer(skb, nhoff, sizeof(_ip6h), &_ip6h);
> +	return ip6h && ip6h->version == 6 && ipv6_ext_hdr(ip6h->nexthdr);
> +}
> +
> +static bool skb_has_ipv6_extension_hdr(const struct sk_buff *skb)
> +{
> +	if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
> +	    __skb_has_ipv6_ext_hdr(skb, skb_network_offset(skb)))
> +		return true;
> +
> +	/* Tunnels without an inner network header, such as SCTP-in-UDP or
> +	 * PSP, have no inner IP header and thus no inner extension header.
> +	 */
> +	if (skb->encapsulation && skb_inner_network_header_was_set(skb) &&
> +	    __skb_has_ipv6_ext_hdr(skb, skb_inner_network_offset(skb)))
> +		return true;
> +
> +	return false;
>  }

[ ... ]

> @@ -3988,8 +3997,7 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
>  		return 0;
>  
>  	if (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) {
> -		if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
> -		    skb_network_header_len(skb) != sizeof(struct ipv6hdr))
> +		if (skb_has_ipv6_extension_hdr(skb))
>  			goto sw_checksum;

[Severity: High]
Does this weaken the check for non-encapsulated IPv6 CHECKSUM_PARTIAL
skbs?

When !skb->encapsulation, validate_xmit_skb() sets the transport header to
csum_start just before it calls skb_csum_hwoffload_help():

net/core/dev.c:validate_xmit_skb() {
    ...
		if (skb->ip_summed == CHECKSUM_PARTIAL) {
			if (skb->encapsulation)
				skb_set_inner_transport_header(skb,
							       skb_checksum_start_offset(skb));
			else
				skb_set_transport_header(skb,
							 skb_checksum_start_offset(skb));
			if (skb_csum_hwoffload_help(skb, features))
				goto out_kfree_skb;
    ...
}

That means the removed skb_network_header_len() test checked that
csum_start sits right after the 40 byte IPv6 header.

The new helper only checks whether the outer nexthdr is one of the six
values listed in ipv6_ext_hdr(). It skips the inner check because
encapsulation is 0.

Take IPv6|UDP|VXLAN|Eth|IP|TCP, or IPv6|IPv6|TCP (nexthdr 41), with
csum_start at the inner TCP header and csum_offset 16. Would that packet
now reach the csum_offset switch and return 0?

Such skbs can come from untrusted input. __virtio_net_hdr_to_skb() is
used by tap, vhost-net and AF_PACKET with PACKET_VNET_HDR. It accepts
VIRTIO_NET_HDR_F_NEEDS_CSUM with any csum_start via
skb_partial_csum_set(). It does not look at nexthdr and does not set
skb->encapsulation.

One case: a guest sends IPv6 VXLAN traffic with a CHECKSUM_PARTIAL inner
TCP header (LCO). The host bridges it to a device that has
NETIF_F_IPV6_CSUM but not NETIF_F_HW_CSUM.

On a device that parses headers itself, such as bnxt, bnxt_exthdr_check()
accepts an outer nexthdr of UDP for non-encapsulated skbs. The hardware
would then fill in the outer UDP checksum and leave the inner TCP checksum
holding only the pseudo-header seed. Before this patch, the kernel
computed these checksums in software.

The skbuff.h description of NETIF_F_IPV6_CSUM covers only "unencapsulated
packets of the form IPv6|TCP or IPv6|UDP where the Next Header field in
the IPv6 header is either TCP or UDP."

gso_features_check() has the same gap. For non-encapsulated skbs, the old
branch removed NETIF_F_TSO6 and NETIF_F_GSO_UDP_L4 whenever the transport
header was not at nh + 40. The new code removes them only when the outer
nexthdr is an extension header.

The commit message talks about removing false positives, but it doesn't
mention this relaxation for non-encapsulated skbs.

Could the offset test stay for the !skb->encapsulation case? For example:

	skb_checksum_start_offset(skb) - skb_network_offset(skb) != sizeof(struct ipv6hdr)

Or could the check require nexthdr to be IPPROTO_TCP or IPPROTO_UDP,
instead of testing ipv6_ext_hdr()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924192128.1197118-1-willemdebruijn.kernel%40gmail.com

Re: [PATCH net] net: extend IPv6 exthdr detection of tunneled packets

[Willem de Bruijn]

netdev-bot+sashiko@ wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] Regression for non-encapsulated IPv6 CHECKSUM_PARTIAL skbs in
>   skb_csum_hwoffload_help(), plus a matching gap in…
> 
> --
> 
> net: extend IPv6 exthdr detection of tunneled packets
> 
> This patch replaces skb_gso_has_extension_hdr() with
> skb_has_ipv6_extension_hdr(). The new helper runs ipv6_ext_hdr() on the
> outer IPv6 nexthdr. For encapsulated skbs that have an inner network
> header, it also runs it on the inner IPv6 nexthdr. skb_csum_hwoffload_help()
> now uses the same helper instead of its open coded skb_network_header_len()
> test.
> 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index f660fccfc0dbc..992b78308c13f 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3818,20 +3818,29 @@ static netdev_features_t dflt_features_check(struct sk_buff *skb,
> >  	return vlan_features_check(skb, features);
> >  }
> >  
> > -static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
> > -{
> > -	if (!skb->encapsulation)
> > -		return ((skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
> > -			 (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
> > -			  vlan_get_protocol(skb) == htons(ETH_P_IPV6))) &&
> > -			skb_transport_header_was_set(skb) &&
> > -			skb_network_header_len(skb) != sizeof(struct ipv6hdr));
> 
> [ ... ]
> 
> > +static bool __skb_has_ipv6_ext_hdr(const struct sk_buff *skb, int nhoff)
> > +{
> > +	const struct ipv6hdr *ip6h;
> > +	struct ipv6hdr _ip6h;
> > +
> > +	ip6h = skb_header_pointer(skb, nhoff, sizeof(_ip6h), &_ip6h);
> > +	return ip6h && ip6h->version == 6 && ipv6_ext_hdr(ip6h->nexthdr);
> > +}
> > +
> > +static bool skb_has_ipv6_extension_hdr(const struct sk_buff *skb)
> > +{
> > +	if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
> > +	    __skb_has_ipv6_ext_hdr(skb, skb_network_offset(skb)))
> > +		return true;
> > +
> > +	/* Tunnels without an inner network header, such as SCTP-in-UDP or
> > +	 * PSP, have no inner IP header and thus no inner extension header.
> > +	 */
> > +	if (skb->encapsulation && skb_inner_network_header_was_set(skb) &&
> > +	    __skb_has_ipv6_ext_hdr(skb, skb_inner_network_offset(skb)))
> > +		return true;
> > +
> > +	return false;
> >  }
> 
> [ ... ]
> 
> > @@ -3988,8 +3997,7 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
> >  		return 0;
> >  
> >  	if (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) {
> > -		if (vlan_get_protocol(skb) == htons(ETH_P_IPV6) &&
> > -		    skb_network_header_len(skb) != sizeof(struct ipv6hdr))
> > +		if (skb_has_ipv6_extension_hdr(skb))
> >  			goto sw_checksum;
> 
> [Severity: High]
> Does this weaken the check for non-encapsulated IPv6 CHECKSUM_PARTIAL
> skbs?
> 
> When !skb->encapsulation, validate_xmit_skb() sets the transport header to
> csum_start just before it calls skb_csum_hwoffload_help():
> 
> net/core/dev.c:validate_xmit_skb() {
>     ...
> 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> 			if (skb->encapsulation)
> 				skb_set_inner_transport_header(skb,
> 							       skb_checksum_start_offset(skb));
> 			else
> 				skb_set_transport_header(skb,
> 							 skb_checksum_start_offset(skb));
> 			if (skb_csum_hwoffload_help(skb, features))
> 				goto out_kfree_skb;
>     ...
> }
> 
> That means the removed skb_network_header_len() test checked that
> csum_start sits right after the 40 byte IPv6 header.
> 
> The new helper only checks whether the outer nexthdr is one of the six
> values listed in ipv6_ext_hdr(). It skips the inner check because
> encapsulation is 0.
> 
> Take IPv6|UDP|VXLAN|Eth|IP|TCP, or IPv6|IPv6|TCP (nexthdr 41), with
> csum_start at the inner TCP header and csum_offset 16. Would that packet
> now reach the csum_offset switch and return 0?
> 
> Such skbs can come from untrusted input. __virtio_net_hdr_to_skb() is
> used by tap, vhost-net and AF_PACKET with PACKET_VNET_HDR. It accepts
> VIRTIO_NET_HDR_F_NEEDS_CSUM with any csum_start via
> skb_partial_csum_set(). It does not look at nexthdr and does not set
> skb->encapsulation.

This is correct. virtio_net_hdr can insert packets with tunnels,
without setting skb->encapsulation.

> One case: a guest sends IPv6 VXLAN traffic with a CHECKSUM_PARTIAL inner
> TCP header (LCO). The host bridges it to a device that has
> NETIF_F_IPV6_CSUM but not NETIF_F_HW_CSUM.
> 
> On a device that parses headers itself, such as bnxt, bnxt_exthdr_check()
> accepts an outer nexthdr of UDP for non-encapsulated skbs. The hardware
> would then fill in the outer UDP checksum and leave the inner TCP checksum
> holding only the pseudo-header seed. Before this patch, the kernel
> computed these checksums in software.
> 
> The skbuff.h description of NETIF_F_IPV6_CSUM covers only "unencapsulated
> packets of the form IPv6|TCP or IPv6|UDP where the Next Header field in
> the IPv6 header is either TCP or UDP."
> 
> gso_features_check() has the same gap. For non-encapsulated skbs, the old
> branch removed NETIF_F_TSO6 and NETIF_F_GSO_UDP_L4 whenever the transport
> header was not at nh + 40. The new code removes them only when the outer
> nexthdr is an extension header.
> 
> The commit message talks about removing false positives, but it doesn't
> mention this relaxation for non-encapsulated skbs.
> 
> Could the offset test stay for the !skb->encapsulation case? For example:
> 
> 	skb_checksum_start_offset(skb) - skb_network_offset(skb) != sizeof(struct ipv6hdr)

Will do in v2.

Reintroducing this check is the safest way to detect this edge case.

> Or could the check require nexthdr to be IPPROTO_TCP or IPPROTO_UDP,
> instead of testing ipv6_ext_hdr()?

Only checking two transport types is insufficient.

> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924192128.1197118-1-willemdebruijn.kernel%40gmail.com