All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Carlier <devnexen@gmail.com>
To: netdev@vger.kernel.org
Cc: David Carlier <devnexen@gmail.com>,
	Antonio Quartulli <antonio@openvpn.net>,
	Sabrina Dubroca <sd@queasysnail.net>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH net 1/2] ovpn: tcp - use cached peer pointer in ovpn_tcp_close()
Date: Tue, 12 May 2026 05:19:12 +0100	[thread overview]
Message-ID: <20260512042036.19870-2-devnexen@gmail.com> (raw)
In-Reply-To: <20260512042036.19870-1-devnexen@gmail.com>

ovpn_tcp_close() loads the ovpn_socket via rcu_dereference_sk_user_data()
under rcu_read_lock(), takes a reference on sock->peer, caches the peer
pointer in a local, and drops the read lock. It then passes sock->peer
(rather than the cached local) to ovpn_peer_del(), re-dereferencing the
ovpn_socket after the RCU read section has ended.

Unlike ovpn_tcp_sendmsg(), which uses the same "load under RCU, use
after unlock" pattern but is protected by lock_sock() held across the
function, ovpn_tcp_close() runs without the socket lock: inet_release()
invokes sk_prot->close() without taking lock_sock first.

ovpn_socket_release() can therefore complete its kref_put -> detach ->
synchronize_rcu -> kfree(sock) sequence concurrently, in the window
after ovpn_tcp_close() drops rcu_read_lock() but before it dereferences
sock->peer. The synchronize_rcu() in ovpn_socket_release() protects
readers that use the dereferenced pointer inside the RCU read section,
not those that escape the pointer to a local and use it afterwards.

A reproducer follows the pattern of commit 94560267d6c4 ("ovpn: tcp -
don't deref NULL sk_socket member after tcp_close()"): trigger a peer
removal (keepalive expiration or netlink OVPN_CMD_DEL_PEER) at the same
moment userspace closes the TCP fd. That commit fixed the detach-side
of the same race window; this one fixes the close-side at a different
victim.

Use the already-loaded peer local, which is held by the
ovpn_peer_hold() taken under RCU and is the correct argument anyway.
The remaining lines in the function already use peer; switching this
call makes the function consistent and removes the dangling sock
dereference.

Fixes: 11851cbd60ea ("ovpn: implement TCP transport")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/net/ovpn/tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
index 65054cc84be5..ed4782de141a 100644
--- a/drivers/net/ovpn/tcp.c
+++ b/drivers/net/ovpn/tcp.c
@@ -588,7 +588,7 @@ static void ovpn_tcp_close(struct sock *sk, long timeout)
 	peer = sock->peer;
 	rcu_read_unlock();
 
-	ovpn_peer_del(sock->peer, OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT);
+	ovpn_peer_del(peer, OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT);
 	peer->tcp.sk_cb.prot->close(sk, timeout);
 	ovpn_peer_put(peer);
 }
-- 
2.53.0


  reply	other threads:[~2026-05-12  4:20 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12  4:19 [PATCH net 0/2] ovpn: fix TCP teardown UAF races David Carlier
2026-05-12  4:19 ` David Carlier [this message]
2026-05-12  4:29   ` [PATCH net 1/2] ovpn: tcp - use cached peer pointer in ovpn_tcp_close() Eric Dumazet
2026-05-12  4:56     ` David CARLIER
2026-05-12  7:29       ` Antonio Quartulli
2026-05-12 13:55       ` Antonio Quartulli
2026-05-12 14:11         ` Sabrina Dubroca
2026-05-12 14:17           ` Antonio Quartulli
2026-05-12 15:04             ` Sabrina Dubroca
2026-05-12  4:19 ` [PATCH net 2/2] ovpn: respect peer refcount in CMD_NEW_PEER error path David Carlier
2026-05-12  7:33   ` Antonio Quartulli
2026-05-12 15:13   ` Sabrina Dubroca
2026-05-13  9:10     ` Antonio Quartulli
2026-05-13 10:55 ` [PATCH net v2 0/2] ovpn: fix TCP teardown UAF races David Carlier
2026-05-14 14:20   ` Antonio Quartulli
2026-05-13 10:55 ` [PATCH v2 1/2] ovpn: tcp - use cached peer pointer in ovpn_tcp_close() David Carlier
2026-05-13 10:55 ` [PATCH v2 2/2] ovpn: respect peer refcount in CMD_NEW_PEER error path David Carlier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260512042036.19870-2-devnexen@gmail.com \
    --to=devnexen@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=antonio@openvpn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.