On 09/02/2026 16:32, Antonio Quartulli wrote:
What if we go the other way around: we have ovpn_tcp_close() check if the peer was already gone from the ovpn hash or not.

If it did, we know the peer is going through its own lifecycle and we skip the ovpn_peer_del() call.

This is what I have mind (not tested yet! only compiled):

diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index 1c02cc06f21b..83e7a5354266 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -543,8 +543,8 @@ int ovpn_tcp_socket_attach(struct ovpn_socket *ovpn_sock,

  static void ovpn_tcp_close(struct sock *sk, long timeout)
  {
+       struct ovpn_peer *peer, *tmp;
         struct ovpn_socket *sock;
-       struct ovpn_peer *peer;

         rcu_read_lock();
         sock = rcu_dereference_sk_user_data(sk);
@@ -555,7 +555,26 @@ static void ovpn_tcp_close(struct sock *sk, long timeout)
         peer = sock->peer;
         rcu_read_unlock();

-       ovpn_peer_del(sock->peer, OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT);
+       spin_lock_bh(&peer->ovpn->lock);
+       /* if the peer is already unhashed, it means it already
+        * went through ovpn_peer_remove() due to the actual removal event.
+        * This tcp_close call is just the socket being cleaned up by
+        * userspace and we don't need to double process the deletion
+        *
+        * NOTE: we need to lock (instead of just rcu-read-lock) because
+        * we have to synchronize with other calls to ovpn_peer_remove()
+        * happening with ovpn->lock held.
+        * This way we are guaranteed to call ovpn_peer_del() without
+        * racing with another ovpn_peer_remove().
+        */
+       tmp = ovpn_peer_get_by_id(peer->ovpn, peer->id);
+       if (tmp) {

or even better, we could check for !hlist_unhashed(&peer->hash_entry_id), to avoid hitting another peer that just got the same ID re-assigned (nearly impossible, but you know ..)


Cheers,

+               ovpn_peer_del(sock->peer,
+                             OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT);
+               ovpn_peer_put(tmp);
+       }
+       spin_unlock_bh(&peer->ovpn->lock);
+
         peer->tcp.sk_cb.prot->close(sk, timeout);
         ovpn_peer_put(peer);
  }


What do you think?



--
Antonio Quartulli



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to