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

[Stefano Garzarella <sgarzare@redhat.com>]

On Fri, Sep 18, 2026 at 12:02:24AM +0200, Bartłomiej Dmitruk wrote:
>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.

Is the VMCI transport supporting namespaces? (CCing Bobby)

>
>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.

2 issues -> 2 fixes, no?

In this case mainly to backport them properly, since for example we 
added namespace support only recently.

Please also add the right Fixes tag and all maintainers in CC.

Stefano

>
>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 <bartlomiej.dmitruk@isec.pl>
>---
>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
>