[PATCH net v2] xfrm: esp: reset outer TCP GSO metadata on espintcp input

[PATCH net v2] xfrm: esp: reset outer TCP GSO metadata on espintcp input

[Guoshuai Yang]

When ESP packets are received over TCP (RFC 8229 espintcp), strparser
slices individual length-prefixed ESP records from the outer TCP stream by
cloning the incoming TCP skb(s) in __strp_recv().

If the outer TCP skb was GRO-coalesced before reaching strparser, its
skb_shinfo carries the outer TCP stream's GSO metadata (gso_size, gso_segs,
and gso_type). When an ESP record spans two outer TCP segments, strparser
chains the spillover clone onto skb_shinfo(head)->frag_list.

Subsequently in esp_input() and esp6_input(), skb_cow_data() unshares the
head skb via pskb_expand_head() (preserving the outer TCP gso_size and
gso_type in the new private skb_shinfo) and calls skb_copy() on each cloned
frag_list member, which copies the outer TCP GSO fields via
skb_copy_header() onto a linear frag_list child skb allocated from slab
with head_frag == 0.

After ESP decryption and header removal, the resulting single inner packet
still has skb_is_gso(skb) == true with the stale outer TCP gso_size, while
skb_headlen(head_skb) is now smaller than that gso_size due to stripping
the outer framing and ESP header. When this packet passes through
gro_cells_receive() (which flushes frag_list packets via GRO_NORMAL
without resetting gso_size) and is forwarded, validate_xmit_skb() invokes
tcp_gso_segment() -> skb_segment(). Because head_skb is shorter than the
stale gso_size and SKB_GSO_DODGY is not set, skb_segment() attempts to map
the slab-backed frag_list member as a scatter-gather page fragment and hits
BUG_ON(!list_skb->head_frag):

  Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
  PC is at skb_segment+0x574/0xd20
  LR is at skb_segment+0x884/0xd20
  Call trace:
   skb_segment from tcp_gso_segment
   tcp_gso_segment from inet_gso_segment
   inet_gso_segment from skb_mac_gso_segment
   skb_mac_gso_segment from __skb_gso_segment
   __skb_gso_segment from validate_xmit_skb
   validate_xmit_skb from __dev_queue_xmit
   __dev_queue_xmit from ip_finish_output2
   ip_finish_output2 from ip_output
   ip_output from ip_sublist_rcv_finish
   ip_sublist_rcv_finish from ip_sublist_rcv
   ip_list_rcv from __netif_receive_skb_list_core
   __netif_receive_skb_list_core from __netif_receive_skb_list
   __netif_receive_skb_list from gro_normal_list
   gro_normal_list from gro_cell_poll

Furthermore, if a decrypted inner packet with a frag_list later takes the
ip_do_fragment() frag_list fast path, each detached frag_list member is
transmitted as a standalone IP fragment and would still carry the stale
outer TCP GSO state when reaching validate_xmit_skb(). And even when an
ESP record does not have a frag_list, leaving the stale outer TCP gso_size
on the decrypted inner skb prevents dev_gro_receive() from initializing
gso_size to the inner packet's skb_gro_len().

We cannot reset GSO state earlier in espintcp_rcv() / handle_esp() because
the skb is still a clone sharing skb_shinfo with the outer TCP skb held by
strparser. Instead, call skb_gso_reset() on both the head skb and each
skb_walk_frags() member at skip_cow in esp_input() and esp6_input(), where
the head skb and all frag_list members are guaranteed to be uncloned with
private skb_shinfo.

Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Cc: stable@vger.kernel.org
Signed-off-by: Guoshuai Yang <yangguoshuai@gmail.com>
---
v2:
  - Also call skb_gso_reset() on each skb_walk_frags() member at skip_cow
    in esp_input() and esp6_input() so detached frag_list members on the
    ip_do_fragment() fast path do not retain stale outer TCP GSO state.
  - Specify target tree 'net' in subject prefix.
  - Link to v1: https://lore.kernel.org/netdev/20261018005218.53267-1-yangguoshuai@gmail.com/
 net/ipv4/esp4.c | 8 ++++++++
 net/ipv6/esp6.c | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index e76db58..012b59f 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -900,6 +900,14 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 	nfrags = err;
 
 skip_cow:
+	if (x->encap && x->encap->encap_type == TCP_ENCAP_ESPINTCP) {
+		struct sk_buff *iter;
+
+		skb_gso_reset(skb);
+		skb_walk_frags(skb, iter)
+			skb_gso_reset(iter);
+	}
+
 	err = -ENOMEM;
 	tmp = esp_alloc_tmp(aead, nfrags, seqhilen);
 	if (!tmp)
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b1c9b36..b08cc1c 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -943,6 +943,14 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 	}
 
 skip_cow:
+	if (x->encap && x->encap->encap_type == TCP_ENCAP_ESPINTCP) {
+		struct sk_buff *iter;
+
+		skb_gso_reset(skb);
+		skb_walk_frags(skb, iter)
+			skb_gso_reset(iter);
+	}
+
 	ret = -ENOMEM;
 	tmp = esp_alloc_tmp(aead, nfrags, seqhilen);
 	if (!tmp)
-- 
2.54.0 (Apple Git-157)

Re: [PATCH net v2] xfrm: esp: reset outer TCP GSO metadata on espintcp input

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · 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).

- [Medium] Problem: outer TCP GRO metadata can still break espintcp input
  when the outer GRO aggregate carries SKB_GSO_FRAGLIST.

--

xfrm: esp: reset outer TCP GSO metadata on espintcp input

On espintcp input, this patch clears stale outer TCP GSO metadata. It
calls skb_gso_reset() on the head skb and on each frag_list member at the
skip_cow label in esp_input() and esp6_input(). This is meant to stop the
decrypted inner packet from reaching skb_segment() with outer GSO state
and hitting BUG_ON(!list_skb->head_frag).

> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index e76db5817e78..012b59f69292 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -900,6 +900,14 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>  	nfrags = err;
>  
>  skip_cow:
> +	if (x->encap && x->encap->encap_type == TCP_ENCAP_ESPINTCP) {
> +		struct sk_buff *iter;
> +
> +		skb_gso_reset(skb);
> +		skb_walk_frags(skb, iter)
> +			skb_gso_reset(iter);
> +	}

[Severity: Medium]
Is this reset reached when the outer TCP GRO aggregate carries
SKB_GSO_FRAGLIST? It only runs after skb_cow_data() succeeds, and
skb_cow_data() can fail on that gso_type before we get here.

tcp4_gro_complete() sets this on fraglist GRO aggregates:

    skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV4;

tcp4_check_fraglist_gro() chooses fraglist GRO when it finds no socket in
the ingress device's netns. That happens when rx-gro-list is enabled and
the espintcp socket is in another netns reached through veth or a bridge.
veth passes the aggregate on intact, and TCP queues it unsegmented.

When an ESP record spans such an aggregate, __strp_recv() makes it a
frag_list child of the message head and keeps the FRAGLIST gso_type. This
happens in one of two ways:

- skb_unclone() on an appended clone that has its own frag_list
- moving an old head that has a frag_list under a newly allocated head

esp_input() then takes the skb_cow_data() path, and for that child:

net/core/skbuff.c:skb_cow_data() {
    ...
		if (copyflag ||
		    skb_cloned(skb1) ||
		    ntail ||
		    skb_shinfo(skb1)->nr_frags ||
		    skb_has_frag_list(skb1)) {
			...
			if (ntail == 0)
				skb2 = skb_copy(skb1, GFP_ATOMIC);
			...
			if (unlikely(skb2 == NULL))
				return -ENOMEM;
    ...
}

net/core/skbuff.c:skb_copy() {
    ...
	if (WARN_ON_ONCE(skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST))
		return NULL;
    ...
}

esp_input() then takes the goto out before skip_cow, so neither reset
runs. The result is a WARN_ON_ONCE() splat, which panics with
panic_on_warn. Each valid ESP-in-TCP record like this is also dropped.
All of this happens before ESP authentication, on data that the TCP peer
controls.

esp6_input() has the same ordering:

	nfrags = skb_cow_data(skb, 0, &trailer);
	if (nfrags < 0) {
		ret = -EINVAL;
		goto out;
	}

The commit message says that at skip_cow:

    the head skb and all frag_list members are guaranteed to be uncloned
    with private skb_shinfo.

In this case skb_cow_data() fails before skip_cow is reached.

The skb_copy() WARN_ON_ONCE() itself is older than this patch. This is
still the same stale outer TCP GRO metadata problem the patch sets out to
fix, and a reset placed after skb_cow_data() can't cover it. Should the
outer GSO state be handled before or inside the COW step instead?

[ ... ]

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