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