[PATCH] ovpn: avoid putting unrelated P2P peer on socket release

[PATCH] ovpn: avoid putting unrelated P2P peer on socket release

[Qing Ming]

ovpn_peer_release_p2p() is called when an OVPN UDP socket is being
destroyed. It checks the currently published P2P peer and releases it only
if that peer still uses the socket being destroyed.

A peer replacement can publish a new peer before the old UDP socket is
destroyed. When the old socket destruction path runs afterwards,
ovpn_peer_release_p2p() observes the new peer through ovpn->peer. Since the
new peer uses a different socket, the function takes the socket mismatch
branch.

That branch still calls ovpn_peer_put(peer). At this point, however, peer
is the currently published replacement peer, not the peer associated with
the socket being destroyed. Dropping its reference can free it while
ovpn->peer still points to it, leading to later use-after-free accesses
from the peer and socket cleanup paths.

KASAN reports this as a slab-use-after-free on the kmalloc-1k ovpn_peer
object. In the reproducer, the object is allocated from ovpn_peer_new() via
ovpn_nl_peer_new_doit(), and freed through ovpn_peer_release_rcu() from RCU
callback processing. Observed access sites include ovpn_peer_remove(),
ovpn_socket_release(), ovpn_nl_peer_del_notify(), and unlock_ovpn().

Fix this by returning from the socket mismatch branch without putting the
peer.

Fixes: f6226ae7a0cd ("ovpn: introduce the ovpn_socket object")
Signed-off-by: Qing Ming <a0yami@mailbox.org>
---
 drivers/net/ovpn/peer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index a09d61296425..1844d97154ce 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -1167,7 +1167,6 @@ static void ovpn_peer_release_p2p(struct ovpn_priv *ovpn, struct sock *sk,
 		ovpn_sock = rcu_access_pointer(peer->sock);
 		if (!ovpn_sock || ovpn_sock->sk != sk) {
 			spin_unlock_bh(&ovpn->lock);
-			ovpn_peer_put(peer);
 			return;
 		}
 	}
-- 
2.53.0

Re: [PATCH] ovpn: avoid putting unrelated P2P peer on socket release

[Simon Horman]

On Sat, May 23, 2026 at 04:15:43PM +0800, Qing Ming wrote:
> ovpn_peer_release_p2p() is called when an OVPN UDP socket is being
> destroyed. It checks the currently published P2P peer and releases it only
> if that peer still uses the socket being destroyed.
> 
> A peer replacement can publish a new peer before the old UDP socket is
> destroyed. When the old socket destruction path runs afterwards,
> ovpn_peer_release_p2p() observes the new peer through ovpn->peer. Since the
> new peer uses a different socket, the function takes the socket mismatch
> branch.
> 
> That branch still calls ovpn_peer_put(peer). At this point, however, peer
> is the currently published replacement peer, not the peer associated with
> the socket being destroyed. Dropping its reference can free it while
> ovpn->peer still points to it, leading to later use-after-free accesses
> from the peer and socket cleanup paths.
> 
> KASAN reports this as a slab-use-after-free on the kmalloc-1k ovpn_peer
> object. In the reproducer, the object is allocated from ovpn_peer_new() via
> ovpn_nl_peer_new_doit(), and freed through ovpn_peer_release_rcu() from RCU
> callback processing. Observed access sites include ovpn_peer_remove(),
> ovpn_socket_release(), ovpn_nl_peer_del_notify(), and unlock_ovpn().
> 
> Fix this by returning from the socket mismatch branch without putting the
> peer.
> 
> Fixes: f6226ae7a0cd ("ovpn: introduce the ovpn_socket object")
> Signed-off-by: Qing Ming <a0yami@mailbox.org>

It is probably not necessary to resubmit because of this,
but for future reference bug fixes for code present in the net tree
should be targeted at that tree like this:

	Subject: [PATCH net] ...

You can see more about the Networking development workflow here:
https://docs.kernel.org/process/maintainer-netdev.html


FTR, there is an AI-generated review of this patchset available on
sashiko.dev. I do not believe it should effect the progress of this patch.
But, rather, be considered in the context of possible follow-up.


Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH] ovpn: avoid putting unrelated P2P peer on socket release

[Antonio Quartulli]

Hi,

On 27/05/2026 10:55, Simon Horman wrote:
> On Sat, May 23, 2026 at 04:15:43PM +0800, Qing Ming wrote:
>> ovpn_peer_release_p2p() is called when an OVPN UDP socket is being
>> destroyed. It checks the currently published P2P peer and releases it only
>> if that peer still uses the socket being destroyed.
>>
>> A peer replacement can publish a new peer before the old UDP socket is
>> destroyed. When the old socket destruction path runs afterwards,
>> ovpn_peer_release_p2p() observes the new peer through ovpn->peer. Since the
>> new peer uses a different socket, the function takes the socket mismatch
>> branch.
>>
>> That branch still calls ovpn_peer_put(peer). At this point, however, peer
>> is the currently published replacement peer, not the peer associated with
>> the socket being destroyed. Dropping its reference can free it while
>> ovpn->peer still points to it, leading to later use-after-free accesses
>> from the peer and socket cleanup paths.
>>
>> KASAN reports this as a slab-use-after-free on the kmalloc-1k ovpn_peer
>> object. In the reproducer, the object is allocated from ovpn_peer_new() via
>> ovpn_nl_peer_new_doit(), and freed through ovpn_peer_release_rcu() from RCU
>> callback processing. Observed access sites include ovpn_peer_remove(),
>> ovpn_socket_release(), ovpn_nl_peer_del_notify(), and unlock_ovpn().
>>
>> Fix this by returning from the socket mismatch branch without putting the
>> peer.
>>
>> Fixes: f6226ae7a0cd ("ovpn: introduce the ovpn_socket object")
>> Signed-off-by: Qing Ming <a0yami@mailbox.org>
> 
> It is probably not necessary to resubmit because of this,
> but for future reference bug fixes for code present in the net tree
> should be targeted at that tree like this:
> 
> 	Subject: [PATCH net] ...
> 
> You can see more about the Networking development workflow here:
> https://docs.kernel.org/process/maintainer-netdev.html
> 

Thanks for spotting this!
I'll queue this patch in my tree and send it to net soon.

> 
> FTR, there is an AI-generated review of this patchset available on
> sashiko.dev. I do not believe it should effect the progress of this patch.
> But, rather, be considered in the context of possible follow-up.
> 

We are already actively working on all pre-existing issues.

Posted fixes can be seen on the openvpn-devel mailing list.

We are triaging patches there first (Sashiko is picking them up too) to 
avoid clogging netdev.

> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Cheers!

-- 
Antonio Quartulli
OpenVPN Inc.

Re: [PATCH] ovpn: avoid putting unrelated P2P peer on socket release

[Antonio Quartulli]

On Sat, 23 May 2026 16:15:43 +0800, Qing Ming wrote:
> ovpn_peer_release_p2p() is called when an OVPN UDP socket is being
> destroyed. It checks the currently published P2P peer and releases it only
> if that peer still uses the socket being destroyed.
> 
> A peer replacement can publish a new peer before the old UDP socket is
> destroyed. When the old socket destruction path runs afterwards,
> ovpn_peer_release_p2p() observes the new peer through ovpn->peer. Since the
> new peer uses a different socket, the function takes the socket mismatch
> branch.
> 
> [...]

Applied, thanks!

[1/1] ovpn: avoid putting unrelated P2P peer on socket release
      commit: 532692e621e44252c99a3322c0604889f0af7dee

Best regards,
-- 
Antonio Quartulli <antonio@openvpn.net>