Re: [PATCH net-next v11 06/23] ovpn: introduce the ovpn_peer object

2024-10-29, 11:47:19 +0100, Antonio Quartulli wrote:

+static void ovpn_peer_release(struct ovpn_peer *peer) +{

• ovpn_bind_reset(peer, NULL);
• dst_cache_destroy(&peer->dst_cache);

Is it safe to destroy the cache at this time? In the same function, we use rcu to free the peer, but AFAICT the dst_cache will be freed immediately:

void dst_cache_destroy(struct dst_cache *dst_cache) { [...] free_percpu(dst_cache->cache); }

(probably no real issue because ovpn_udp_send_skb gets called while we hold a reference to the peer?)

• netdev_put(peer->ovpn->dev, &peer->ovpn->dev_tracker);
• kfree_rcu(peer, rcu);

+}

[...]

+static int ovpn_peer_del_p2p(struct ovpn_peer *peer,

• 	     enum ovpn_del_peer_reason reason)
  

• __must_hold(&peer->ovpn->lock)

+{

• struct ovpn_peer *tmp;
• tmp = rcu_dereference_protected(peer->ovpn->peer,

• 			lockdep_is_held(&peer->ovpn->lock));
  

• if (tmp != peer) {

• DEBUG_NET_WARN_ON_ONCE(1);
  

• if (tmp)
  

• 	ovpn_peer_put(tmp);
  

Does peer->ovpn->peer need to be set to NULL here as well? Or is it going to survive this _put?

• return -ENOENT;
  

• }
• tmp->delete_reason = reason;
• RCU_INIT_POINTER(peer->ovpn->peer, NULL);
• ovpn_peer_put(tmp);
• return 0;

+}