Thread (4 messages) flat view 4 messages, 4 authors, 3d ago
WARM3d

[PATCH] vsock/vmci: harden datagram receive access check (netns bypass + cached_peer race)

From: Bartłomiej Dmitruk <hidden>
Date: 2026-09-17 22:02:30
Also in: virtualization
Subsystem: networking [general], the rest, vm sockets (af_vsock), vmware vsock vmci transport driver · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Stefano Garzarella, Bryan Tan, Vishnu Dasa

vmci_transport_allow_dgram() gates delivery of incoming VMCI datagrams. It
has two problems, both in this one function, fixed together here.

1. Namespace bypass: the send hook vmci_transport_dgram_allow() refuses
   datagrams when the socket's netns is not in global mode
   (vsock_net_mode_global()), but the receive path never checks it. A socket
   bound in a non-global (local) netns therefore receives datagrams from
   peers it could never send to, defeating namespace isolation.

2. Data race / stale ACL: the decision was cached in vsock->cached_peer and
   vsock->cached_peer_allow_dgram with an unsynchronized check-then-set.
   The function is called both from the lockless receive tasklet
   (vmci_transport_recv_dgram_cb(), no socket lock) and from the lock_sock()
   send path; lock_sock() does not exclude bottom halves, so the two contexts
   race on those fields. An interleave can return a stale 'allow' for a
   VMCI_PRIVILEGE_FLAG_RESTRICTED peer (and it is a plain data race
   regardless). The in-code comment claiming the fields are never modified
   outside create/destruct is contradicted by the send path.

Add the symmetric net-mode check and drop the cache, evaluating the decision
on every datagram; there is no shared mutable state left to race. The now
unused cached_peer{,_allow_dgram} members can be removed in a follow-up.

Signed-off-by: Bartłomiej Dmitruk <redacted>
---
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -529,18 +529,25 @@
 	if (VMADDR_CID_HYPERVISOR == peer_cid)
 		return true;
 
-	if (vsock->cached_peer != peer_cid) {
-		vsock->cached_peer = peer_cid;
-		if (!vmci_transport_is_trusted(vsock, peer_cid) &&
-		    (vmci_context_get_priv_flags(peer_cid) &
-		     VMCI_PRIVILEGE_FLAG_RESTRICTED)) {
-			vsock->cached_peer_allow_dgram = false;
-		} else {
-			vsock->cached_peer_allow_dgram = true;
-		}
-	}
+	/* Enforce the per-netns mode on the receive path, symmetrically with
+	 * the send hook vmci_transport_dgram_allow(): a socket in a non-global
+	 * (local) netns must not receive datagrams it could never send.
+	 */
+	if (!vsock_net_mode_global(vsock))
+		return false;
 
-	return vsock->cached_peer_allow_dgram;
+	/* Evaluate on every datagram instead of caching the decision in
+	 * vsock->cached_peer{,_allow_dgram}: those fields were an
+	 * unsynchronized check-then-set shared between the lockless receive
+	 * tasklet and the lock_sock() send path, which could return a stale
+	 * 'allow' for a restricted peer.
+	 */
+	if (!vmci_transport_is_trusted(vsock, peer_cid) &&
+	    (vmci_context_get_priv_flags(peer_cid) &
+	     VMCI_PRIVILEGE_FLAG_RESTRICTED))
+		return false;
+
+	return true;
 }
 
 static int
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help