* [PATCH net] gro: do not GRO GSO packets with inconsistent payload
@ 2026-10-28 17:13 Eric Dumazet
2026-10-28 17:15 ` netdev-bot+sinfo
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-10-28 17:13 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Willem de Bruijn, netdev, edumazet, Kyle Zeng
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload
2026-10-28 17:13 [PATCH net] gro: do not GRO GSO packets with inconsistent payload Eric Dumazet
@ 2026-10-28 17:15 ` netdev-bot+sinfo
2026-10-28 23:33 ` Willem de Bruijn
2026-10-29 16:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sinfo @ 2026-10-28 17:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Willem de Bruijn, netdev, Kyle Zeng
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload
2026-10-28 17:13 [PATCH net] gro: do not GRO GSO packets with inconsistent payload Eric Dumazet
2026-10-28 17:15 ` netdev-bot+sinfo
@ 2026-10-28 23:33 ` Willem de Bruijn
2026-10-29 6:08 ` Eric Dumazet
2026-10-29 16:20 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2026-10-28 23:33 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Willem de Bruijn, netdev, edumazet, Kyle Zeng
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload
2026-10-28 23:33 ` Willem de Bruijn
@ 2026-10-29 6:08 ` Eric Dumazet
2026-10-29 14:15 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-10-29 6:08 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Willem de Bruijn, netdev, Kyle Zeng
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload
2026-10-29 6:08 ` Eric Dumazet
@ 2026-10-29 14:15 ` Willem de Bruijn
0 siblings, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2026-10-29 14:15 UTC (permalink / raw)
To: Eric Dumazet, Willem de Bruijn
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Willem de Bruijn, netdev, Kyle Zeng
Eric Dumazet wrote:
> 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.
Should those get fixed instead (eventually).
> 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.
Argh. Good point. In that case:
Reviewed-by: Willem de Bruijn <willemb@google.com>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] gro: do not GRO GSO packets with inconsistent payload
2026-10-28 17:13 [PATCH net] gro: do not GRO GSO packets with inconsistent payload Eric Dumazet
2026-10-28 17:15 ` netdev-bot+sinfo
2026-10-28 23:33 ` Willem de Bruijn
@ 2026-10-29 16:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-29 16:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, kuba, pabeni, horms, willemb, netdev, kylebot
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 28 Oct 2026 18:13:50 +0100 you 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.
>
> [...]
Here is the summary with links:
- [net] gro: do not GRO GSO packets with inconsistent payload
https://git.kernel.org/netdev/net/c/de788d8a7032
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-29 16:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 17:13 [PATCH net] gro: do not GRO GSO packets with inconsistent payload Eric Dumazet
2026-10-28 17:15 ` netdev-bot+sinfo
2026-10-28 23:33 ` Willem de Bruijn
2026-10-29 6:08 ` Eric Dumazet
2026-10-29 14:15 ` Willem de Bruijn
2026-10-29 16:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox