Re: [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 24 Mar 2026 11:09:11 +0100 Sabrina Dubroca wrote:
> -------- 8< --------
> diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c
> index c7f382437630..ebec8c2ff427 100644
> --- a/drivers/net/ovpn/netlink.c
> +++ b/drivers/net/ovpn/netlink.c
> @@ -90,8 +90,11 @@ void ovpn_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
>  	netdevice_tracker *tracker = (netdevice_tracker *)&info->user_ptr[1];
>  	struct ovpn_priv *ovpn = info->user_ptr[0];
>  
> -	if (ovpn)
> +	if (ovpn) {
> +		if (READ_ONCE(dev->reg_state) >= NETREG_UNREGISTERING)
> +			ovpn_peers_free(ovpn, NULL, OVPN_DEL_PEER_REASON_TEARDOWN);
>  		netdev_put(ovpn->dev, tracker);
> +	}
>  }
>  
>  static bool ovpn_nl_attr_sockaddr_remote(struct nlattr **attrs,
> -------- 8< --------
> 
> This would clean up a peer that may have been added while we were
> starting device unregistration. We hold a reference on the device so
> no UAF possible, netdev_wait_allrefs_any will wait for this. If we
> don't have a racing peer creation, ndo_uninit takes care of the peers.

LGTM. This or change all the write paths to check if the device is still
alive after taking the lock.

> Or we can call ovpn_peers_free on every NETDEV_UNREGISTER notification
> that netdev_wait_allrefs_any sends us (but then we don't need it in
> ndo_uninit).

Hm, wouldn't we need a notification _after_ netdev_wait_allrefs_any() ?

> And s/cancel_delayed_work_sync/disable_delayed_work_sync/ for the
> keepalive_work.
> 
> 
> LLM claims it's because of parallel_ops, I don't think this is
> related? It also claims this issue is only for UDP sockets (and TCP
> would see a UAF on the keepalive), but ovpn_peer_new always holds the
> ovpn netdev, so I don't think there's a difference there.

Yup, I think the LLMs are trying to be helpful and are looking for some
write lock earlier in the path. As much as they are annoying I can't
blame them here, I feel like we try to make things lockless too often.