[PATCH v1 net-next 0/5] udp_tunnel: Speed up UDP tunnel device destruction (Part II)

[PATCH v1 net-next 0/5] udp_tunnel: Speed up UDP tunnel device destruction (Part II)

[Kuniyuki Iwashima]

Most of the UDP tunnel devices called synchronize_rcu() twice
during destruction, for example, vxlan had

  1) synchronize_rcu() in udp_tunnel_sock_release()

  2) synchronize_net() in vxlan_sock_release()

The former was already removed by:
https://lore.kernel.org/netdev/20260502031401.3557229-1-kuniyu@google.com/

This series removes the latter.

Patch 1 & 2 & 4 remove synchronize_net() placed before
udp_tunnel_sock_release().

Patch 3 removes yet another unnecessary synchronize_net() in
geneve_unquiesce().

Patch 5 is a follow-up patch for a sparse report by kernel test robot.


Kuniyuki Iwashima (5):
  vxlan: Remove synchronize_net() in vxlan_sock_release().
  geneve: Remove synchronize_net() in geneve_sock_release().
  geneve: Remove synchronize_net() in geneve_unquiesce().
  bareudp: Remove synchronize_net() in bareudp_sock_release().
  bareudp: Use rtnl_dereference() in bareudp_sock_release().

 drivers/net/bareudp.c          | 3 +--
 drivers/net/geneve.c           | 2 --
 drivers/net/vxlan/vxlan_core.c | 1 -
 3 files changed, 1 insertion(+), 5 deletions(-)

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v1 net-next 1/5] vxlan: Remove synchronize_net() in vxlan_sock_release().

[Kuniyuki Iwashima]

Initially, a dedicated workqueue was used to defer calling
udp_tunnel_sock_release(vxlan_sock->sock) and kfree(vxlan_sock).

Later, commit 0412bd931f5f ("vxlan: synchronously and race-free
destruction of vxlan sockets") removed the workqueue and instead
invoked these two functions immediately after synchronize_net().

This was intended to prevent UAF of the UDP socket in the fast path.

( Note that the "nondeterministic behaviour" mentioned in that
  commit was not addressed, as another thread not waiting RCU gp
  still sees the same behaviour. )

However, a week prior to that change, commit ca065d0cf80f ("udp:
no longer use SLAB_DESTROY_BY_RCU") had already moved UDP socket
freeing to after the RCU grace period.  This made the synchronize_net()
in vxlan_sock_release() completely redundant.

Since vxlan_sock now uses kfree_rcu() and is invoked after
udp_tunnel_sock_release(), vxlan_sock is guaranteed to be freed
either at the same time or after the UDP socket is released,
following the RCU grace period.

Let's remove the redundant synchronize_net() in vxlan_sock_release().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/vxlan/vxlan_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 00facbfabced..8c3885665b58 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1515,7 +1515,6 @@ static void vxlan_sock_release(struct vxlan_dev *vxlan)
 #endif
 
 	RCU_INIT_POINTER(vxlan->vn4_sock, NULL);
-	synchronize_net();
 
 	if (vxlan->cfg.flags & VXLAN_F_VNIFILTER)
 		vxlan_vs_del_vnigrp(vxlan);
-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v1 net-next 2/5] geneve: Remove synchronize_net() in geneve_sock_release().

[Kuniyuki Iwashima]

vxlan previously had an issue where the fast path could access
stale pointers, which was fixed by commit c6fcc4fc5f8b ("vxlan:
avoid using stale vxlan socket.").

geneve later followed the same pattern, and commit fceb9c3e3825
("geneve: avoid using stale geneve socket.") copied synchronize_net()
from vxlan_sock_release() into geneve_sock_release().

However, that change occurred after commit ca065d0cf80f ("udp: no
longer use SLAB_DESTROY_BY_RCU"), and geneve had already been
using kfree_rcu() to free geneve_sock.

Therefore, the synchronize_net() was never actually needed there.

Let's remove the redundant synchronize_net() in geneve_sock_release().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/geneve.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index b36fad833724..4e5d0a09a82d 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1032,7 +1032,6 @@ static void geneve_sock_release(struct geneve_dev *geneve)
 #endif
 
 	rcu_assign_pointer(geneve->sock4, NULL);
-	synchronize_net();
 
 	__geneve_sock_release(gs4);
 #if IS_ENABLED(CONFIG_IPV6)
-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v1 net-next 3/5] geneve: Remove synchronize_net() in geneve_unquiesce().

[Kuniyuki Iwashima]

When changing the geneve config, geneve_changelink() sandwiches
the config memcpy() between geneve_quiesce() and geneve_unquiesce().

geneve_quiesce() temporarily clears geneve->sock[46] and their
sk_user_data, and then calls synchronize_net() to wait for inflight
fast paths to finish.

geneve_unquiesce() then restores the cleared pointers, but it also
superfluously calls synchronize_net().

The latter synchronize_net() provides no benefit; with or without it,
inflight fast paths can see either the NULL pointers or the original
pointers alongside the new configuration.

Let's remove the redundant synchronize_net() in geneve_unquiesce().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/geneve.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 4e5d0a09a82d..e8ff03ed87dc 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -2206,7 +2206,6 @@ static void geneve_unquiesce(struct geneve_dev *geneve, struct geneve_sock *gs4,
 	if (gs6)
 		rcu_assign_sk_user_data(gs6->sk, gs6);
 #endif
-	synchronize_net();
 }
 
 static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v1 net-next 4/5] bareudp: Remove synchronize_net() in bareudp_sock_release().

[Kuniyuki Iwashima]

synchronize_net() in bareudp_sock_release() has existed since
day 1, commit 571912c69f0e ("net: UDP tunnel encapsulation module
for tunnelling different protocols like MPLS, IP, NSH etc.").

It was most likely copied from a similar tunneling device like
vxlan or geneve.

bareudp_sock_release() is called from dev->netdev_ops->ndo_stop(),
and synchronize_net() in unregister_netdevice_many_notify() ensures
that inflight bareudp fast paths finish before bareudp_dev is freed.

Let's remove the redundant synchronize_net() in bareudp_sock_release().

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 drivers/net/bareudp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index 890a0650d9cf..bfcecdcffe67 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -287,7 +287,6 @@ static void bareudp_sock_release(struct bareudp_dev *bareudp)
 
 	sk = bareudp->sk;
 	rcu_assign_pointer(bareudp->sk, NULL);
-	synchronize_net();
 	udp_tunnel_sock_release(sk);
 }
 
-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v1 net-next 5/5] bareudp: Use rtnl_dereference() in bareudp_sock_release().

[Kuniyuki Iwashima]

kernel test robot reported sparse warning in bareudp_sock_release():

  drivers/net/bareudp.c:288:12: warning: incorrect type in assignment (different address spaces)
  drivers/net/bareudp.c:288:12:    expected struct sock *sk
  drivers/net/bareudp.c:288:12:    got struct sock [noderef] __rcu *sk

The warning is not new and exists since the initial bareudp commit
571912c69f0e ("net: UDP tunnel encapsulation module for tunnelling
different protocols like MPLS, IP, NSH etc.").

Let's use rtnl_dereference().

Note that bareudp_sock_release() is called from bareudp_stop()
under RTNL, so there is no real issue even without the helper.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605062359.e3gOfZCr-lkp@intel.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
I didn't add Fixes tag so we don't bother resolving conflicts
for harmless warning.
---
 drivers/net/bareudp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
index bfcecdcffe67..5ef841c85526 100644
--- a/drivers/net/bareudp.c
+++ b/drivers/net/bareudp.c
@@ -285,7 +285,7 @@ static void bareudp_sock_release(struct bareudp_dev *bareudp)
 {
 	struct sock *sk;
 
-	sk = bareudp->sk;
+	sk = rtnl_dereference(bareudp->sk);
 	rcu_assign_pointer(bareudp->sk, NULL);
 	udp_tunnel_sock_release(sk);
 }
-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v1 net-next 1/5] vxlan: Remove synchronize_net() in vxlan_sock_release().

[Kuniyuki Iwashima]

For Sashiko review:

On Sun, May 17, 2026 at 10:07 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> Initially, a dedicated workqueue was used to defer calling
> udp_tunnel_sock_release(vxlan_sock->sock) and kfree(vxlan_sock).
>
> Later, commit 0412bd931f5f ("vxlan: synchronously and race-free
> destruction of vxlan sockets") removed the workqueue and instead
> invoked these two functions immediately after synchronize_net().
>
> This was intended to prevent UAF of the UDP socket in the fast path.
>
> ( Note that the "nondeterministic behaviour" mentioned in that
>   commit was not addressed, as another thread not waiting RCU gp
>   still sees the same behaviour. )
>
> However, a week prior to that change, commit ca065d0cf80f ("udp:
> no longer use SLAB_DESTROY_BY_RCU") had already moved UDP socket
> freeing to after the RCU grace period.  This made the synchronize_net()
> in vxlan_sock_release() completely redundant.
>
> Since vxlan_sock now uses kfree_rcu() and is invoked after
> udp_tunnel_sock_release(), vxlan_sock is guaranteed to be freed
> either at the same time or after the UDP socket is released,
> following the RCU grace period.

---8<---
Is this reasoning complete? The commit message notes that the socket memory
is freed after the RCU grace period, but does this account for references
taken inside the RCU read-side critical section that escape it?
In the transmit path, vxlan_xmit() accesses vn4_sock inside an
rcu_read_lock() block and passes the socket to udp_tunnel_xmit_skb(). This
eventually calls skb_set_owner_w(), which increments sk->sk_wmem_alloc to
keep the socket memory alive until the packet transmission completes.
---8<---

The skb_set_owner_w() Sashiko mentions is the one called
for a fragmented skb in ip_do_fragment().

In this case, the original skb still holds the same amount of
sk->sk_wmem_alloc and prevents sk from being freed, so
skb_set_owner_w() there is safe without synchronize_net().


>
> Let's remove the redundant synchronize_net() in vxlan_sock_release().
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  drivers/net/vxlan/vxlan_core.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 00facbfabced..8c3885665b58 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1515,7 +1515,6 @@ static void vxlan_sock_release(struct vxlan_dev *vxlan)
>  #endif
>
>         RCU_INIT_POINTER(vxlan->vn4_sock, NULL);
> -       synchronize_net();
>
>         if (vxlan->cfg.flags & VXLAN_F_VNIFILTER)
>                 vxlan_vs_del_vnigrp(vxlan);

---8<---
Does removing this synchronize_net() introduce a use-after-free regression
when tearing down the socket while packets are concurrently transmitting?
Without synchronize_net(), udp_tunnel_sock_release() runs immediately after
setting vn4_sock to NULL. If sk_refcnt reaches zero, sk_free() is queued.
...
---8<---

Same.

Re: [PATCH v1 net-next 2/5] geneve: Remove synchronize_net() in geneve_sock_release().

[Kuniyuki Iwashima]

For Sashiko review:

On Sun, May 17, 2026 at 10:07 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> vxlan previously had an issue where the fast path could access
> stale pointers, which was fixed by commit c6fcc4fc5f8b ("vxlan:
> avoid using stale vxlan socket.").
>
> geneve later followed the same pattern, and commit fceb9c3e3825
> ("geneve: avoid using stale geneve socket.") copied synchronize_net()
> from vxlan_sock_release() into geneve_sock_release().
>
> However, that change occurred after commit ca065d0cf80f ("udp: no
> longer use SLAB_DESTROY_BY_RCU"), and geneve had already been
> using kfree_rcu() to free geneve_sock.
>
> Therefore, the synchronize_net() was never actually needed there.
>
> Let's remove the redundant synchronize_net() in geneve_sock_release().
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  drivers/net/geneve.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index b36fad833724..4e5d0a09a82d 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1032,7 +1032,6 @@ static void geneve_sock_release(struct geneve_dev *geneve)
>  #endif
>
>         rcu_assign_pointer(geneve->sock4, NULL);
> -       synchronize_net();
>
>         __geneve_sock_release(gs4);

---8<---
Does this function leave the device's hlist node linked in the socket's
vni_list?
In geneve_open(), if the IPv6 socket setup succeeds, the device's hlist6
node is linked into the IPv6 socket's vni_list via hlist_add_head_rcu().
If the subsequent IPv4 socket setup fails, it calls geneve_sock_release()
and returns an error.
While geneve_sock_release() drops the socket refcount, it doesn't remove
the hlist node. Because geneve_open() failed, the interface never gets the
IFF_UP flag. When the interface is subsequently deleted, dev_close()
bypasses ndo_stop (geneve_stop()), which is where the hlist removal is
normally handled.
If the IPv6 socket is shared and remains active, can concurrent RX traffic
trigger a use-after-free when geneve6_lookup() iterates the vni_list and
accesses the freed geneve_dev?
---8<---

pre-existing issue in geneve_open(), so orthogonal to
this patch.  I'll follow up on net.git separately.

Re: [PATCH v1 net-next 3/5] geneve: Remove synchronize_net() in geneve_unquiesce().

[Kuniyuki Iwashima]

On Sun, May 17, 2026 at 10:07 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> When changing the geneve config, geneve_changelink() sandwiches
> the config memcpy() between geneve_quiesce() and geneve_unquiesce().
>
> geneve_quiesce() temporarily clears geneve->sock[46] and their
> sk_user_data, and then calls synchronize_net() to wait for inflight
> fast paths to finish.
>
> geneve_unquiesce() then restores the cleared pointers, but it also
> superfluously calls synchronize_net().
>
> The latter synchronize_net() provides no benefit; with or without it,
> inflight fast paths can see either the NULL pointers or the original
> pointers alongside the new configuration.
>
> Let's remove the redundant synchronize_net() in geneve_unquiesce().
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  drivers/net/geneve.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 4e5d0a09a82d..e8ff03ed87dc 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -2206,7 +2206,6 @@ static void geneve_unquiesce(struct geneve_dev *geneve, struct geneve_sock *gs4,
>         if (gs6)
>                 rcu_assign_sk_user_data(gs6->sk, gs6);
>  #endif
> -       synchronize_net();

---8<---
Can the fast path read a torn geneve->cfg during geneve_changelink()?
...
---8<---

Yes, it's a pre-existing issue, and probably that's why vxlan does
not have even a function like geneve_quiesce() and accepts data-race
during changelink.