Re: [PATCH net v4] net: gro: Fix nesting of TCP GSO SKBs in skb_gro_receive_list()
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2026-08-19 23:01:38
Also in:
linux-arm-kernel, linux-mediatek
On Wed, Aug 19, 2026 at 5:37 AM [off-list ref] wrote:
From: HW He <redacted>
Perhaps a brief one sentence summary of the problem, before a detailed repro. For instance: Fraglist GRO plus hardware GRO can create an fraglist of HW-GRO packets. This cannot be segmented back into the original form. Avoid constructing such a GSO packet, by flushing an already built fraglist GRO packet if a hardware GRO packet arrives.
quoted hunk ↗ jump to hunk
A device supports GRO_HW, and the device driver enables the NETIF_F_GRO_FRAGLIST feature. During a tethering test, skb_gro_receive_list() reaggregates the GSO packet. However, skb_segment_list() cannot segment this packet back into the original packets, which leads to IP fragmentation or packet drop. Scenario (Tethering/Forwarding): 1.Driver submits a single TCP packet, P1. P1 is kept in the gro_list as the first packet. 2. The driver submits a TCP GSO skb, P2. P2 has already aggregated multiple TCP packets by HW_GRO, and its non-linear data is stored in frags[]. 3. P1 and P2 match the GRO rules, and since there is no local socket, they are aggregated by skb_gro_receive_list(). The resulting skb, P3, has a frag_list entry that still contains frags[]: P3: [ Linear Data ] -> frag_list -> [ Linear Data ] [ frag[1] ] [ frag[2] ] ... 4. Later, tcp4_gso_segment() or tcp6_gso_segment() calls skb_segment_list() to segment P3. However, skb_segment_list() only segments the entries in frag_list. It does not segment the frags[] inside P2, so P3 is not restored to the original packets, which leads to IP fragmentation or packet drop in the following path. Check skb_is_gso(skb) and current GRO method, make sure fraglist GRO applies to consecutive non-GSO skb, others adopt regular GRO path. Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO") Signed-off-by: Zhaoping Shu <redacted> Signed-off-by: HW He <redacted> --- [3]: https://patchwork.kernel.org/patch/14747095 [2]: https://patchwork.kernel.org/patch/14706032 [1]: https://patchwork.kernel.org/patch/14702209 --- net/ipv4/tcp_offload.c | 22 ++++++++++++++++------ net/ipv6/tcpv6_offload.c | 15 +++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-)diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index 3b1fdcd3cb29..e74d99ca9fac 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c@@ -332,6 +332,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb, flush |= skb->ip_summed != p->ip_summed; flush |= skb->csum_level != p->csum_level; flush |= NAPI_GRO_CB(p)->count >= 64; + flush |= NAPI_GRO_CB(p)->is_flist != NAPI_GRO_CB(skb)->is_flist;
Technically, is this one-line sufficient?
quoted hunk ↗ jump to hunk
skb_set_network_header(skb, skb_gro_receive_network_offset(skb)); if (flush || skb_gro_receive_list(p, skb))@@ -395,12 +396,20 @@ static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb, struct net *net; int iif, sdif; - if (likely(!(skb->dev->features & NETIF_F_GRO_FRAGLIST))) - return; - p = tcp_gro_lookup(head, th); if (p) { - NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist; + /* flist GRO applies to consecutive non-GSO skbs */ + if (!skb_is_gso(skb) || !NAPI_GRO_CB(p)->is_flist) { + NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist; + return; + } + + /* Fall back to the regular GRO path */ + if (NAPI_GRO_CB(p)->count == 1) + NAPI_GRO_CB(p)->is_flist = 0; + + NAPI_GRO_CB(skb)->is_flist = 0; +
This reduces the chance to have to flush, by downgrading fraglist GRO packets if a hardware GRO packet is detected at the stage (count == 1) where fraglist is not enabled in practice. This is quite a bit of complexity. And if hardware GRO is enabled, what are the odds in practice that the device does not produce any hardware GRO packets, yet the software GRO stack is capable of assembling signficant GSO packets. I suspect low. The solution is technically correct. But would simply disabling fraglist GRO when hardware GRO is enabled not be simpler? Or only keeping the flush and avoiding this micro-optimization.
quoted hunk ↗ jump to hunk
return; }@@ -410,7 +419,7 @@ static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb, sk = __inet_lookup_established(net, iph->saddr, th->source, iph->daddr, ntohs(th->dest), iif, sdif); - NAPI_GRO_CB(skb)->is_flist = !sk; + NAPI_GRO_CB(skb)->is_flist = !sk && !skb_is_gso(skb); if (sk) sock_gen_put(sk); }@@ -430,7 +439,8 @@ struct sk_buff *tcp4_gro_receive(struct list_head *head, struct sk_buff *skb) if (!th) goto flush; - tcp4_check_fraglist_gro(head, skb, th); + if (unlikely(skb->dev->features & NETIF_F_GRO_FRAGLIST)) + tcp4_check_fraglist_gro(head, skb, th); return tcp_gro_receive(head, skb, th);diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c index f2a659cd6183..eec3778855eb 100644 --- a/net/ipv6/tcpv6_offload.c +++ b/net/ipv6/tcpv6_offload.c@@ -26,7 +26,18 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb, p = tcp_gro_lookup(head, th); if (p) { - NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist; + /* flist GRO applies to consecutive non-GSO skbs */ + if (!skb_is_gso(skb) || !NAPI_GRO_CB(p)->is_flist) { + NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist; + return; + } + + /* Fall back to the regular GRO path */ + if (NAPI_GRO_CB(p)->count == 1) + NAPI_GRO_CB(p)->is_flist = 0; + + NAPI_GRO_CB(skb)->is_flist = 0; + return; }@@ -36,7 +47,7 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb, sk = __inet6_lookup_established(net, &hdr->saddr, th->source, &hdr->daddr, ntohs(th->dest), iif, sdif); - NAPI_GRO_CB(skb)->is_flist = !sk; + NAPI_GRO_CB(skb)->is_flist = !sk && !skb_is_gso(skb); if (sk) sock_gen_put(sk); #endif /* IS_ENABLED(CONFIG_IPV6) */ --2.17.0