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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3dc6a26f-e02a-4518-ab12-5a1b39b4ae92@openvpn.net>
Date: Wed, 14 May 2025 16:13:24 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Sabrina Dubroca <sd@...asysnail.net>,
 David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH net-next] ovpn: properly deconfigure UDP-tunnel

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.