[PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[Antonio Quartulli]

When deconfiguring a UDP-tunnel from a socket, we cannot
call setup_udp_tunnel_sock(), as it is expected to be invoked
only during setup.

Implement a new function named cleanup_udp_tunnel_sock(),
that reverts was what done during setup, and invoke it.

This new function takes care of reverting everything that
was done by setup_udp_tunnel_sock() (i.e. unsetting various
sk members and cleaning up the encap state in the kernel).

Cc: David Ahern <dsahern@kernel.org>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Reported-by: Paolo Abeni <pabeni@redhat.com>
Closes: https://lore.kernel.org/netdev/1a47ce02-fd42-4761-8697-f3f315011cc6@redhat.com
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---

@Jakub: I am sending this directly to net-next as it'd be better reviewed
by the broader audience on netdev right away.

@Paolo: I think this is the best way to handle the UDP tunnel cleanup.
Doing nothing is not truly an option because we leave all the encap
state enabled and this may lead to other issues.
cleanup_udp_tunnel_sock() will just undo anything that
setup_udp_tunnel_sock() had done during setup.

 drivers/net/ovpn/udp.c     |  5 +----
 include/net/udp_tunnel.h   |  1 +
 net/ipv4/udp_tunnel_core.c | 30 ++++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index aef8c0406ec9..137b58d49569 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -442,8 +442,5 @@ int ovpn_udp_socket_attach(struct ovpn_socket *ovpn_sock,
  */
 void ovpn_udp_socket_detach(struct ovpn_socket *ovpn_sock)
 {
-	struct udp_tunnel_sock_cfg cfg = { };
-
-	setup_udp_tunnel_sock(sock_net(ovpn_sock->sock->sk), ovpn_sock->sock,
-			      &cfg);
+	cleanup_udp_tunnel_sock(ovpn_sock->sock);
 }
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 2df3b8344eb5..387445133eb7 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -92,6 +92,7 @@ struct udp_tunnel_sock_cfg {
 /* Setup the given (UDP) sock to receive UDP encapsulated packets */
 void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
 			   struct udp_tunnel_sock_cfg *sock_cfg);
+void cleanup_udp_tunnel_sock(struct socket *sock);
 
 /* -- List of parsable UDP tunnel types --
  *
diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
index 2326548997d3..83e9fd2d499e 100644
--- a/net/ipv4/udp_tunnel_core.c
+++ b/net/ipv4/udp_tunnel_core.c
@@ -98,6 +98,36 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
 }
 EXPORT_SYMBOL_GPL(setup_udp_tunnel_sock);
 
+void cleanup_udp_tunnel_sock(struct socket *sock)
+{
+	struct sock *sk = sock->sk;
+
+	/* Re-enable multicast loopback */
+	inet_set_bit(MC_LOOP, sk);
+
+	/* Disable CHECKSUM_UNNECESSARY to CHECKSUM_COMPLETE conversion */
+	inet_dec_convert_csum(sk);
+
+	rcu_assign_sk_user_data(sk, NULL);
+
+	udp_sk(sk)->encap_type = 0;
+	udp_sk(sk)->encap_rcv = NULL;
+	udp_sk(sk)->encap_err_rcv = NULL;
+	udp_sk(sk)->encap_err_lookup = NULL;
+	udp_sk(sk)->encap_destroy = NULL;
+	udp_sk(sk)->gro_receive = NULL;
+	udp_sk(sk)->gro_complete = NULL;
+
+	udp_clear_bit(ENCAP_ENABLED, sk);
+#if IS_ENABLED(CONFIG_IPV6)
+	if (READ_ONCE(sk->sk_family) == PF_INET6)
+		static_branch_dec(&udpv6_encap_needed_key);
+#endif
+	udp_encap_disable();
+	udp_tunnel_cleanup_gro(sk);
+}
+EXPORT_SYMBOL_GPL(cleanup_udp_tunnel_sock);
+
 void udp_tunnel_push_rx_port(struct net_device *dev, struct socket *sock,
 			     unsigned short type)
 {
-- 
2.49.0

Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[Jakub Kicinski]

On Wed, 14 May 2025 11:58:42 +0200 Antonio Quartulli wrote:
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (READ_ONCE(sk->sk_family) == PF_INET6)
> +		static_branch_dec(&udpv6_encap_needed_key);

udpv6_encap_needed_key is only exported for ipv6.ko
I think you need to correct the export, because with IPV6=y 
and UDP_TUNNEL=m we get:

ERROR: modpost: "udpv6_encap_needed_key" [net/ipv4/udp_tunnel.ko] undefined!
-- 
pw-bot: cr

Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[Antonio Quartulli]

On 14/05/2025 15:50, Jakub Kicinski wrote:
> On Wed, 14 May 2025 11:58:42 +0200 Antonio Quartulli wrote:
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +	if (READ_ONCE(sk->sk_family) == PF_INET6)
>> +		static_branch_dec(&udpv6_encap_needed_key);
> 
> udpv6_encap_needed_key is only exported for ipv6.ko
> I think you need to correct the export, because with IPV6=y
> and UDP_TUNNEL=m we get:
> 
> ERROR: modpost: "udpv6_encap_needed_key" [net/ipv4/udp_tunnel.ko] undefined!

Or define and export udpv6_encap_enable() which calls only the 
static_branch_dec()?
Basically the same as the current udpv6_encap_enable().

This way we don't need to touch the export for udpv6_encap_needed_key.

Regards,

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[Antonio Quartulli]

On 14/05/2025 16:02, Antonio Quartulli wrote:
> 
> 
> On 14/05/2025 15:50, Jakub Kicinski wrote:
>> On Wed, 14 May 2025 11:58:42 +0200 Antonio Quartulli wrote:
>>> +#if IS_ENABLED(CONFIG_IPV6)
>>> +    if (READ_ONCE(sk->sk_family) == PF_INET6)
>>> +        static_branch_dec(&udpv6_encap_needed_key);
>>
>> udpv6_encap_needed_key is only exported for ipv6.ko
>> I think you need to correct the export, because with IPV6=y
>> and UDP_TUNNEL=m we get:
>>
>> ERROR: modpost: "udpv6_encap_needed_key" [net/ipv4/udp_tunnel.ko] 
>> undefined!
> 
> Or define and export udpv6_encap_enable() which calls only the 

EDIT: I meant udpv6_encap_disable() here ^^

> static_branch_dec()?
> Basically the same as the current udpv6_encap_enable().
> 
> This way we don't need to touch the export for udpv6_encap_needed_key.
> 
> Regards,
> 

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[Antonio Quartulli]

On 14/05/2025 11:58, Antonio Quartulli wrote:
> --- a/drivers/net/ovpn/udp.c
> +++ b/drivers/net/ovpn/udp.c
> @@ -442,8 +442,5 @@ int ovpn_udp_socket_attach(struct ovpn_socket *ovpn_sock,
>    */
>   void ovpn_udp_socket_detach(struct ovpn_socket *ovpn_sock)
>   {
> -	struct udp_tunnel_sock_cfg cfg = { };
> -
> -	setup_udp_tunnel_sock(sock_net(ovpn_sock->sock->sk), ovpn_sock->sock,
> -			      &cfg);
> +	cleanup_udp_tunnel_sock(ovpn_sock->sock);

After looking at this some more, I think this is still racy.

We have no guarantee that sock->sk won't be nullified while executing 
cleanup_udp_tunnel_sock().

The alternative would be to change cleanup_udp_tunnel_sock() to take a 
'struct sock' argument (the 'struct socket' container is not really useful).

I see that also setup_udp_tunnel_sock() takes a 'struct socket' argument 
although there is no need for that. Is there some kind of abstraction in 
udp_tunnel which wants the user to always pass 'struct socket' instead 
of 'struct sock' ?


Regards,


-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

[kernel test robot]

Hi Antonio,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Antonio-Quartulli/ovpn-properly-deconfigure-UDP-tunnel/20250514-180029
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250514095842.12067-1-antonio%40openvpn.net
patch subject: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20250518/202505182055.9AF6d1fu-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250518/202505182055.9AF6d1fu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505182055.9AF6d1fu-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "udpv6_encap_needed_key" [net/ipv4/udp_tunnel.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki