[PATCH RFC net v2 0/3] vsock/virtio: don't let sockets send from another local CID

[PATCH RFC net v2 0/3] vsock/virtio: don't let sockets send from another local CID

[Yiran Qiu]

The virtio transports send every packet from the transport's CID, so a
socket bound to another local CID sends from another socket's address.
Patch 1 refuses connect() on such a socket; patches 2 and 3 close two
other ways a socket can send from another address or have its local
CID changed.

I'm sending this as an RFC because one problem is still open.  Over
vsock_loopback, a socket bound to (VMADDR_CID_LOCAL, P) sends from the
same address as the children of a listener on (guest CID, P) -- or, on
a host, on (VMADDR_CID_HOST, P) -- and vsock_check_source()
(dee44f41f206) accepts packets from either one as coming from the
peer.  So either one can shut down or stall the other's connections,
including those of clients that never called bind().  Making
vsock_loopback send from the bound CID would fix this and make patch 1
unnecessary for loopback.  Would you prefer that, or these patches as
they are?

Changes in v2:
- New approach.  v1 made bind() treat the same port on two local CIDs
  as a conflict, but as the Sashiko review pointed out, its alias list
  missed VMADDR_CID_HOST and did not handle guest->host transports
  without a CID.  I reproduced both gaps.  v2 leaves bind() alone.
- Patches 2 and 3 are new.
- Only the virtio transports change, vsock_loopback included.  VMCI
  picks its source context by route (a nested VMware guest reaches its
  VMs from VMADDR_CID_HOST), and hv_sock has no CIDs.
- v1: https://patch.msgid.link/20261028065231.655701-1-eritque-arcus@ikuyo.dev

I tested on net 4dca97647a3e (net/vmw_vsock is unchanged through
5838f10563d5) under QEMU/KVM with vsock_loopback built in, in four
setups: guests with vhost_vsock, with and without the virtio-vsock
device; a guest with the device but no vhost_vsock; and a host with
vhost_vsock and no virtio transport.  The table shows where each
problem reproduces as the patches are applied one at a time ("no dev"
is the guest without the device):

                       base          +1            +1+2     +1+2+3
  connect() reset      all           fixed         fixed    fixed
  credit update stall  all           all           fixed    fixed
  rebind to CID ANY    no dev, host  no dev, host  no dev   fixed

vsock_test passes at every step, and no connect() that used to succeed
is now refused.  I have not tested VMware, Hyper-V or nested setups,
nor run with KASAN, and there is no vsock_test case yet; I can add one.

The patch 1 problem was found with afb-afl, an LLM-driven audit
pipeline, and this letter was drafted with Claude.

Yiran Qiu (3):
  vsock/virtio: refuse connect() from a CID the transport does not own
  vsock/virtio: send credit updates only while connecting or connected
  vsock/virtio: only update a socket's local CID to the transport's CID

 net/vmw_vsock/virtio_transport_common.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

-- 
2.56.0

[PATCH RFC net v2 1/3] vsock/virtio: refuse connect() from a CID the transport does not own

[Yiran Qiu]

Since commit c0cfa2d8a788 ("vsock: add multi-transports support"),
bind() accepts any local CID, but connect() picks the transport from the
remote CID only.  The virtio transports send from the transport's CID,
so a socket bound to another local CID sends as if it were bound to
the same port on the transport's CID, and its connect() never
completes.

Over vsock_loopback, one process can use this against another that is
bound to (VMADDR_CID_LOCAL, P):
 1. it binds to (guest CID, P) and connects to a local listener; the
    request arrives from (VMADDR_CID_LOCAL, P) and leaves behind a
    child socket for that address, which stays until the listener
    accepts and closes it;
 2. the other process's connect() to the same listener matches that
    child and fails with ECONNRESET.

Refuse such a connect() with EADDRNOTAVAIL, as bind() does for a CID
that is not local.  And whenever the virtio connect() fails, set the
socket to TCP_CLOSE, as vmci_transport_connect() does, instead of
leaving it in TCP_SYN_SENT.

Found by an LLM-driven code audit.  I reproduced it on unmodified
kernels with the two processes running as different users, without
error injection, and tested the fix with the reproducer and vsock_test
in guests and on a host.  Drafted with Claude.

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
 net/vmw_vsock/virtio_transport_common.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 1e762a480df4..8f524d073ae1 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1219,8 +1219,20 @@ int virtio_transport_connect(struct vsock_sock *vsk)
 		.vsk = vsk,
 		.net = sock_net(sk_vsock(vsk)),
 	};
+	u32 local_cid;
+	int ret;
 
-	return virtio_transport_send_pkt_info(vsk, &info);
+	local_cid = vsk->transport->get_local_cid();
+	if (local_cid != VMADDR_CID_ANY &&
+	    vsk->local_addr.svm_cid != VMADDR_CID_ANY &&
+	    vsk->local_addr.svm_cid != local_cid)
+		ret = -EADDRNOTAVAIL;
+	else
+		ret = virtio_transport_send_pkt_info(vsk, &info);
+
+	if (ret < 0)
+		sk_vsock(vsk)->sk_state = TCP_CLOSE;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(virtio_transport_connect);
 
-- 
2.56.0

[PATCH RFC net v2 2/3] vsock/virtio: send credit updates only while connecting or connected

[Yiran Qiu]

Once a socket has a transport, changing its buffer size
(SO_VM_SOCKETS_BUFFER_*) sends a credit update in any state, from the
transport's CID to whatever remote address the last connect() left
behind.  So a socket bound to (guest CID, P) whose loopback connect()
failed or was refused still sends as (VMADDR_CID_LOCAL, P).  If another
process has a connection open from (VMADDR_CID_LOCAL, P) to the
listener that connect() targeted, the socket the listener accepted for
it takes the update's buf_alloc and fwd_cnt as its peer's, and can
stall.

On a host without a guest->host transport, a socket whose loopback
connect() failed can then connect() to VMADDR_CID_ANY with
VMADDR_FLAG_TO_HOST.  That fails with ENODEV but leaves VMADDR_CID_ANY
as the remote address, so the update reaches any loopback socket on
that port.

Only send the update while connecting or connected.  Now that connect()
refuses a CID the transport does not own and leaves a failed socket in
TCP_CLOSE, a socket in either state sends from its own address.

Found by an LLM review of an unposted draft of this series.  On
unmodified kernels, where the first connect() times out instead of
being refused, a non-blocking send() on the accepted socket returns
EAGAIN after the update; with this patch it succeeds, and vsock_test
passes.  Drafted with Claude.

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
 net/vmw_vsock/virtio_transport_common.c |  4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 8f524d073ae1..4d0511629044 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1098,7 +1098,9 @@ void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val)
 
 	vvs->buf_alloc = *val;
 
-	virtio_transport_send_credit_update(vsk);
+	if (sk_vsock(vsk)->sk_state == TCP_ESTABLISHED ||
+	    sk_vsock(vsk)->sk_state == TCP_SYN_SENT)
+		virtio_transport_send_credit_update(vsk);
 }
 EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size);
 
-- 
2.56.0

[PATCH RFC net v2 3/3] vsock/virtio: only update a socket's local CID to the transport's CID

[Yiran Qiu]

virtio_transport_recv_pkt() copies a packet's destination CID into the
local address of the socket it reaches, so that sockets follow a guest
CID change after a transport reset.  But the bound-socket lookup also
delivers packets addressed to VMADDR_CID_ANY, and vsock_loopback can
carry them: a connect() to VMADDR_CID_ANY goes over loopback when the
guest->host transport has no CID (no virtio-vsock device, or hv_sock).
The socket bound to that port then ends up with VMADDR_CID_ANY as its
local CID: a listener on (VMADDR_CID_LOCAL, P) turns into a wildcard
listener, and a socket whose connect() was refused for its CID would
pass the check in virtio_transport_connect() on the next try.

Only take the destination CID if it differs from the socket's current
one, is not VMADDR_CID_ANY, and is the transport's own CID.  The
VMADDR_CID_ANY test is still needed: get_local_cid() itself returns
VMADDR_CID_ANY while the virtio-vsock device is removed or frozen.

In a guest without a virtio-vsock device, after a loopback connect() to
(VMADDR_CID_ANY, P), getsockname() on a listener bound to
(VMADDR_CID_LOCAL, P) returns VMADDR_CID_ANY on unmodified kernels and
VMADDR_CID_LOCAL with this patch; vsock_test passes.  Drafted with
Claude.

Fixes: 408624af4c89 ("vsock: use local transport when it is loaded")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-188830@kernel.org/
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
Since 36c5b48b91ac ("vsock: fix bind() behaviour taking care of CID")
the CID update no longer helps listeners, and connected sockets are
reset on a transport reset, so the update could be dropped instead;
would you prefer that?  Refusing connect() to VMADDR_CID_ANY in
vsock_use_local_transport() would also work, but would stop guests
without a guest->host CID from reaching local listeners.

 net/vmw_vsock/virtio_transport_common.c |  5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 4d0511629044..b276009efab9 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1861,7 +1861,10 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
 	space_available = virtio_transport_space_update(sk, skb);
 
 	/* Update CID in case it has changed after a transport reset event */
-	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
+	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY &&
+	    vsk->local_addr.svm_cid != dst.svm_cid &&
+	    dst.svm_cid != VMADDR_CID_ANY &&
+	    dst.svm_cid == t->transport.get_local_cid())
 		vsk->local_addr.svm_cid = dst.svm_cid;
 
 	if (space_available)
-- 
2.56.0