From patchwork Mon Jun 8 13:32:47 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 5004 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:bc1d:b0:861:c897:cb9d with SMTP id jc29csp1885198mab; Mon, 8 Jun 2026 06:33:12 -0700 (PDT) X-Forwarded-Encrypted: i=2; AFNElJ8H15yb6H4rt4IsmjVQTG5WLO3QlG3uPxUfzGgNAxBl+L4XOeJCh0x0OwWdejHYC0eDz3/x0eA2hzI=@openvpn.net X-Received: by 2002:a05:6830:83b7:b0:7e6:cc17:c7ac with SMTP id 46e09a7af769-7e70caaf882mr10306268a34.22.1780925592482; Mon, 08 Jun 2026 06:33:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1780925592; cv=none; d=google.com; s=arc-20240605; b=Qjsgg7+X3dW5OcC90jxRUJDkfGMNBzG9F3KV+7d2stxZPgmEXcH0AHX8YqfaOruCcA smdXQPBiaM/xIQt9UfNKKV2Lm3HfDm1+GIS6ThyMoi4Pb30evlQDeImisTMfaEplq6Mc NsCKAlCIi11XdbjXAR9FLiNTkN7dUzMHMELHbH7syYgszs4QmopXMdIWzrG7IqrQHSDR 0Y6fciKPYA2oD64/MNDjeAPZMeTORsAgndZS2+A3jXYvbg2sKcqqXsegVYXQnKgCiU7H Zn2DP5ZviFMdyP59RH0pd3DFl1tpXUP+Io3oHFGkuynn9BgcMkDHmNZE3yngC4QP4nG1 18+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:cc:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature:dkim-signature; bh=mL4bJ+LSxd0VnrN4IeDEMjQG0xGPmibO1kB+/h5XxZQ=; fh=BsMg/B0Yb/hS/rzP5Npz4luh0IleZm8REk1XWiWRt2A=; b=h210cH18yZkxW0WAOMal6pChx30Uhdlo6STiSj1W6SWtSmQlDOviiU3r9blgww1LoF wzTJ4L3qCB4SlFEn707gi1u25vjr5DamTRdPVPavNxCxzUbQMDMeIH/KIMJiKYvTo0cA +NCrsrxZx6Gc6DY1a2vB+1ZZEV4MpWclKfE12RDAdoIFBYaQ5q0aIdIoKy2SkW7yYcK5 rSLCOhqYqDKIUzZq+rsAkUZqMNjoHrWy/o5dHmoBYYLFxSascsfckH6uLwSydJ/hr2aG IkeudB5lHM30sDzr4HpA/WOce+1wl+4oD5R0JZU+y9ERn5Vwiq/PcYkR/8XoP2A+rgNo VM6Q==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=Zu3LaDb9; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=Tef9WFJJ; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=Aggwh8yS; dkim=neutral (body hash did not verify) header.i=@unstable.cc header.s=MBO0001 header.b=rZkv6LnH; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-440d88cd231si13690747fac.372.2026.06.08.06.33.12 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Jun 2026 06:33:12 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=Zu3LaDb9; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=Tef9WFJJ; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=Aggwh8yS; dkim=neutral (body hash did not verify) header.i=@unstable.cc header.s=MBO0001 header.b=rZkv6LnH; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mL4bJ+LSxd0VnrN4IeDEMjQG0xGPmibO1kB+/h5XxZQ=; b=Zu3LaDb9hcLA3/CcfqApI6QhIp Xmn4JDOBzuIAPQpu8cWNurb712s9uZFKu1JFCI9Why9+XyqmxfG1uzZRYTSfweUItyk4UajvbEj4u LJeOvycczSldiy8J+mqoQkvhRkYb32b+5BarWr/J1KXgeCVqAkIvYs7tbN3K1LQM99WQ=; Received: from [127.0.0.1] (helo=sfs-ml-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1wWa6G-0001Cf-Ty; Mon, 08 Jun 2026 13:33:09 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1wWa6E-0001CW-Bv for openvpn-devel@lists.sourceforge.net; Mon, 08 Jun 2026 13:33:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=3SO51xGxkJt8zJ1G/O5z8R+QSQzRGqAq8QHjtl6WKcA=; b=Tef9WFJJzXmDbbIvj26bqi9uCW M2Z7adQpNy4P/lBnbzM/7xvE2e39b+nfxpBPRGUCu6cV0f7w+FsYVxQdZt5YbyeMG266FDLIQvTol CESObqjRbHLvCRXTJywA/nFrBO21SmFsvwfsHPIGZFF4GSLEtJCFnxtAy+RHwRwnVZ2I=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=3SO51xGxkJt8zJ1G/O5z8R+QSQzRGqAq8QHjtl6WKcA=; b=Aggwh8ySeEnOSyoOcOi9kUAfH7 cRu/nlIWVCR6MEn3kTNDNpXSYkkMSwOuZcmkfC3ZwtRktEVj/oPwi9FOvzqtD6gM8+m2RBtIaibzs JHTUG0UjjEPvK+CmlW5j3a2lm4UwapwABkiDHq2a2p9fpmoRQgQiyJ5SVM/UhX5RMZNw=; Received: from mout-p-102.mailbox.org ([80.241.56.152]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1wWa6D-0004wN-2V for openvpn-devel@lists.sourceforge.net; Mon, 08 Jun 2026 13:33:07 +0000 Received: from smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4gYtJn5K5yz9vJ5; Mon, 8 Jun 2026 15:32:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=unstable.cc; s=MBO0001; t=1780925577; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3SO51xGxkJt8zJ1G/O5z8R+QSQzRGqAq8QHjtl6WKcA=; b=rZkv6LnHmCoMihylRUSIequejeDam/iUb9WFnpdd9jFt5E2JoD5DLArBy9oD1f28jYa+Ev DV80DvFqBOvwFNgptBKThjZ46YYLT4cFu2GLLzpy4NeZKVTnp/J+blfPhDXSVW53Yu6e+J tLqvXMZJK8l8Ea9HRkuKSa0PcC6W5jyogofhHSR6KEedRi9PqaApzKBXZ6Nd07dXQgcOS5 eU5HBh/bB2J6B27mOHKaBjofAr537xIv2RXs/NzfoINXeECPpDokMWsGlqiezrbftybxtg JL7D7x3YFjbrToWIh/lxjU22okFpEic+exPSbhJmgv7KwVTFFIpbSQc1BHK55A== From: Antonio Quartulli To: openvpn-devel@lists.sourceforge.net Date: Mon, 8 Jun 2026 15:32:47 +0200 Message-ID: <20260608133251.3128542-5-a@unstable.cc> In-Reply-To: <20260608133251.3128542-1-a@unstable.cc> References: <20260608133251.3128542-1-a@unstable.cc> MIME-Version: 1.0 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-2.hosts.colo.sdot.me", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Antonio Quartulli The TCP error paths in ovpn_tcp_rcv() and ovpn_tcp_send_sock() take a peer reference and then schedule the deferred-delete work: ovpn_peer_hold(peer); schedule_work(&peer->tcp.defer_del_work); Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-Headers-End: 1wWa6D-0004wN-2V Subject: [Openvpn-devel] [PATCH ovpn net v2 5/9] ovpn: tcp - fix peer reference leak on deferred deletion X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Antonio Quartulli Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: 1867435833650505285 X-GMAIL-MSGID: 1867435833650505285 From: Antonio Quartulli The TCP error paths in ovpn_tcp_rcv() and ovpn_tcp_send_sock() take a peer reference and then schedule the deferred-delete work: ovpn_peer_hold(peer); schedule_work(&peer->tcp.defer_del_work); ovpn_tcp_peer_del_work() drops exactly one reference per run, but schedule_work() returns false and does not re-queue when the work is already pending. The reference, however, was taken unconditionally, so every hold+schedule that lands on an already-pending work leaks one peer reference. ovpn_tcp_rcv() is the strparser receive callback and has no guard against this: a TCP segment packed with packets whose length header is valid for the stream parser but whose payload is smaller than the opcode size passes ovpn_tcp_parse() and hits the error path. strparser delivers all complete messages in a loop, so many error invocations run before the scheduled work executes, leaking one reference each. A remote peer can exploit this to pin the peer (and the netdev reference it holds) forever, preventing interface teardown - a denial of service. Take the reference only when schedule_work() actually queues the work. schedule_work() flips the work pending bit atomically, so exactly one caller - even across the concurrent RX and TX paths - observes the idle->pending transition and acquires the single reference that the lone ovpn_peer_put() in the worker balances. ovpn_peer_del() is idempotent (ovpn_peer_remove() bails on an already-unhashed peer), so a work item re-queued while running stays refcount-balanced. Fixes: 11851cbd60ea ("ovpn: implement TCP transport") Signed-off-by: Antonio Quartulli --- drivers/net/ovpn/tcp.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c index 433bd07a4f1b..6cf684699ada 100644 --- a/drivers/net/ovpn/tcp.c +++ b/drivers/net/ovpn/tcp.c @@ -148,10 +148,14 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb) ovpn_recv(peer, skb); return; err: - /* 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); + /* schedule deferred peer deletion and take a reference only if the + * work was actually queued: the matching ovpn_peer_put() in + * ovpn_tcp_peer_del_work() runs once per queued work, so re-arming an + * already-pending work must not take another reference (it would be + * leaked, e.g. on a flood of invalid packets) + */ + if (schedule_work(&peer->tcp.defer_del_work)) + ovpn_peer_hold(peer); ovpn_dev_dstats_rx_dropped(peer->ovpn->dev); err_nopeer: kfree_skb(skb); @@ -280,15 +284,20 @@ static void ovpn_tcp_send_sock(struct ovpn_peer *peer, struct sock *sk) peer->id, ret); /* in case of TCP error we can't recover the VPN - * stream therefore we abort the connection + * stream therefore we abort the connection. + * + * Take a reference only if the work was actually + * queued: ovpn_tcp_peer_del_work() drops exactly one + * reference per run, so re-arming an already-pending + * work (e.g. already scheduled from the RX path) must + * not take another reference (it would be leaked). */ - ovpn_peer_hold(peer); - schedule_work(&peer->tcp.defer_del_work); + if (schedule_work(&peer->tcp.defer_del_work)) + ovpn_peer_hold(peer); /* we bail out immediately and keep tx_in_progress set - * to true. This way we prevent more TX attempts - * which would lead to more invocations of - * schedule_work() + * to true, so that no further TX is attempted on the + * aborted stream */ return; }