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

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

[Bartłomiej Dmitruk]

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

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

[Vishnu Dasa]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

On Thu, Sep 17, 2026 at 3:02 PM Bartłomiej Dmitruk
<bartlomiej.dmitruk@isec.pl> 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.
>
> 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 <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;

Shouldn't this be after the vsock_net_mode_global check below?

Thanks for the fix!

- Vishnu

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5417 bytes --]

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

[Stefano Garzarella]

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
>

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/vsock: severe receive fast-path performance regression
--

commit d73a7bc02ed525e8162b0e1c12fa4585af2535c6
Author: Bartłomiej Dmitruk <bartlomiej.dmitruk@isec.pl>

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

This patch fixes a namespace bypass by enforcing vsock_net_mode_global
on the receive path. It also removes the unsynchronized vsock->cached_peer
cache to avoid data races between the lockless receive tasklet and the send
path.

> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 1f186e8f83640..37b6f6eca962b 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -529,18 +529,25 @@ static bool vmci_transport_allow_dgram(struct vsock_sock *vsock, u32 peer_cid)
>  	if (VMADDR_CID_HYPERVISOR == peer_cid)
>  		return true;

[Severity: Medium]
This is a pre-existing issue, but does this early return for the hypervisor
CID skip the newly added netns isolation check below?

A socket bound to a non-global netns could still receive datagrams from the
hypervisor while being blocked from sending to it, which would leave a
namespace isolation boundary partially bypassed.

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

[Severity: High]
Does evaluating this unconditionally for every incoming datagram cause a severe
receive fast-path performance regression?

Both vmci_transport_is_trusted and vmci_context_get_priv_flags call
vmci_ctx_get, which executes a linear scan of the global ctx_list under
rcu_read_lock:

drivers/misc/vmw_vmci/vmci_context.c:vmci_ctx_get() {
    ...
    rcu_read_lock();
    list_for_each_entry_rcu(c, &ctx_list.head, list_item) {
    ...
}

Executing two O(N) list traversals per datagram in the bottom-half (tasklet)
receive path could cause high CPU overhead, increase latency, and risk soft
lockups under high packet rates, replacing what was previously an O(1) cache
check.

> +		return false;
> +
> +	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917220225.56200-1-bartlomiej.dmitruk@isec.pl?part=1

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new per-netns gate in vmci_transport_allow_dgram()
  (net/vmw_vsock/vmci_transport.c:536) is placed after the unconditional…
- [High] net/vsock: severe receive fast-path performance regression
- [Medium] Missing `Fixes:` and `Cc: stable@vger.kernel.org` trailers for
  two distinct, user-visible defects.
- [Low] After this patch, include/net/af_vsock.h:42-45 still defines and
  documents `bool cached_peer_allow_dgram; /* Dgram communication…

--

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

This adds a per-netns mode check to vmci_transport_allow_dgram() on the
datagram receive path and removes the cached_peer/cached_peer_allow_dgram
caching so the access decision is recomputed for every datagram.  A few
questions below about the placement of the new check, the commit trailers,
and the now unused struct members.

On the commit message:

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

[Severity: Medium]
Should this carry Fixes: tags, and Cc: stable for the namespace half?

The receive-path netns gap only exists from the commit that introduced
vsock_net_mode_global() — git blame of include/net/af_vsock.h points at
eafb64f40ca4 ("vsock: add netns to vsock core"):

  static inline bool vsock_net_mode_global(struct vsock_sock *vsk)
  {
          return vsock_net_mode(sock_net(sk_vsock(vsk))) == VSOCK_NET_MODE_GLOBAL;
  }

The cached_peer check-then-set predates that and goes back to the original
VMCI transport.

A sibling netns fix already in this tree, 2ec28c09b320 ("vsock: ignore empty
child namespace mode writes"), carries Fixes: eafb64f40ca4 plus
Cc: stable@vger.kernel.org.  Without equivalent trailers here, how are
backporters expected to know which trees still have the bypass?

> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 1f186e8f83640..37b6f6eca962b 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -529,18 +529,25 @@ static bool vmci_transport_allow_dgram(struct vsock_sock *vsock, u32 peer_cid)
>  	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;

[Severity: High]
Is the new check reachable for peer_cid == VMADDR_CID_HYPERVISOR?

The hypervisor early return above it is unconditional, so in
vmci_transport_allow_dgram() the netns mode is never consulted for CID 0.

The send hook the comment names does the opposite ordering:

net/vmw_vsock/vmci_transport.c:vmci_transport_dgram_allow() {
	if (!vsock_net_mode_global(vsk))
		return false;

	if (cid == VMADDR_CID_HYPERVISOR) {
		/* Registrations of PBRPC Servers do not modify VMX/Hypervisor
		 * state and are allowed.
		 */
		return port == VMCI_UNITY_PBRPC_REGISTER;
	}

	return true;
}

So a SOCK_DGRAM socket in a local-mode netns still accepts hypervisor-sourced
datagrams while its own sends to the hypervisor are refused, which looks like
the exact case the comment says must not happen ("a socket in a non-global
(local) netns must not receive datagrams it could never send").

Nothing appears to keep such a socket from registering the receive callback:
vsock_assign_transport() picks transport_dgram for SOCK_DGRAM with no netns
check, and __vsock_bind_dgram() -> vmci_transport_dgram_bind() registers
vmci_transport_recv_dgram_cb() unconditionally.

Then vmci_transport_allow_dgram() is the only gate before the datagram is
queued:

net/vmw_vsock/vmci_transport.c:vmci_transport_recv_dgram_cb() {
	vsk = vsock_sk(sk);
	if (!vmci_transport_allow_dgram(vsk, dg->src.context))
		return VMCI_ERROR_NO_ACCESS;
	...
}

And hypervisor-sourced datagrams do reach ordinary handlers; only the event
resource is special-cased:

drivers/misc/vmw_vmci/vmci_guest.c {
	if (dg->src.context == VMCI_HYPERVISOR_CONTEXT_ID &&
	    dg->dst.resource == VMCI_EVENT_HANDLER) {
		result = vmci_event_dispatch(dg);
	} else {
		result = vmci_datagram_invoke_guest_handler(dg);
	}
}

Would moving the vsock_net_mode_global() test above the hypervisor early
return be the intended shape here?  If a receive-side exception for CID 0 is
deliberate, could the comment say so, and should it mirror the send side's
VMCI_UNITY_PBRPC_REGISTER port restriction?

>  
> -	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.
> +	 */

[Severity: Low]
This isn't a bug, but could the two struct members go away in this same
patch rather than a follow-up?  After this change, a tree-wide grep for
cached_peer matches only the two definitions and this new prose comment:

include/net/af_vsock.h:struct vsock_sock {
	...
	/* Accessed without the socket lock held. This means it can never be
	 * modified outsided of socket create or destruct.
	 */
	bool trusted;
	bool cached_peer_allow_dgram;	/* Dgram communication allowed to
					 * cached peer?
					 */
	u32 cached_peer;  /* Context ID of last dgram destination check. */
	...
}

The block that this patch deletes was the only reader and writer, so the
header now documents per-socket ACL caching state that no code maintains.

> +	if (!vmci_transport_is_trusted(vsock, peer_cid) &&
> +	    (vmci_context_get_priv_flags(peer_cid) &
> +	     VMCI_PRIVILEGE_FLAG_RESTRICTED))
> +		return false;
> +
> +	return true;
>  }

[Severity: High]
Was the per-datagram cost of dropping the cache measured?

vmci_transport_allow_dgram() is called from
vmci_transport_recv_dgram_cb(), i.e. from the VMCI receive tasklet,
once for every incoming datagram.  After this change both remaining
tests can walk the global context list:

  vmci_transport_is_trusted() -> vmci_is_context_owner() -> vmci_ctx_get()
  vmci_context_get_priv_flags() -> vmci_ctx_get()

and vmci_ctx_get() is a linear search:

drivers/misc/vmw_vmci/vmci_context.c:vmci_ctx_get() {
	rcu_read_lock();
	list_for_each_entry_rcu(c, &ctx_list.head, list_item) {
		if (c->cid == context_id) {
	...
}

For an untrusted socket vsock->trusted is false, so the && does not
short-circuit and both lookups run: two traversals proportional to the
number of registered VMCI contexts, per packet, in softirq context,
where the cached_peer path used to be one u32 compare.  On a host with
many contexts that is new fast-path work; in a guest
vmci_host_code_active() is false so vmci_context_get_priv_flags()
returns early, but vmci_is_context_owner() is still on the path.

I am not defending the racy check-then-set — the interleave you
describe is real, and the stale-allow outcome is worse than the cost.
But is removing the cache outright the only way out?  cached_peer and
cached_peer_allow_dgram could be collapsed into a single word (cid plus
an allow bit) accessed with READ_ONCE()/WRITE_ONCE(), which kills the
torn/mismatched-pair problem while keeping the common case O(1) —
though that still returns a stale verdict if the peer's privilege flags
change after the first datagram, so perhaps you concluded that any
caching is unsound here.

Could you say which it is in the changelog, and give some idea of the
impact (datagram rate, context count) of evaluating on every packet?
The cache was presumably added for a reason, and a note that VMCI
datagram receive is slow enough for two context lookups not to matter
would make the trade-off reviewable.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917220225.56200-1-bartlomiej.dmitruk%40isec.pl