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;
> +}
--
Sabrina
