[PATCH net] udp: check frag_list IP family before using skb_segment_list()

[PATCH net] udp: check frag_list IP family before using skb_segment_list()

[Eric Dumazet]

From: Kyle Zeng <kylebot@openai.com>

UDP fraglist GRO uses SKB_GSO_UDP_L4 for both IPv4 and IPv6.

__udp_gso_segment() reuses the frag_list skbs when the geometry of
the head skb was not modified. skb_segment_list() keeps the network
header of each frag_list skb as it was at GRO time and only adjusts
the transport header offset.

Nothing guarantees that the head skb still has the IP family of the
frag_list skbs: helpers like skb_mpls_pop() let the caller choose
the new protocol. In that case __udpv6_gso_segment_list_csum()
handles the 20 byte IPv4 header of each frag_list skb as a struct
ipv6hdr, and can write IPv6 addresses beyond its linear part.

Check the IP version of the first frag_list skb. All skbs in the
list belong to the same flow, so they share the same IP family.
Use the skb_segment() fallback on a mismatch. Regular fraglist GRO
traffic keeps the fast path.

Fixes: b936a9b8d4a5 ("net: ipv6: fix UDPv6 GSO segmentation with NAT")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
[edumazet: simplified the fix, rewrote the comment and the changelog]
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
This was found by an AI code review agent.
The issue was not triggered, no reproducer was provided.
The fix was tested with tools/testing/selftests/net/udpgro_fwd.sh.

 net/ipv4/udp_offload.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index cf07c3c6611a22c29b57a72420f2f940609f5e9a..802365b4780fdf7dc19ea7374d51d713e4eb736a 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -514,9 +514,14 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 	}
 
 	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
-		 /* Detect modified geometry and pass those to skb_segment. */
+		/* Detect modified geometry and pass those to skb_segment.
+		 * SKB_GSO_UDP_L4 is used by both IPv4 and IPv6: also make sure
+		 * the frag_list skbs still have the IP family we segment for,
+		 * as their network headers are not rebuilt.
+		 */
 		if ((skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size) &&
-		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY))
+		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY) &&
+		    ip_hdr(skb_shinfo(gso_skb)->frag_list)->version == (is_ipv6 ? 6 : 4))
 			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
 
 		ret = __skb_linearize(gso_skb);
-- 
2.53.0

Re: [PATCH net] udp: check frag_list IP family before using skb_segment_list()

[Willem de Bruijn]

Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> UDP fraglist GRO uses SKB_GSO_UDP_L4 for both IPv4 and IPv6.
> 
> __udp_gso_segment() reuses the frag_list skbs when the geometry of
> the head skb was not modified. skb_segment_list() keeps the network
> header of each frag_list skb as it was at GRO time and only adjusts
> the transport header offset.
> 
> Nothing guarantees that the head skb still has the IP family of the
> frag_list skbs: helpers like skb_mpls_pop() let the caller choose
> the new protocol. In that case __udpv6_gso_segment_list_csum()
> handles the 20 byte IPv4 header of each frag_list skb as a struct
> ipv6hdr, and can write IPv6 addresses beyond its linear part.
> 
> Check the IP version of the first frag_list skb. All skbs in the
> list belong to the same flow, so they share the same IP family.
> Use the skb_segment() fallback on a mismatch. Regular fraglist GRO
> traffic keeps the fast path.
> 
> Fixes: b936a9b8d4a5 ("net: ipv6: fix UDPv6 GSO segmentation with NAT")

optionally older

Fixes: c3df39ac9b0e ("udp: ipv4: manipulate network header of NATed UDP GRO fraglist")

> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> [edumazet: simplified the fix, rewrote the comment and the changelog]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
> This was found by an AI code review agent.
> The issue was not triggered, no reproducer was provided.
> The fix was tested with tools/testing/selftests/net/udpgro_fwd.sh.
> 
>  net/ipv4/udp_offload.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a22c29b57a72420f2f940609f5e9a..802365b4780fdf7dc19ea7374d51d713e4eb736a 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -514,9 +514,14 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  	}
>  
>  	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
> -		 /* Detect modified geometry and pass those to skb_segment. */
> +		/* Detect modified geometry and pass those to skb_segment.
> +		 * SKB_GSO_UDP_L4 is used by both IPv4 and IPv6: also make sure
> +		 * the frag_list skbs still have the IP family we segment for,
> +		 * as their network headers are not rebuilt.
> +		 */
>  		if ((skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size) &&
> -		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY))
> +		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY) &&
> +		    ip_hdr(skb_shinfo(gso_skb)->frag_list)->version == (is_ipv6 ? 6 : 4))
>  			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
>  
>  		ret = __skb_linearize(gso_skb);
> -- 
> 2.53.0
> 

Re: [PATCH net] udp: check frag_list IP family before using skb_segment_list()

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 1

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).

- [High] The fix in net/ipv4/udp_offload.c:__udp_gso_segment() is
  incomplete.
- [Low] The comment added in net/ipv4/udp_offload.c:__udp_gso_segment()
  (lines 517-521) says the code makes sure "the frag_list skbs still…

--

udp: check frag_list IP family before using skb_segment_list()

This patch adds a check to __udp_gso_segment(). The fraglist fast path,
__udp_gso_segment_list(), is now taken only when the IP version of the
first frag_list skb matches the family being segmented. On a mismatch
the code falls back to __skb_linearize() and skb_segment(), so that
__udpv6_gso_segment_list_csum() does not treat an IPv4 header as a
struct ipv6hdr.

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a2..802365b4780fd 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -514,9 +514,14 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  	}
>  
>  	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
> -		 /* Detect modified geometry and pass those to skb_segment. */
> +		/* Detect modified geometry and pass those to skb_segment.
> +		 * SKB_GSO_UDP_L4 is used by both IPv4 and IPv6: also make sure
> +		 * the frag_list skbs still have the IP family we segment for,
> +		 * as their network headers are not rebuilt.
> +		 */

[Severity: Low]
This isn't a bug, but the comment says the code makes sure "the frag_list
skbs still have the IP family we segment for". The condition below only
looks at skb_shinfo(gso_skb)->frag_list, which is the first member.

The idea that every member has the same family as the first one is only
stated in the changelog. Could the comment say that only the first
frag_list skb is checked, and why that is enough?

>  		if ((skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size) &&
> -		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY))
> +		    !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY) &&
> +		    ip_hdr(skb_shinfo(gso_skb)->frag_list)->version == (is_ipv6 ? 6 : 4))
>  			return __udp_gso_segment_list(gso_skb, features, is_ipv6);

[Severity: High]
Is it enough to check only the first frag_list skb here? The changelog
says "All skbs in the list belong to the same flow, so they share the
same IP family". On devices with hard_header_len == 0, though, GRO seems
able to append an IPv4 skb to an IPv6 head.

In gro_list_prepare(), maclen comes from skb->dev->hard_header_len. For
tun in IFF_TUN mode and for wireguard this is 0, so the memcmp() compares
nothing. skb->protocol is never compared:

net/core/gro.c:gro_list_prepare() {
    unsigned int maclen = skb->dev->hard_header_len;
    ...
		else if (!diffs)
			diffs = memcmp(skb_mac_header(p),
				       skb_mac_header(skb),
				       maclen);
    ...
}

inet_gro_receive() then checks only the version of the new skb. When p
is an IPv6 skb, it reads p at IPv4 offsets and never checks
iph2->version:

net/ipv4/af_inet.c:inet_gro_receive() {
    ...
		iph2 = (struct iphdr *)(p->data + off);
    ...
		if ((iph->protocol ^ iph2->protocol) |
		    ((__force u32)iph->saddr ^ (__force u32)iph2->saddr) |
		    ((__force u32)iph->daddr ^ (__force u32)iph2->daddr)) {
    ...
}

On an IPv6 head these bytes are saddr[1] and saddr[4..11], and the
sender chooses them. The reverse direction is blocked, because
ipv6_gro_receive() masks first_word with 0xF00FFFFF, and that mask
covers the version nibble.

udp_gro_receive_segment() checks ports, is_flist, ip_summed/csum_level
and gro_receive_network_flush(). It then calls skb_gro_receive_list(p,
skb), which does not check the family.

So an IPv6 head A could end up with frag_list = [B (IPv6), C (IPv4)].
The new check passes because B is version 6. skb_segment_list() keeps
C's own 20 byte IPv4 header and only adjusts the transport header.

If the head's addresses or ports were changed, for example by NAT66, tc
pedit or BPF, the early return in __udpv6_gso_segment_list_csum() is not
taken. The loop then treats C as IPv6:

net/ipv4/udp_offload.c:__udpv6_gso_segment_list_csum() {
    ...
	while ((seg = seg->next)) {
		uh2 = udp_hdr(seg);
		iph2 = ipv6_hdr(seg);

		__udpv6_gso_segment_csum(seg, &iph2->saddr, &iph->saddr,
					 &uh2->source, uh->source);
		__udpv6_gso_segment_csum(seg, &iph2->daddr, &iph->daddr,
					 &uh2->dest, uh->dest);
	}
    ...
}

The *oldip = *newip stores in __udpv6_gso_segment_csum() write offsets
8-23 and 24-39 of C's network header. pskb_may_pull(skb,
skb_gro_offset(skb)) only guarantees 28 bytes are linear, up to the end
of the UDP header. That allows a write of up to 12 bytes past the linear
part, into tailroom or skb_shared_info.

With rx-gro-list enabled, a tun device in an unprivileged user namespace
appears to be enough to reach this path. Remote peers on wireguard or
point-to-point tunnels may also be able to send such packets.

Isn't this the same write past the linear part that the patch is meant
to fix? Would it be better to check every frag_list member here? Another
option is to block cross-family merges in GRO, for example by checking
p's IP version in inet_gro_receive().

>  
>  		ret = __skb_linearize(gso_skb);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106141727.2303218-1-edumazet%40kernel.org

Re: [PATCH net] udp: check frag_list IP family before using skb_segment_list()

[Eric Dumazet]

Le mar. 10 nov. 2026 à 15:34, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 1
>
> 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).
>
> - [High] The fix in net/ipv4/udp_offload.c:__udp_gso_segment() is
>   incomplete.
> - [Low] The comment added in net/ipv4/udp_offload.c:__udp_gso_segment()
>   (lines 517-521) says the code makes sure "the frag_list skbs still…
>
> --
>
> udp: check frag_list IP family before using skb_segment_list()
>
> This patch adds a check to __udp_gso_segment(). The fraglist fast path,
> __udp_gso_segment_list(), is now taken only when the IP version of the
> first frag_list skb matches the family being segmented. On a mismatch
> the code falls back to __skb_linearize() and skb_segment(), so that
> __udpv6_gso_segment_list_csum() does not treat an IPv4 header as a
> struct ipv6hdr.
>
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index cf07c3c6611a2..802365b4780fd 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -514,9 +514,14 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> >       }
> >
> >       if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
> > -              /* Detect modified geometry and pass those to skb_segment. */
> > +             /* Detect modified geometry and pass those to skb_segment.
> > +              * SKB_GSO_UDP_L4 is used by both IPv4 and IPv6: also make sure
> > +              * the frag_list skbs still have the IP family we segment for,
> > +              * as their network headers are not rebuilt.
> > +              */
>
> [Severity: Low]
> This isn't a bug, but the comment says the code makes sure "the frag_list
> skbs still have the IP family we segment for". The condition below only
> looks at skb_shinfo(gso_skb)->frag_list, which is the first member.
>
> The idea that every member has the same family as the first one is only
> stated in the changelog. Could the comment say that only the first
> frag_list skb is checked, and why that is enough?
>
> >               if ((skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size) &&
> > -                 !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY))
> > +                 !(skb_shinfo(gso_skb)->gso_type & SKB_GSO_DODGY) &&
> > +                 ip_hdr(skb_shinfo(gso_skb)->frag_list)->version == (is_ipv6 ? 6 : 4))
> >                       return __udp_gso_segment_list(gso_skb, features, is_ipv6);
>
> [Severity: High]
> Is it enough to check only the first frag_list skb here? The changelog
> says "All skbs in the list belong to the same flow, so they share the
> same IP family". On devices with hard_header_len == 0, though, GRO seems
> able to append an IPv4 skb to an IPv6 head.
>
> In gro_list_prepare(), maclen comes from skb->dev->hard_header_len. For
> tun in IFF_TUN mode and for wireguard this is 0, so the memcmp() compares
> nothing. skb->protocol is never compared:
>
> net/core/gro.c:gro_list_prepare() {
>     unsigned int maclen = skb->dev->hard_header_len;
>     ...
>                 else if (!diffs)
>                         diffs = memcmp(skb_mac_header(p),
>                                        skb_mac_header(skb),
>                                        maclen);
>     ...
> }
>
> inet_gro_receive() then checks only the version of the new skb. When p
> is an IPv6 skb, it reads p at IPv4 offsets and never checks
> iph2->version:
>
> net/ipv4/af_inet.c:inet_gro_receive() {
>     ...
>                 iph2 = (struct iphdr *)(p->data + off);
>     ...
>                 if ((iph->protocol ^ iph2->protocol) |
>                     ((__force u32)iph->saddr ^ (__force u32)iph2->saddr) |
>                     ((__force u32)iph->daddr ^ (__force u32)iph2->daddr)) {
>     ...
> }
>
> On an IPv6 head these bytes are saddr[1] and saddr[4..11], and the
> sender chooses them. The reverse direction is blocked, because
> ipv6_gro_receive() masks first_word with 0xF00FFFFF, and that mask
> covers the version nibble.
>
> udp_gro_receive_segment() checks ports, is_flist, ip_summed/csum_level
> and gro_receive_network_flush(). It then calls skb_gro_receive_list(p,
> skb), which does not check the family.
>
> So an IPv6 head A could end up with frag_list = [B (IPv6), C (IPv4)].
> The new check passes because B is version 6. skb_segment_list() keeps
> C's own 20 byte IPv4 header and only adjusts the transport header.
>
> If the head's addresses or ports were changed, for example by NAT66, tc
> pedit or BPF, the early return in __udpv6_gso_segment_list_csum() is not
> taken. The loop then treats C as IPv6:
>
> net/ipv4/udp_offload.c:__udpv6_gso_segment_list_csum() {
>     ...
>         while ((seg = seg->next)) {
>                 uh2 = udp_hdr(seg);
>                 iph2 = ipv6_hdr(seg);
>
>                 __udpv6_gso_segment_csum(seg, &iph2->saddr, &iph->saddr,
>                                          &uh2->source, uh->source);
>                 __udpv6_gso_segment_csum(seg, &iph2->daddr, &iph->daddr,
>                                          &uh2->dest, uh->dest);
>         }
>     ...
> }
>
> The *oldip = *newip stores in __udpv6_gso_segment_csum() write offsets
> 8-23 and 24-39 of C's network header. pskb_may_pull(skb,
> skb_gro_offset(skb)) only guarantees 28 bytes are linear, up to the end
> of the UDP header. That allows a write of up to 12 bytes past the linear
> part, into tailroom or skb_shared_info.
>
> With rx-gro-list enabled, a tun device in an unprivileged user namespace
> appears to be enough to reach this path. Remote peers on wireguard or
> point-to-point tunnels may also be able to send such packets.
>
> Isn't this the same write past the linear part that the patch is meant
> to fix? Would it be better to check every frag_list member here? Another
> option is to block cross-family merges in GRO, for example by checking
> p's IP version in inet_gro_receive().
>
> >
> >               ret = __skb_linearize(gso_skb);
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106141727.2303218-1-edumazet%40kernel.org

The cross-family merge you describe is a GRO issue on devices with
hard_header_len == 0 (L3 GRE, tun in IFF_TUN mode, wireguard). It is
addressed separately by:

https://lore.kernel.org/netdev/20261109164540.3104392-1-edumazet@kernel.org/
("net: gro: compare skb->protocol in gro_list_prepare()")

With it, a held packet and an incoming one must share skb->protocol.
ipv6_gro_receive() also compares the version nibble with the held
packet, so all frag_list members share the IP version of the first
one, which is what this patch relies on.

Re: [PATCH net] udp: check frag_list IP family before using skb_segment_list()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Dave Miller <davem@davem-thinkpadp1gen8.ibmlond.csb>:

On Fri,  6 Nov 2026 15:17:27 +0100 you wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> UDP fraglist GRO uses SKB_GSO_UDP_L4 for both IPv4 and IPv6.
> 
> __udp_gso_segment() reuses the frag_list skbs when the geometry of
> the head skb was not modified. skb_segment_list() keeps the network
> header of each frag_list skb as it was at GRO time and only adjusts
> the transport header offset.
> 
> [...]

Here is the summary with links:
  - [net] udp: check frag_list IP family before using skb_segment_list()
    https://git.kernel.org/netdev/net/c/22b7cb932c6a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html