[PATCH net v2] udp_tunnel: fix GRO callback mismatch during setup

[PATCH net v2] udp_tunnel: fix GRO callback mismatch during setup

[Jérémy Jean]

setup_udp_tunnel_sock() updates an already bound socket. GRO can find it
while the socket callbacks and global static call disagree. Receive and
completion may then parse one packet differently and write out of bounds,
KASAN reports as:

  BUG: KASAN: slab-out-of-bounds in udp6_gro_complete+0x48a/0x4b0
  Write of size 2 at addr ffff8880044d8e78 by task repro/44

Publish gro_receive last and read it once. Count tunnel GRO receive
callbacks and consume one at each UDP completion. Only use the static
call if its target matches, and wait before changing it.

Fixes: 5d7f5b2f6b93 ("udp_tunnel: use static call for GRO hooks when possible")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---

v1: https://lore.kernel.org/all/20260906211351.1272699-2-Jeremy.Jean@oss.cyber.gouv.fr/
Sashiko: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906211351.1272699-2-Jeremy.Jean%40oss.cyber.gouv.fr

 include/net/gro.h          |  3 ++-
 net/core/gro.c             |  1 +
 net/ipv4/udp_offload.c     | 51 +++++++++++++++++++++++++++-----------
 net/ipv4/udp_tunnel_core.c |  5 ++--
 4 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/include/net/gro.h b/include/net/gro.h
index 7a19d4486456..81bd838f268f 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -45,7 +45,8 @@ struct napi_gro_cb {
 	/* Used in ipv6_gro_receive() and foo-over-udp and esp-in-udp */
 	u16	proto;
 
-	u16	pad;
+	/* UDP tunnel GRO callbacks left to complete */
+	u16	udp_tunnel_gro_count;
 
 /* Used in napi_gro_cb::free */
 #define NAPI_GRO_FREE             1
diff --git a/net/core/gro.c b/net/core/gro.c
index 75f6a5f39a4b..96c3bbd6c153 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -526,6 +526,7 @@ static enum gro_result dev_gro_receive(struct gro_node *gro,
 	*(u32 *)&NAPI_GRO_CB(skb)->zeroed = 0;
 	NAPI_GRO_CB(skb)->flush = skb_has_frag_list(skb);
 	NAPI_GRO_CB(skb)->count = 1;
+	NAPI_GRO_CB(skb)->udp_tunnel_gro_count = 0;
 	if (unlikely(skb_is_gso(skb))) {
 		NAPI_GRO_CB(skb)->count = skb_shinfo(skb)->gso_segs;
 		/* Only support TCP and non DODGY users, with a sane gso_segs. */
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index cd9d7426b227..39f06fcd59ab 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -136,6 +136,7 @@ void udp_tunnel_update_gro_rcv(struct sock *sk, bool add)
 		static_branch_enable(&udp_tunnel_static_call);
 	} else if (old_gro_type_nr == 1) {
 		static_branch_disable(&udp_tunnel_static_call);
+		synchronize_net();
 		static_call_update(udp_tunnel_gro_rcv, dummy_gro_rcv);
 	}
 
@@ -144,27 +145,37 @@ void udp_tunnel_update_gro_rcv(struct sock *sk, bool add)
 }
 EXPORT_SYMBOL_GPL(udp_tunnel_update_gro_rcv);
 
-static struct sk_buff *udp_tunnel_gro_rcv(struct sock *sk,
+static struct sk_buff *udp_tunnel_gro_rcv(udp_tunnel_gro_receive_t gro_receive,
+					  struct sock *sk,
 					  struct list_head *head,
 					  struct sk_buff *skb)
 {
-	if (static_branch_likely(&udp_tunnel_static_call)) {
-		if (unlikely(gro_recursion_inc_test(skb))) {
-			NAPI_GRO_CB(skb)->flush |= 1;
-			return NULL;
-		}
-		return static_call(udp_tunnel_gro_rcv)(sk, head, skb);
+	if (unlikely(gro_recursion_inc_test(skb))) {
+		NAPI_GRO_CB(skb)->flush |= 1;
+		return NULL;
 	}
-	return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
+	NAPI_GRO_CB(skb)->udp_tunnel_gro_count++;
+
+	if (static_branch_likely(&udp_tunnel_static_call) &&
+	    likely(gro_receive == static_call_query(udp_tunnel_gro_rcv)))
+		return static_call(udp_tunnel_gro_rcv)(sk, head, skb);
+	return gro_receive(sk, head, skb);
 }
 
 #else
 
-static struct sk_buff *udp_tunnel_gro_rcv(struct sock *sk,
+static struct sk_buff *udp_tunnel_gro_rcv(udp_tunnel_gro_receive_t gro_receive,
+					  struct sock *sk,
 					  struct list_head *head,
 					  struct sk_buff *skb)
 {
-	return call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
+	if (unlikely(gro_recursion_inc_test(skb))) {
+		NAPI_GRO_CB(skb)->flush |= 1;
+		return NULL;
+	}
+	NAPI_GRO_CB(skb)->udp_tunnel_gro_count++;
+
+	return gro_receive(sk, head, skb);
 }
 
 #endif
@@ -777,6 +788,7 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
 struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 				struct udphdr *uh, struct sock *sk)
 {
+	udp_tunnel_gro_receive_t gro_receive = NULL;
 	struct sk_buff *pp = NULL;
 	struct sk_buff *p;
 	struct udphdr *uh2;
@@ -795,7 +807,11 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	 * cases. Such packets should be caught in udp_unexpected_gso later.
 	 */
 	NAPI_GRO_CB(skb)->is_flist = 0;
-	if (!sk || !udp_sk(sk)->gro_receive) {
+	if (sk) {
+		/* Pairs with smp_store_release() in setup_udp_tunnel_sock(). */
+		gro_receive = smp_load_acquire(&udp_sk(sk)->gro_receive);
+	}
+	if (!gro_receive) {
 		/* If the packet was locally encapsulated in a UDP tunnel that
 		 * wasn't detected above, do not GRO.
 		 */
@@ -842,7 +858,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 
 	skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
 	skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
-	pp = udp_tunnel_gro_rcv(sk, head, skb);
+	pp = udp_tunnel_gro_rcv(gro_receive, sk, head, skb);
 
 out:
 	skb_gro_flush_final(skb, pp, flush);
@@ -921,6 +937,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 {
 	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
 	unsigned int newlen = skb->len - nhoff;
+	udp_tunnel_gro_complete_t gro_complete = NULL;
 	struct sock *sk;
 	int err;
 
@@ -928,7 +945,12 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 
 	sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
 				udp4_lib_lookup_skb, skb, uh->source, uh->dest);
-	if (sk && udp_sk(sk)->gro_complete) {
+	if (NAPI_GRO_CB(skb)->udp_tunnel_gro_count) {
+		NAPI_GRO_CB(skb)->udp_tunnel_gro_count--;
+		if (sk)
+			gro_complete = READ_ONCE(udp_sk(sk)->gro_complete);
+	}
+	if (gro_complete) {
 		skb_shinfo(skb)->gso_type = uh->check ? SKB_GSO_UDP_TUNNEL_CSUM
 					: SKB_GSO_UDP_TUNNEL;
 
@@ -941,8 +963,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 		 * functions to make them set up the inner offsets.
 		 */
 		skb->encapsulation = 1;
-		err = udp_sk(sk)->gro_complete(sk, skb,
-				nhoff + sizeof(struct udphdr));
+		err = gro_complete(sk, skb, nhoff + sizeof(struct udphdr));
 	} else {
 		err = udp_gro_complete_segment(skb);
 	}
diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
index a128fe85620d..147c92374975 100644
--- a/net/ipv4/udp_tunnel_core.c
+++ b/net/ipv4/udp_tunnel_core.c
@@ -84,8 +84,9 @@ void setup_udp_tunnel_sock(struct net *net, struct sock *sk,
 	udp_sk(sk)->encap_err_rcv = cfg->encap_err_rcv;
 	udp_sk(sk)->encap_err_lookup = cfg->encap_err_lookup;
 	udp_sk(sk)->encap_destroy = cfg->encap_destroy;
-	udp_sk(sk)->gro_receive = cfg->gro_receive;
-	udp_sk(sk)->gro_complete = cfg->gro_complete;
+	WRITE_ONCE(udp_sk(sk)->gro_complete, cfg->gro_complete);
+	/* Pairs with smp_load_acquire() in udp_gro_receive(). */
+	smp_store_release(&udp_sk(sk)->gro_receive, cfg->gro_receive);
 
 	udp_tunnel_encap_enable(sk);
 
-- 
2.47.3

Re: [PATCH net v2] udp_tunnel: fix GRO callback mismatch during setup

[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 v2] udp_tunnel: fix GRO callback mismatch during setup

[Jérémy Jean]

On 2026-11-11 22:51, netdev-bot+sinfo@kernel.org wrote:
> 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.

The bug was discovered by code inspection assisted by LLM.

Re: [PATCH net v2] udp_tunnel: fix GRO callback mismatch during setup

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · 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 still lets receive and completion use different tunnel
  parsers on the same packet, so the out-of-bounds write the commit…
- [Medium] udp_tunnel_gro_count only describes how the head skb of a GRO
  entry was parsed, not the packets merged into it, so packets in the…
- [Medium] The Fixes: tag blames only 5d7f5b2f6b93 ("udp_tunnel: use
  static call for GRO hooks when possible", v6.16), but part of what the…
- [Low] The new synchronize_net() in udp_tunnel_update_gro_rcv()
  (net/ipv4/udp_offload.c:139) runs with the global…

--

udp_tunnel: fix GRO callback mismatch during setup

setup_udp_tunnel_sock() now writes gro_complete first and publishes
gro_receive last with release semantics, and udp_gro_receive() reads
gro_receive once. A new per-skb udp_tunnel_gro_count in napi_gro_cb is
incremented for each tunnel gro_receive callback and consumed in
udp_gro_complete(). The static call is used only when its target matches
the socket's callback, and a grace period is waited before resetting it.

> Fixes: 5d7f5b2f6b93 ("udp_tunnel: use static call for GRO hooks when possible")

[Severity: Medium]
Is 5d7f5b2f6b93 the only commit that should be named here?

That commit added the static call machinery and udp_tunnel_gro_rcv(). It
did not touch the gro_receive/gro_complete stores in
setup_udp_tunnel_sock() or this check in udp_gro_complete():

    if (sk && udp_sk(sk)->gro_complete) {

The older mismatch goes like this. Receive treats the packet as plain UDP
and holds it through udp_gro_receive_segment(). One example is sk == NULL
because udp_encap_needed_key is not enabled yet, on a device with
NETIF_F_GRO_UDP_FWD. setup_udp_tunnel_sock() then sets gro_complete. The
unconditional lookup in udp_gro_complete() finds the socket and runs the
tunnel gro_complete on a packet that was never tunnel-parsed.

The new udp_tunnel_gro_count is what closes that case: with a count of 0
the packet goes to udp_gro_complete_segment(). The commit message only
describes the socket callbacks and the global static call disagreeing,
though.

Stable tooling relies on the Fixes: tag, so kernels without 5d7f5b2f6b93
(6.12, 6.6, 6.1, ...) would not get this fix. Could another Fixes: tag be
added for the commit that introduced socket-based UDP tunnel GRO
completion, or at least a note in the commit message?

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index b77ae2f00c626..92c5e34bcf734 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -136,6 +136,7 @@ void udp_tunnel_update_gro_rcv(struct sock *sk, bool add)
>  		static_branch_enable(&udp_tunnel_static_call);
>  	} else if (old_gro_type_nr == 1) {
>  		static_branch_disable(&udp_tunnel_static_call);
> +		synchronize_net();
>  		static_call_update(udp_tunnel_gro_rcv, dummy_gro_rcv);
>  	}

[Severity: Low]
This synchronize_net() runs with the global udp_tunnel_gro_type_lock mutex
held. Is that a concern, given that unprivileged users can reach this
path?

udp_lib_setsockopt(UDP_ENCAP, UDP_ENCAP_ESPINUDP) on a socket with
UDP_GRO set:
    set_xfrm_gro_udp_encap_rcv()->udp_tunnel_update_gro_rcv(sk, true)

close():
    udp_destroy_sock()->udp_tunnel_cleanup_gro()
        ->udp_tunnel_update_gro_rcv(sk, false)

These callers don't hold rtnl, so synchronize_net() turns into a regular
synchronize_rcu(). An unprivileged process can keep triggering the 1->0
or 1->2 transitions, for example by alternating xfrm4/xfrm6 ESP-in-UDP
sockets.

vxlan, geneve and fou setup and teardown take the same mutex, usually
under rtnl. Would they then wait behind those grace periods and add to
rtnl latency?

[ ... ]

> @@ -847,7 +863,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>  
>  	skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
>  	skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> -	pp = udp_tunnel_gro_rcv(sk, head, skb);
> +	pp = udp_tunnel_gro_rcv(gro_receive, sk, head, skb);
>  
>  out:
>  	skb_gro_flush_final(skb, pp, flush);

[Severity: Medium]
Does udp_tunnel_gro_count cover the packets merged into a held GRO entry,
or only the head skb?

Say a packet P is held before setup_udp_tunnel_sock() publishes
gro_receive. P goes through udp_gro_receive_segment() in one of two ways:

  - NETIF_F_GRO_FRAGLIST, with is_flist = 1 because the kernel tunnel
    socket lacks GRO_ENABLED
  - sk == NULL plus NETIF_F_GRO_UDP_FWD while udp_encap_needed_key is
    still off

Either way, P has udp_tunnel_gro_count == 0 and encap_mark == 0.

A packet S on the same outer flow arrives after setup and takes the
tunnel path. The loop over head above only compares the ports and
whether the checksum is zero:

    if ((*(u32 *)&uh->source != *(u32 *)&uh2->source) ||
        (!uh->check ^ !uh2->check)) {

Nothing checks whether p itself was tunnel-parsed. Then
udp_tunnel_gro_rcv()->vxlan_gro_receive()->eth/inet/tcp gro_receive walk
P at p->data + off_vx and the inner offsets. They can merge S's inner
payload into P through skb_gro_receive() or skb_gro_receive_list().

At flush, udp_gro_complete() (or the fraglist shortcut in
udp4/6_gro_complete()) looks only at P's own count of 0. It completes the
aggregate as plain SKB_GSO_UDP_L4 or fraglist. P's gso_size is the outer
UDP payload size, but the merged data has its tunnel and inner headers
stripped.

Can this corrupt segmentation and payload for packets that arrive during
the setup window?

For napi_gro_frags() style drivers, P's linear area ends at the outer UDP
payload (see gro_try_pull_from_frag0()). Would S's tunnel receive then
also read P's inner headers beyond skb_headlen(P)?

[ ... ]

> @@ -933,7 +950,12 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
>  
>  	sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
>  				udp4_lib_lookup_skb, skb, uh->source, uh->dest);
> -	if (sk && udp_sk(sk)->gro_complete) {
> +	if (NAPI_GRO_CB(skb)->udp_tunnel_gro_count) {
> +		NAPI_GRO_CB(skb)->udp_tunnel_gro_count--;
> +		if (sk)
> +			gro_complete = READ_ONCE(udp_sk(sk)->gro_complete);
> +	}
> +	if (gro_complete) {
>  		skb_shinfo(skb)->gso_type = uh->check ? SKB_GSO_UDP_TUNNEL_CSUM
>  					: SKB_GSO_UDP_TUNNEL;
>  

[Severity: High]
Does the counter actually tie completion to the gro_receive that ran at
receive time?

udp_tunnel_gro_rcv() only increments udp_tunnel_gro_count. It does not
record which callback or socket ran. Here udp_gro_complete() still does a
fresh udp4_lib_lookup_skb()/udp6_lib_lookup_skb() at flush time. Whenever
the counter is non-zero, it calls the gro_complete of whatever socket
that lookup returns.

Held GRO packets are not inside an RCU section. They can stay held across
NAPI polls when gro_flush_timeout and napi_defer_hard_irqs are set, since
napi_complete_done() only does gro_flush_normal(&n->gro, !!timeout). So
something like this looks possible:

  vxlan_gro_receive() aggregates the packet, udp_tunnel_gro_count = 1,
  and the packet is held on the NAPI GRO list

  the vxlan link is deleted:
    vxlan_sock_release()->udp_tunnel_sock_release()
    (nothing flushes the lower device's GRO lists)

  a fou/GUE port is added on the same UDP port:
    setup_udp_tunnel_sock() runs while the packet is still held

  flush:
    udp_gro_complete()->udp4_lib_lookup_skb() returns the GUE socket
    gro_complete is gue_gro_complete()

gue_gro_complete() in net/ipv4/fou_core.c then does:

    guehlen = sizeof(*guehdr) + (guehdr->hlen << 2);
    ...
    err = ops->callbacks.gro_complete(skb, nhoff + guehlen);

hlen comes from vxlan header bytes. vxlan_gro_prepare_receive() only
compared those bytes for flags and VNI, so guehlen can be up to 128
bytes. The inner inet/ipv6 gro_complete handlers then write
tot_len/payload_len/check and dispatch L4 completes at offsets that
receive never validated.

Can this still cause the slab-out-of-bounds write shown in the KASAN
report in the commit message? It needs CAP_NET_ADMIN in a netns, which
an unprivileged user can get through a user namespace with a tun
IFF_NAPI device.

The opposite case also seems possible. If the new socket has no
gro_complete, or the completion lookup returns NULL while the receive
lookup found a socket, the counter is still consumed. Then
udp_gro_complete_segment() marks a tunnel-aggregated skb as plain
SKB_GSO_UDP_L4.

One way for the two lookups to differ is the port-only fast path in
udp4_gro_lookup_skb():

    sk = udp_tunnel_sk(net, false);
    if (sk && dport == htons(sk->sk_num))
        return sk;

__udp4_lib_lookup() at completion has no such path. A VRF slave with
udp_l3mdev_accept=0 is one example.

This sequence could also happen before this patch. However, the stated
goal of the patch is to stop receive and completion from parsing one
packet differently. Would it work better to record at receive time which
callback or socket ran, or to refuse tunnel completion when they differ,
instead of counting callbacks?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111214622.2482547-2-Jeremy.Jean%40oss.cyber.gouv.fr