Re: [PATCH net 2/2] ovpn: don't re-hash a removed peer on float

[Antonio Quartulli <antonio@openvpn.net>]

Hi Junrui,

On 05/08/2026 15:29, Junrui Luo via B4 Relay wrote:
> From: Junrui Luo <moonafterrain@outlook.com>
> 
> ovpn_peer_endpoints_update() releases peer->lock before sending the
> float notification, then re-acquires ovpn->lock and peer->lock to move
> the peer to its new bucket in the by_transp_addr table. The only
> re-check in that second critical section is for a NULL bind, which
> cannot detect removal: bind is cleared by ovpn_peer_release() only
> after the refcount drops to zero, and the RX path holds a reference
> across the whole float.
> 
> Both paths take ovpn->lock, but that only serialises them - it does
> not order them:
> 
>    CPU0 (RX softirq)              CPU1
>    ovpn_peer_endpoints_update()
>      spin_unlock_bh(&peer->lock)
>      ovpn_nl_peer_float_notify()
>                                   ovpn_nl_peer_del_doit()
>                                     ovpn_peer_remove()  <- unlinks peer
>                                     unlock_ovpn()       <- drops last ref
>      spin_lock_bh(&ovpn->lock)
>      hlist_nulls_add_head_rcu()   <- removed peer re-linked
> 
> ovpn_peer_release_rcu() then frees the peer without unlinking it again,
> leaving a dangling node in by_transp_addr that every later datagram
> walks in ovpn_peer_get_by_transp_addr():
> 
>    BUG: KASAN: slab-use-after-free in ovpn_peer_endpoints_update+0xa5a/0x1010
>    Write of size 8 at addr ffff888008576858 by task trigger/78
> 
>    Call Trace:
>     ovpn_peer_endpoints_update+0xa5a/0x1010
>     ovpn_decrypt_post+0x212/0x1040
>     ovpn_recv+0x2b6/0x540
>     ovpn_udp_encap_recv+0x21c/0x420
>     udp_queue_rcv_one_skb+0x1060/0x11a0
>     process_backlog+0x451/0x600
> 
>    Freed by task 0:
>     kfree+0x11a/0x390
>     rcu_core+0x7aa/0x1570
> 
>    Last potentially related work creation:
>     call_rcu+0x82/0x720
>     ovpn_peer_release_kref+0x5c/0xd0
>     ovpn_nl_peer_del_doit+0x355/0x550
> 
> Fix it by extending the existing early return to also bail out when the
> peer is no longer hashed by ID. hash_entry_id is unhashed with
> hlist_del_init_rcu() by ovpn_peer_remove() under ovpn->lock, which the
> float path holds across both the check and the rehash, so a peer that
> passes the test cannot be removed before it is re-linked.
> 
> Fixes: f0281c1d3732 ("ovpn: add support for updating local or remote UDP endpoint")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Assisted-by: Claude:claude-opus-5
> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

A fix for this issue was recently merged to net:

commit 33ec10567fe14456063daf549fdf1a4f53448e4c
Author: Antonio Quartulli <antonio@openvpn.net>
Date:   Tue Jul 28 13:48:47 2026 +0200

     ovpn: skip rehash for peers already removed from by_id

Probably you came up with this fix on an older codebase.

I suggest two things:
1) always check the openvpn-devel mailing list and our patchwork 
instance for already submitted patches that are waiting review (or that 
are queued for sending to net/net-next)
2) make sure to always pull the latest tree patches are based on (net or 
net-next) before sending them over.

In this period of hectic AI works things are moving exceptionally fast 
(well, not always :))

Thanks a lot anyway for looking after our code!

Best Regards,

-- 
Antonio Quartulli
OpenVPN Inc.