Re: [PATCH RFC 6/8] vsock: allow datagrams to use directional transports
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-28 16:34:31
Also in:
bpf, kvm, virtualization
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] vsock/virtio: bypass of checks in virtio_transport_dgram_source_allow()
- [Medium] vsock: vsock_dgram_sendmsg() mutates connected vsk->remote_addr on fallback
--
commit 634f5c11de9d307a1a88027855c76c28c15f7d96
Author: Sergio Lopez [off-list ref]
vsock: allow datagrams to use directional transports
This commit enables AF_VSOCK datagram sockets to initialize through and
route across directional transports (such as virtio, vhost, and loopback)
when a dedicated datagram transport is not registered.
quoted hunk ↗ jump to hunk
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index dc1add2d2e896..6419498a6fe27 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1493,7 +1606,8 @@ static int vsock_dgram_sendmsg(struct socket *sock, struct msghdr *msg, remote_addr = &vsk->remote_addr; if (remote_addr->svm_cid == VMADDR_CID_ANY) - remote_addr->svm_cid = transport->get_local_cid(); + remote_addr->svm_cid = transport->dgram_transport_match ? + VMADDR_CID_LOCAL : transport->get_local_cid(); /* XXX Should connect() or this function ensure remote_addr is * bound?@@ -1507,13 +1621,14 @@ static int vsock_dgram_sendmsg(struct socket *sock, struct msghdr *msg, goto out; } - if (!transport->dgram_allow(vsk, remote_addr->svm_cid, - remote_addr->svm_port)) { - err = -EINVAL; + transport = vsock_dgram_get_transport(vsk, remote_addr); + if (IS_ERR(transport)) { + err = PTR_ERR(transport); goto out; }
[Severity: Medium] When vsock_dgram_sendmsg() is called without msg_name on a connected socket, remote_addr points directly to &vsk->remote_addr. If the socket connected to an active H2G guest CID (leaving vsk->remote_addr.svm_flags without VMADDR_FLAG_TO_HOST) and that guest is temporarily unavailable while g2h_fallback is enabled, vsock_dgram_get_transport() takes the fallback path and mutates remote_addr in place: net/vmw_vsock/af_vsock.c:vsock_dgram_get_transport() { ... } else if (sock_net(sk)->vsock.g2h_fallback && transport_g2h && transport_g2h->has_remote_cid && transport_g2h->has_remote_cid(vsk, cid)) { transport = transport_g2h; to_host = true; } ... if (to_host) remote_addr->svm_flags |= VMADDR_FLAG_TO_HOST; } Because vsock_dgram_get_transport() checks VMADDR_FLAG_TO_HOST before transport_h2g->has_remote_cid(), won't this permanently pin vsk->remote_addr to transport_g2h for subsequent sends, receive source checks, and getpeername() even after the H2G guest restarts?
quoted hunk ↗ jump to hunk
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index ae33630d99e71..7c1ec8feb853c 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c@@ -322,13 +322,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, info->type = virtio_transport_get_type(sk_vsock(vsk)); - t_ops = virtio_transport_get_ops(vsk); + t_ops = info->t_ops ?: virtio_transport_get_ops(vsk); if (unlikely(!t_ops)) return -EFAULT; src_cid = t_ops->transport.get_local_cid(); + /* Preserve explicit datagram bindings so loopback replies can find them. */ + if (info->type == VIRTIO_VSOCK_TYPE_DGRAM && + src_cid == VMADDR_CID_LOCAL && + vsk->local_addr.svm_cid != VMADDR_CID_ANY) + src_cid = vsk->local_addr.svm_cid; src_port = vsk->local_addr.svm_port;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1861,6 +1877,33 @@ static bool virtio_transport_valid_type(u16 type) (type == VIRTIO_VSOCK_TYPE_DGRAM); } +static bool +virtio_transport_dgram_source_allow(struct vsock_sock *vsk, + const struct vsock_transport *transport, + const struct sockaddr_vm *src) +{ + const struct vsock_transport *peer_transport; + struct sockaddr_vm peer = vsk->remote_addr; + bool allow; + + if (!virtio_transport_dgram_transport_match(vsk->transport) || + !virtio_transport_dgram_transport_match(transport)) + return false; + /* Shutdown preserves the peer; only disconnect removes it. */ + if (sk_vsock(vsk)->sk_socket->state == SS_UNCONNECTED) + return true; + + peer_transport = vsock_dgram_get_transport(vsk, &peer); + if (IS_ERR(peer_transport)) + return false; + allow = peer_transport == transport && + src->svm_port == vsk->remote_addr.svm_port && + (src->svm_cid == vsk->remote_addr.svm_cid || + transport->get_local_cid() == VMADDR_CID_LOCAL); + module_put(peer_transport->module); + return allow; +} + /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex * lock. */@@ -1911,14 +1954,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, vsk = vsock_sk(sk); lock_sock(sk); if (sock_flag(sk, SOCK_DEAD) || - vsk->transport != &t->transport || - (sk->sk_socket->state != SS_UNCONNECTED && - !vsock_check_source(vsk, &t->transport, &src))) { + !virtio_transport_dgram_source_allow(vsk, &t->transport, &src)) { release_sock(sk); sock_put(sk); goto free_pkt; }
[Severity: High] Can virtio_transport_dgram_source_allow() bypass feature negotiation, local CID transport isolation, and connected peer CID checks? First, for SS_UNCONNECTED sockets, virtio_transport_dgram_source_allow() returns true immediately after virtio_transport_dgram_transport_match(), which is a static callback comparison that succeeds for virtio_transport, vhost_transport, and loopback_transport without calling transport->dgram_allow(vsk, src->svm_cid, src->svm_port). Since neither vhost_vsock_handle_tx_kick() nor virtio_transport_rx_work() checks vsock->dgram_allow before calling virtio_transport_recv_pkt(), a peer that has not negotiated VIRTIO_VSOCK_F_DGRAM can deliver VIRTIO_VSOCK_TYPE_DGRAM packets to any unconnected bound socket on the host (bypassing vhost_transport_dgram_allow()) or on a guest initialized via the vhost_transport fallback (bypassing virtio_transport_dgram_allow_check()). Second, returning true early for SS_UNCONNECTED sockets skips checking whether vsk->local_addr.svm_cid is compatible with the receiving transport. Because virtio_transport_rx_work() does not validate hdr->dst_cid against the guest CID and __vsock_find_bound_socket_net() matches dst.svm_cid == vsk->local_addr.svm_cid or VMADDR_CID_ANY, an external peer on virtio_transport can send datagrams addressed to VMADDR_CID_LOCAL, VMADDR_CID_HOST, or VMADDR_CID_ANY and reach an unconnected socket bound to VMADDR_CID_LOCAL or VMADDR_CID_HOST. Third, on loopback_transport, virtio_transport_send_pkt_info() now preserves explicit vsk->local_addr.svm_cid values in hdr->src_cid, and __vsock_find_bound_socket_net() allows distinct local sockets to bind to the same port on different local CIDs (such as VMADDR_CID_LOCAL and the guest or host CID). Because transport->get_local_cid() == VMADDR_CID_LOCAL is always true for loopback_transport, won't a local socket bound to (VMADDR_CID_LOCAL, port) be accepted by a socket connected to (guest_cid, port) despite having a mismatched src->svm_cid? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261028-vsock-dgram-support-v1-0-e457cf7cee78@sinrega.org?part=6