[PATCH v2] ovpn: fix peer refcount leak in TCP error paths

Pavitra Jha posted 1 patch 2 days, 8 hours ago
There is a newer version of this series
drivers/net/ovpn/tcp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH v2] ovpn: fix peer refcount leak in TCP error paths
Posted by Pavitra Jha 2 days, 8 hours ago
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
Re: [PATCH v2] ovpn: fix peer refcount leak in TCP error paths
Posted by Antonio Quartulli 2 days, 8 hours ago
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.