[PATCH net] net: reset transport_header for ndo_features_check

[Matt Vollrath <tactii@gmail.com>]

ndo_features_check() can see a transport_header that does not point at
the L4 header, while ndo_start_xmit() for the same skb sees the right
one.

Commit 663ead3bb8d5 ("[NET]: Use csum_start offset instead of
skb_transport_header") made csum_start authoritative because receive
processing overwrites transport_header, and resets it from csum_start
"just before we push a packet with partial checksums off into the device
driver". That reset, which now sets inner_transport_header instead for
encapsulated skbs, sits in validate_xmit_skb(), on the non-GSO path
only, after netif_skb_features() has called ndo_features_check().

So the hook sees a wrong transport_header for a CHECKSUM_PARTIAL skb
(after GRO, or from a container or VM) when no stacked device (VLAN,
bond, tunnel) ran validate_xmit_skb() first and something earlier
pointed the header elsewhere:

* IPv6 forwarding with a destination options or routing header:
  ip6_rcv_core() sets it just past the fixed header, and only hop-by-hop
  options are stepped over before forwarding.
* br_nf_pre_routing_ipv6() does the same for bridged IPv6.
* seg6 inline, rpl and ioam6 point it at the header they insert.

A hook that bounds header lengths with transport_header can then leave
offloads enabled that the hardware cannot handle. For example,
i40e_features_check() limits IPLEN using skb_network_header_len(),
which returns 40, while i40e_tx_enable_csum() writes the real length
into the 7-bit IPLEN field, so more than 468 bytes of extension headers
overflow into L4_FC_LEN. Rather than make every driver measure from
csum_start, give the hook the headers the transmit path gets.

Move the reset above netif_skb_features(). Skbs that skip
skb_gso_segment() reach ndo_start_xmit() with the same values as
before, since VLAN insertion, the TLS device fallback (whose tcp_hdr()
now sees the reset header) and linearization keep csum_start and the
header offsets in step. Software segmentation parses the headers
itself, except for SCTP over UDP, where inner_transport_header already
equals csum_start. Encapsulated SKB_GSO_DODGY skbs that the device
segments now reach the driver with inner_transport_header at csum_start,
which TCP or UDP checked, rather than a value bpf_skb_adjust_room() or
bpf_lwt_push_ip_encap() may have copied from a stale transport_header.

Fixes: 5f35227ea34b ("net: Generalize ndo_gso_check to ndo_features_check")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Matt Vollrath <tactii@gmail.com>
---
If this is considered too expensive, I'll make a documentation patch
instead and the drivers can fix the holes as they please. Either way,
the core-driver contract needs improvement.

Drivers exposed, by reading the code (none tested on hardware):

Every stale source above involves IPv6 extension headers. Since commit
5f0764cb1c81 ("net: extend IPv6 exthdr detection of tunneled packets"),
the core keeps IPV6_CSUM, TSO6 and GSO_UDP_L4 off such skbs by reading
nexthdr, so on net only HW_CSUM devices are exposed:

- i40e: as described above.
- ice in GCS mode (E830): the same IPLEN check, and ice_tx_csum() also
  writes the real length unmasked into the 7-bit IPLEN field. GCS mode
  has no TSO.
- gve DQO: gve_can_send_tso() counts buffers per segment from
  skb_tcp_all_headers(), which has to match the header length that
  gve_prep_tso() programs from the reset header. Device effect unknown.
- hns3 with HW Tx checksum: removes offloads when the transport offset
  plus 60 exceeds its 480-byte header limit, which a stale offset can
  hide. Device effect unknown.

Outside drivers, the TLS device fallback reads tcp_hdr()->seq between
the two points, for TLS offload traffic over an SRv6 inline, rpl or
ioam6 route that falls back to software.

Not exposed: drivers that measure from csum_start (igb, igc, ixgbe,
fbnic and others), and on net the IPV6_CSUM drivers (r8169, stmmac,
iavf and others).

For stable: i40e is HW_CSUM since v5.19, hns3's HW Tx checksum since
v5.11, gve's check since v6.8 and ice GCS since v6.15. 6.1.y (as of
6.1.191) has commit 864e3396976e ("net: gso: Forbid IPv6 TSO with
extensions on devices with only IPV6_CSUM") but not 5f0764cb1c81, and
that guard reads transport_header. A stale header lets a TSO6 skb with
extension headers past it, and validate_xmit_skb() then WARNs and drops
the skb on any IPV6_CSUM device with TSO6. This patch fixes that as
well, though a backport of 5f0764cb1c81 is the targeted fix.

In QEMU, a kprobe on ndo_features_check() found transport_header
differing from csum_start in 18425 of 18427 skbs of a routed IPv6 TCP
flow with destination options, and in all 23300 of a TCP flow over an
SRv6 inline route. With this patch, it found none.

This surfaced while working on a new ndo_features_check()
implementation [1].

[1] https://lore.kernel.org/all/20261028170239.28606-1-tactii@gmail.com/
---
 net/core/dev.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index df1a840b03343..155be1f51494c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4207,6 +4207,15 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 	if (unlikely(!skb))
 		goto out_null;
 
+	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));
+	}
+
 	features = netif_skb_features(skb);
 	skb = validate_xmit_vlan(skb, features);
 	if (unlikely(!skb))
@@ -4239,16 +4248,9 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 		 * support checksumming for this protocol, complete
 		 * checksumming here.
 		 */
-		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;
-		}
+		if (skb->ip_summed == CHECKSUM_PARTIAL &&
+		    skb_csum_hwoffload_help(skb, features))
+			goto out_kfree_skb;
 	}
 
 	skb = validate_xmit_xfrm(skb, features, again);

base-commit: 03897066069fd47caf2908183edf3b22b822ea73
-- 
2.43.0