* [PATCH net 1/6] ovpn: avoid putting unrelated P2P peer on socket release
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 2/6] ovpn: fix peer refcount leak in TCP error paths Antonio Quartulli
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, Qing Ming, Simon Horman,
Antonio Quartulli
From: Qing Ming <a0yami@mailbox.org>
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>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
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.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH net 2/6] ovpn: fix peer refcount leak in TCP error paths
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 1/6] ovpn: avoid putting unrelated P2P peer on socket release Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 3/6] ovpn: hold peer before scheduling keepalive work Antonio Quartulli
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, Pavitra Jha, stable,
Antonio Quartulli
From: Pavitra Jha <jhapavitra98@gmail.com>
When either the TCP RX or TX error path calls ovpn_peer_hold() followed
by schedule_work(&peer->tcp.defer_del_work), and the work item is already
pending from the other path, schedule_work() returns false and the work
runs only once. Since ovpn_tcp_peer_del_work() calls ovpn_peer_put()
exactly once, the extra reference taken by the losing path is never
dropped, leaking the peer object.
The race window:
CPU0 (strparser/RX error): CPU1 (tcp_tx_work/TX error):
ovpn_peer_hold() <- refcnt+1 ovpn_peer_hold() <- refcnt+2
schedule_work() <- queued schedule_work() <- NO-OP
(work already pending)
ovpn_tcp_peer_del_work runs:
ovpn_peer_del()
ovpn_peer_put() <- refcnt+1
<- peer never freed
Fix by checking the return value of schedule_work() in both paths and
calling ovpn_peer_put() to drop the extra reference if the work was
already pending. ovpn_peer_hold() is kept unconditional in the TX path
as it cannot fail at that point.
Fixes: a6a5e87b3ee4 ("ovpn: avoid sleep in atomic context in TCP RX error path")
Cc: stable@vger.kernel.org
Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/tcp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index 433bd07a4f1b..0af14055c39a 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -151,7 +151,8 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
/* take reference for deferred peer deletion. should never fail */
if (WARN_ON(!ovpn_peer_hold(peer)))
goto err_nopeer;
- schedule_work(&peer->tcp.defer_del_work);
+ if (!schedule_work(&peer->tcp.defer_del_work))
+ ovpn_peer_put(peer);
ovpn_dev_dstats_rx_dropped(peer->ovpn->dev);
err_nopeer:
kfree_skb(skb);
@@ -283,7 +284,8 @@ static void ovpn_tcp_send_sock(struct ovpn_peer *peer, struct sock *sk)
* stream therefore we abort the connection
*/
ovpn_peer_hold(peer);
- schedule_work(&peer->tcp.defer_del_work);
+ if (!schedule_work(&peer->tcp.defer_del_work))
+ ovpn_peer_put(peer);
/* we bail out immediately and keep tx_in_progress set
* to true. This way we prevent more TX attempts
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH net 3/6] ovpn: hold peer before scheduling keepalive work
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 1/6] ovpn: avoid putting unrelated P2P peer on socket release Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 2/6] ovpn: fix peer refcount leak in TCP error paths Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 4/6] selftests/net: ovpn: fix getaddrinfo memory leak in ovpn_parse_remote() Antonio Quartulli
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, Shuvam Pandey, stable,
Antonio Quartulli
From: Shuvam Pandey <shuvampandey1@gmail.com>
ovpn_peer_keepalive_send() passes its peer reference to
ovpn_xmit_special(), which ultimately drops it. The keepalive scheduler
currently queues the work first and takes the reference only after
schedule_work() reports that the work was queued.
Once schedule_work() queues the item, another CPU may run the worker
before the caller gets to ovpn_peer_hold(). In that case the worker can
consume a reference that was not acquired for it, corrupting the peer
lifetime accounting.
Take the peer reference before queueing the work and drop it again when
the work was already pending.
Fixes: 3ecfd9349f40 ("ovpn: implement keepalive mechanism")
Cc: stable@vger.kernel.org
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/peer.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 1844d97154ce..2b6096d8b1cc 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -1284,8 +1284,10 @@ static time64_t ovpn_peer_keepalive_work_single(struct ovpn_peer *peer,
netdev_dbg(peer->ovpn->dev,
"sending keepalive to peer %u\n",
peer->id);
- if (schedule_work(&peer->keepalive_work))
- ovpn_peer_hold(peer);
+ if (WARN_ON(!ovpn_peer_hold(peer)))
+ return 0;
+ if (!schedule_work(&peer->keepalive_work))
+ ovpn_peer_put(peer);
}
if (next_run1 < next_run2)
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH net 4/6] selftests/net: ovpn: fix getaddrinfo memory leak in ovpn_parse_remote()
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
` (2 preceding siblings ...)
2026-07-20 14:41 ` [PATCH net 3/6] ovpn: hold peer before scheduling keepalive work Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 5/6] ovpn: fix use after free in unlock_ovpn() Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 6/6] ovpn: use monotonic clock for peer keepalive timeouts Antonio Quartulli
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, longlong yan,
Antonio Quartulli
From: longlong yan <yanlonglong@kylinos.cn>
The ovpn_parse_remote() function has two memory management issues:
1. When both 'host' and 'vpnip' are non-NULL, the first getaddrinfo()
allocation is leaked because 'result' is overwritten by the second
getaddrinfo() call without freeing the first allocation.
2. When both 'host' and 'vpnip' are NULL, 'result' is an uninitialized
stack variable passed to freeaddrinfo(), which is undefined behavior.
Fix by initializing 'result' to NULL and calling freeaddrinfo() after
the first getaddrinfo() result is consumed.
Fixes: 959bc330a439 ("testing/selftests: add test tool and scripts for ovpn module")
Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
tools/testing/selftests/net/ovpn/ovpn-cli.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c
index d40953375c86..f4effa7580c0 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -1785,7 +1785,7 @@ static int ovpn_parse_remote(struct ovpn_ctx *ovpn, const char *host,
const char *service, const char *vpnip)
{
int ret;
- struct addrinfo *result;
+ struct addrinfo *result = NULL;
struct addrinfo hints = {
.ai_family = ovpn->sa_family,
.ai_socktype = SOCK_DGRAM,
@@ -1809,6 +1809,8 @@ static int ovpn_parse_remote(struct ovpn_ctx *ovpn, const char *host,
}
memcpy(&ovpn->remote, result->ai_addr, result->ai_addrlen);
+ freeaddrinfo(result);
+ result = NULL;
}
if (vpnip) {
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH net 5/6] ovpn: fix use after free in unlock_ovpn()
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
` (3 preceding siblings ...)
2026-07-20 14:41 ` [PATCH net 4/6] selftests/net: ovpn: fix getaddrinfo memory leak in ovpn_parse_remote() Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
2026-07-20 14:41 ` [PATCH net 6/6] ovpn: use monotonic clock for peer keepalive timeouts Antonio Quartulli
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, Marco Baffo,
Antonio Quartulli
From: Marco Baffo <marco@mandelbit.com>
unlock_ovpn() iterates over the release_list using llist_for_each_entry()
and drops the peer reference inside the loop body via ovpn_peer_put().
If this drops the last reference, the peer is eventually freed. However,
llist_for_each_entry() reads peer->release_entry.next in the loop advance
expression, which runs after the body. By that time the peer may have
already been freed, resulting in a use after free when advancing to the
next list entry.
Fix this by using llist_for_each_entry_safe(), which caches the next
pointer before executing the loop body.
Fixes: 80747caef33d ("ovpn: introduce the ovpn_peer object")
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/peer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 2b6096d8b1cc..8fdbb5050690 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -26,11 +26,12 @@ static void unlock_ovpn(struct ovpn_priv *ovpn,
struct llist_head *release_list)
__releases(&ovpn->lock)
{
- struct ovpn_peer *peer;
+ struct ovpn_peer *peer, *next;
spin_unlock_bh(&ovpn->lock);
- llist_for_each_entry(peer, release_list->first, release_entry) {
+ llist_for_each_entry_safe(peer, next, release_list->first,
+ release_entry) {
ovpn_socket_release(peer);
ovpn_peer_put(peer);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH net 6/6] ovpn: use monotonic clock for peer keepalive timeouts
2026-07-20 14:41 [PATCH net 0/6] pull request: fixes for ovpn 2026-07-20 Antonio Quartulli
` (4 preceding siblings ...)
2026-07-20 14:41 ` [PATCH net 5/6] ovpn: fix use after free in unlock_ovpn() Antonio Quartulli
@ 2026-07-20 14:41 ` Antonio Quartulli
5 siblings, 0 replies; 8+ messages in thread
From: Antonio Quartulli @ 2026-07-20 14:41 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Jakub Kicinski, Paolo Abeni, David S. Miller,
Eric Dumazet, Andrew Lunn, Ralf Lici, Marco Baffo,
Antonio Quartulli
From: Marco Baffo <marco@mandelbit.com>
Replace ktime_get_real_seconds() with the monotonic
ktime_get_boottime_seconds() to ensure the keepalive mechanism is robust
against system clock modifications.
Right now, the driver uses ktime_get_real_seconds() to track peer
timeouts, relying on the system wall-clock.
An administrative time adjustment or an NTP sync that steps the clock
forward can cause `now' to instantly exceed `last_recv + timeout'.
When this occurs, the driver artificially expires healthy peers.
Depending on the OpenVPN user-space configuration, this triggers a
premature tunnel restart (if --keepalive or --ping-restart is used) or
a complete disconnection of the client (if --ping-exit is used).
Fixes: 3ecfd9349f40 ("ovpn: implement keepalive mechanism")
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/io.c | 4 ++--
drivers/net/ovpn/peer.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index a6b777a9c2d9..9a66d693039a 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -142,7 +142,7 @@ void ovpn_decrypt_post(void *data, int ret)
}
/* keep track of last received authenticated packet for keepalive */
- WRITE_ONCE(peer->last_recv, ktime_get_real_seconds());
+ WRITE_ONCE(peer->last_recv, ktime_get_boottime_seconds());
rcu_read_lock();
sock = rcu_dereference(peer->sock);
@@ -294,7 +294,7 @@ void ovpn_encrypt_post(void *data, int ret)
ovpn_peer_stats_increment_tx(&peer->link_stats, orig_len);
/* keep track of last sent packet for keepalive */
- WRITE_ONCE(peer->last_sent, ktime_get_real_seconds());
+ WRITE_ONCE(peer->last_sent, ktime_get_boottime_seconds());
/* skb passed down the stack - don't free it */
skb = NULL;
err_unlock:
diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c
index 8fdbb5050690..a21d02ac715e 100644
--- a/drivers/net/ovpn/peer.c
+++ b/drivers/net/ovpn/peer.c
@@ -45,7 +45,7 @@ static void unlock_ovpn(struct ovpn_priv *ovpn,
*/
void ovpn_peer_keepalive_set(struct ovpn_peer *peer, u32 interval, u32 timeout)
{
- time64_t now = ktime_get_real_seconds();
+ time64_t now = ktime_get_boottime_seconds();
netdev_dbg(peer->ovpn->dev,
"scheduling keepalive for peer %u: interval=%u timeout=%u\n",
@@ -1359,7 +1359,7 @@ void ovpn_peer_keepalive_work(struct work_struct *work)
{
struct ovpn_priv *ovpn = container_of(work, struct ovpn_priv,
keepalive_work.work);
- time64_t next_run = 0, now = ktime_get_real_seconds();
+ time64_t next_run = 0, now = ktime_get_boottime_seconds();
LLIST_HEAD(release_list);
spin_lock_bh(&ovpn->lock);
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread