* [PATCH v2] ovpn: fix peer refcount leak in TCP error paths
@ 2026-05-22 9:17 Pavitra Jha
2026-05-22 9:21 ` Antonio Quartulli
0 siblings, 1 reply; 2+ messages in thread
From: Pavitra Jha @ 2026-05-22 9:17 UTC (permalink / raw)
To: antonio
Cc: sd, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, stable, Pavitra Jha
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>
---
Changes since v1:
- TX path: keep ovpn_peer_hold() unconditional per Antonio Quartulli's
review; only check schedule_work() return value
- Link: https://lore.kernel.org/netdev/20260521083739.65061-1-jhapavitra98@gmail.com/
---
drivers/net/ovpn/tcp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index d651ce85c..2c7d830e7 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -283,9 +283,9 @@ static void ovpn_tcp_send_sock(struct ovpn_peer *peer, struct sock *sk)
/* in case of TCP error we can't recover the VPN
* stream therefore we abort the connection
*/
- if (ovpn_peer_hold(peer))
- if (!schedule_work(&peer->tcp.defer_del_work))
- ovpn_peer_put(peer);
+ ovpn_peer_hold(peer);
+ 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.53.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] ovpn: fix peer refcount leak in TCP error paths
2026-05-22 9:17 [PATCH v2] ovpn: fix peer refcount leak in TCP error paths Pavitra Jha
@ 2026-05-22 9:21 ` Antonio Quartulli
0 siblings, 0 replies; 2+ messages in thread
From: Antonio Quartulli @ 2026-05-22 9:21 UTC (permalink / raw)
To: Pavitra Jha
Cc: sd, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, stable
Hi,
On 22/05/2026 11:17, Pavitra Jha wrote:
> diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
> index d651ce85c..2c7d830e7 100644
> --- a/drivers/net/ovpn/tcp.c
> +++ b/drivers/net/ovpn/tcp.c
> @@ -283,9 +283,9 @@ static void ovpn_tcp_send_sock(struct ovpn_peer *peer, struct sock *sk)
> /* in case of TCP error we can't recover the VPN
> * stream therefore we abort the connection
> */
> - if (ovpn_peer_hold(peer))
> - if (!schedule_work(&peer->tcp.defer_del_work))
> - ovpn_peer_put(peer);
> + ovpn_peer_hold(peer);
> + 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
This patch now lacks the RX part.
Please wait 24h between submissions (i.e. before sending v3).
More comments may come in in the meantime.
Regards,
--
Antonio Quartulli
OpenVPN Inc.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-22 9:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-22 9:17 [PATCH v2] ovpn: fix peer refcount leak in TCP error paths Pavitra Jha
2026-05-22 9:21 ` Antonio Quartulli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®