On 17/01/2025 12:48, Sabrina Dubroca wrote:
[...]
-------- 8< --------

diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
index 72357bb5f30b..19aa4ee6d468 100644
--- a/drivers/net/ovpn/netlink.c
+++ b/drivers/net/ovpn/netlink.c
@@ -733,6 +733,9 @@ int ovpn_nl_peer_del_doit(struct sk_buff *skb, struct 
genl_info *info)
netdev_dbg(ovpn->dev, "del peer %u\n", peer->id);
        ret = ovpn_peer_del(peer, OVPN_DEL_PEER_REASON_USERSPACE);
+       if (ret >= 0 && peer->sock)
+               wait_for_completion(&peer->sock_detach);
+
        ovpn_peer_put(peer);
return ret;
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index b032390047fe..6120521d0c32 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -92,6 +92,7 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 
id)
        ovpn_peer_stats_init(&peer->vpn_stats);
        ovpn_peer_stats_init(&peer->link_stats);
        INIT_WORK(&peer->keepalive_work, ovpn_peer_keepalive_send);
+       init_completion(&peer->sock_detach);
ret = dst_cache_init(&peer->dst_cache, GFP_KERNEL);
        if (ret < 0) {
diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h
index 7a062cc5a5a4..8c54bf5709ef 100644
--- a/drivers/net/ovpn/peer.h
+++ b/drivers/net/ovpn/peer.h
@@ -112,6 +112,7 @@ struct ovpn_peer {
        struct rcu_head rcu;
        struct work_struct remove_work;
        struct work_struct keepalive_work;
+       struct completion sock_detach;
  };
/**
diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c
index a5c3bc834a35..7cefac42c3be 100644
--- a/drivers/net/ovpn/socket.c
+++ b/drivers/net/ovpn/socket.c
@@ -31,6 +31,8 @@ static void ovpn_socket_release_kref(struct kref *kref)
sockfd_put(sock->sock);
        kfree_rcu(sock, rcu);
+
+       complete(&sock->peer->sock_detach);
  }
/**
@@ -181,12 +183,12 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, 
struct ovpn_peer *peer)
ovpn_sock->sock = sock;
        kref_init(&ovpn_sock->refcount);
+       ovpn_sock->peer = peer;

Unfortunately this won't work for UDP, because we are taking a reference only to the *first* peer using this specific socket (= the peer allocating the ovpn_socket object).
All other peers won't get to this line.

Instead, we should somewhat only take the reference to the "last" peer using this socket: the peer which will truly ignite the socket detachment.
That's the peer that has to wait upon peer_del for full socket detachment.

Trying to get something smart out of my brains right now ...

Cheers,

/* TCP sockets are per-peer, therefore they are linked to their unique
         * peer
         */
        if (sock->sk->sk_protocol == IPPROTO_TCP) {
-               ovpn_sock->peer = peer;
                ovpn_peer_hold(peer);
        } else if (sock->sk->sk_protocol == IPPROTO_UDP) {
                /* in UDP we only link the ovpn instance since the socket is
diff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h
index 15827e347f53..3f5a35fd9048 100644
--- a/drivers/net/ovpn/socket.h
+++ b/drivers/net/ovpn/socket.h
@@ -28,12 +28,12 @@ struct ovpn_peer;
   * @rcu: member used to schedule RCU destructor callback
   */
  struct ovpn_socket {
+       struct ovpn_peer *peer;
        union {
                struct {
                        struct ovpn_priv *ovpn;
                        netdevice_tracker dev_tracker;
                };
-               struct ovpn_peer *peer;
        };
struct socket *sock;



--
Antonio Quartulli
OpenVPN Inc.


Reply via email to