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

[sashiko-bot@kernel.org]

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