On Thu, Sep 17, 2026 at 3:02 PM Bartłomiej Dmitruk <[email protected]> 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 <[email protected]> > --- > 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
smime.p7s
Description: S/MIME Cryptographic Signature
