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

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 20 Mar 2026 11:03:51 +0100 Antonio Quartulli wrote:
> While deleting an existing ovpn interface, there is a very
> narrow window where adding a new peer via netlink may cause
> the netdevice to hang and prevent its unregistration.
> 
> It may happen during ovpn_dellink(), when all existing peers are
> freed and the device is queued for deregistration, but a
> CMD_PEER_NEW message comes in adding a new peer that takes again
> a reference to the netdev.
> 
> At this point there is no way to release the device because we are
> under the assumption that all peers were already released.
> 
> Fix the race condition by releasing all peers in ndo_uninit(),
> when the netdevice has already been removed from the netdev
> list and thus an incoming CMD_PEER_NEW cannot have any effect
> anymore.
> 
> At this point ovpn_dellink() becomes empty and can just be
> removed.

This looks like a step in the right direction but AI points out that
it's not enough:

Does this completely resolve the race condition?
If a CMD_PEER_NEW netlink message executes concurrently, could the
following sequence occur since ovpn_nl_family uses parallel_ops:
1. In ovpn_nl_pre_doit(), the netlink thread looks up the device via
   dev_get_by_index_rcu(), increments its reference count via netdev_hold(),
   and drops the RCU lock.
2. Concurrently, device unregistration unlists the device and calls
   synchronize_net(). Since the RCU lock in ovpn_nl_pre_doit() was dropped,
   unregistration proceeds without waiting for the netlink command.
3. Unregistration executes ndo_uninit() (ovpn_net_uninit()), which calls
   cancel_delayed_work_sync() and ovpn_peers_free(), emptying the interface.
4. The preempted CMD_PEER_NEW thread resumes and adds the new peer via
   ovpn_peer_add(). Because the device registration state isn't verified
   (e.g., checking if dev->reg_state == NETREG_REGISTERED), the peer is
   added to the cleared hash tables.
If this sequence happens, wouldn't it cause a permanent hang for UDP sockets?
ovpn_socket_new() acquires a permanent netdev reference. Since
ovpn_peers_free() already ran, this peer is never removed, causing
netdev_wait_allrefs() to hang the kernel indefinitely.
Additionally, for TCP sockets, the keepalive timer would be re-armed
after being canceled here, leading to a use-after-free when the timer
eventually fires on the freed device memory.