[PATCH net] gro: do not GRO GSO packets with inconsistent payload

[PATCH net] gro: do not GRO GSO packets with inconsistent payload

[Eric Dumazet]

From: Kyle Zeng <kylebot@openai.com>

tcp_gro_receive() excludes empty non-GSO packets with the unsigned
len - 1 check, but its GSO branch only checks that gso_size matches
the held packet's MSS. A trusted TCP GSO skb can reach GRO again with
no payload after TC pedit increases its TCP data offset.

If the skb is fully linear, skb_gro_receive() then enters its fragment
transfer path with nr_frags == 0. The do-while loop copies skb_frag_t
entries backwards beyond both fragment arrays. An unprivileged process
using its own user and network namespaces can trigger a KASAN
slab-out-of-bounds write and panic through veth, pedit and GRE.

More generally, the payload of such a packet can be shrunk to any
value, making it inconsistent with its gso_segs, and a GSO packet
with gso_segs == 0 contributes no segment to the GRO packet.

Do not aggregate GSO packets with gso_segs == 0, and require the
payload of GSO packets to be consistent with gso_segs and gso_size,
the last segment being allowed to be short. This rejects the empty
skb before either TCP merge helper, and covers both IPv4 and IPv6.

Fixes: 5eddb24901ee ("gro: add support of (hw)gro packets to gro stack")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
[edumazet: check payload against gso_segs, refuse gso_segs == 0]
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/core/gro.c         |  5 +++--
 net/ipv4/tcp_offload.c | 12 +++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/core/gro.c b/net/core/gro.c
index ff828a3dce3ca649e149a840bd6e3dfd9ddaf0c4..1b033855a20db6b8b996179f0b81e38393d3864a 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -513,9 +513,10 @@ static enum gro_result dev_gro_receive(struct gro_node *gro,
 	NAPI_GRO_CB(skb)->count = 1;
 	if (unlikely(skb_is_gso(skb))) {
 		NAPI_GRO_CB(skb)->count = skb_shinfo(skb)->gso_segs;
-		/* Only support TCP and non DODGY users. */
+		/* Only support TCP and non DODGY users, with a sane gso_segs. */
 		if (!skb_is_gso_tcp(skb) ||
-		    (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY))
+		    (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) ||
+		    !NAPI_GRO_CB(skb)->count)
 			NAPI_GRO_CB(skb)->flush = 1;
 	}
 
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index e74d99ca9face6ecb853e928b538bd66a377038c..b84fcd07c7230fc3230fdbc3a79d68d1e79b058e 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -315,14 +315,20 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb,
 
 	mss = skb_shinfo(p)->gso_size;
 
-	/* If skb is a GRO packet, make sure its gso_size matches prior packet mss.
+	/* If skb is a GRO packet, make sure its gso_size matches prior packet
+	 * mss, and that its payload is consistent with its gso_segs
+	 * (the last segment can be short).
 	 * If it is a single frame, do not aggregate it if its length
 	 * is bigger than our mss.
 	 */
-	if (unlikely(skb_is_gso(skb)))
+	if (unlikely(skb_is_gso(skb))) {
+		unsigned int segs = NAPI_GRO_CB(skb)->count;
+
 		flush |= (mss != skb_shinfo(skb)->gso_size);
-	else
+		flush |= len <= (segs - 1) * mss || len > segs * mss;
+	} else {
 		flush |= (len - 1) >= mss;
+	}
 
 	flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq);
 	flush |= skb_cmp_decrypted(p, skb);
-- 
2.53.0

Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload

[Willem de Bruijn]

Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> tcp_gro_receive() excludes empty non-GSO packets with the unsigned
> len - 1 check, but its GSO branch only checks that gso_size matches
> the held packet's MSS. A trusted TCP GSO skb can reach GRO again with
> no payload after TC pedit increases its TCP data offset.
> 
> If the skb is fully linear, skb_gro_receive() then enters its fragment
> transfer path with nr_frags == 0. The do-while loop copies skb_frag_t
> entries backwards beyond both fragment arrays. An unprivileged process
> using its own user and network namespaces can trigger a KASAN
> slab-out-of-bounds write and panic through veth, pedit and GRE.
> 
> More generally, the payload of such a packet can be shrunk to any
> value, making it inconsistent with its gso_segs, and a GSO packet
> with gso_segs == 0 contributes no segment to the GRO packet.
> 
> Do not aggregate GSO packets with gso_segs == 0, and require the
> payload of GSO packets to be consistent with gso_segs and gso_size,
> the last segment being allowed to be short. This rejects the empty
> skb before either TCP merge helper, and covers both IPv4 and IPv6.
> 
> Fixes: 5eddb24901ee ("gro: add support of (hw)gro packets to gro stack")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> [edumazet: check payload against gso_segs, refuse gso_segs == 0]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
>  net/core/gro.c         |  5 +++--
>  net/ipv4/tcp_offload.c | 12 +++++++++---
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/gro.c b/net/core/gro.c
> index ff828a3dce3ca649e149a840bd6e3dfd9ddaf0c4..1b033855a20db6b8b996179f0b81e38393d3864a 100644
> --- a/net/core/gro.c
> +++ b/net/core/gro.c
> @@ -513,9 +513,10 @@ static enum gro_result dev_gro_receive(struct gro_node *gro,
>  	NAPI_GRO_CB(skb)->count = 1;
>  	if (unlikely(skb_is_gso(skb))) {
>  		NAPI_GRO_CB(skb)->count = skb_shinfo(skb)->gso_segs;
> -		/* Only support TCP and non DODGY users. */
> +		/* Only support TCP and non DODGY users, with a sane gso_segs. */
>  		if (!skb_is_gso_tcp(skb) ||
> -		    (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY))
> +		    (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) ||
> +		    !NAPI_GRO_CB(skb)->count)

Or make tc_pedit set SKB_GSO_DODGY? BPF already does.

In pedit_l4_skb_offset and pedit_l4_skb_offset.

Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload

[Eric Dumazet]

Le jeu. 29 oct. 2026 à 00:33, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> a écrit :
>
> Eric Dumazet wrote:
> > From: Kyle Zeng <kylebot@openai.com>
> >
> > tcp_gro_receive() excludes empty non-GSO packets with the unsigned
> > len - 1 check, but its GSO branch only checks that gso_size matches
> > the held packet's MSS. A trusted TCP GSO skb can reach GRO again with
> > no payload after TC pedit increases its TCP data offset.
> >
> > If the skb is fully linear, skb_gro_receive() then enters its fragment
> > transfer path with nr_frags == 0. The do-while loop copies skb_frag_t
> > entries backwards beyond both fragment arrays. An unprivileged process
> > using its own user and network namespaces can trigger a KASAN
> > slab-out-of-bounds write and panic through veth, pedit and GRE.
> >
> > More generally, the payload of such a packet can be shrunk to any
> > value, making it inconsistent with its gso_segs, and a GSO packet
> > with gso_segs == 0 contributes no segment to the GRO packet.
> >
> > Do not aggregate GSO packets with gso_segs == 0, and require the
> > payload of GSO packets to be consistent with gso_segs and gso_size,
> > the last segment being allowed to be short. This rejects the empty
> > skb before either TCP merge helper, and covers both IPv4 and IPv6.
> >
> > Fixes: 5eddb24901ee ("gro: add support of (hw)gro packets to gro stack")
> > Signed-off-by: Kyle Zeng <kylebot@openai.com>
> > [edumazet: check payload against gso_segs, refuse gso_segs == 0]
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> > ---
> >  net/core/gro.c         |  5 +++--
> >  net/ipv4/tcp_offload.c | 12 +++++++++---
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/core/gro.c b/net/core/gro.c
> > index ff828a3dce3ca649e149a840bd6e3dfd9ddaf0c4..1b033855a20db6b8b996179f0b81e38393d3864a 100644
> > --- a/net/core/gro.c
> > +++ b/net/core/gro.c
> > @@ -513,9 +513,10 @@ static enum gro_result dev_gro_receive(struct gro_node *gro,
> >       NAPI_GRO_CB(skb)->count = 1;
> >       if (unlikely(skb_is_gso(skb))) {
> >               NAPI_GRO_CB(skb)->count = skb_shinfo(skb)->gso_segs;
> > -             /* Only support TCP and non DODGY users. */
> > +             /* Only support TCP and non DODGY users, with a sane gso_segs. */
> >               if (!skb_is_gso_tcp(skb) ||
> > -                 (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY))
> > +                 (skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) ||
> > +                 !NAPI_GRO_CB(skb)->count)
>
> Or make tc_pedit set SKB_GSO_DODGY? BPF already does.
>
> In pedit_l4_skb_offset and pedit_l4_skb_offset.

Good idea, this would also make sure gso_segs is recomputed if
such packets are later segmented.

I would still keep the !count check: ixgbe and libwx RSC packets reach
GRO with gso_segs == 0, and the payload check in tcp_gro_receive()
relies on count being non zero.

pedit is also not the only way to change the TCP header of a GSO
packet (nft payload on older kernels, NFQUEUE with
NFQA_CFG_F_GSO, bpf_skb_store_bytes()), so I prefer GRO to stay
robust anyway.

pedit can be changed later in net-next to set SKB_GSO_DODGY on
GSO packets it modifies. Note this needs skb_unclone(), which can
fail. pedit does not drop packets on allocation errors today, so
it would probably leave the packet unmarked, another reason for
GRO to not trust these packets.

Thanks.