[PATCH RFC net] vsock: make local-transport CIDs conflict on the same port

[PATCH RFC net] vsock: make local-transport CIDs conflict on the same port

[Yiran Qiu]

The local transport serves VMADDR_CID_LOCAL and the guest->host
transport's CID (VMADDR_CID_HOST if there is none), and sends every
packet with source CID VMADDR_CID_LOCAL.  A port bound on either CID is
therefore one port on the wire, but the bind conflict check compares
full addresses, so (guest_cid, P) and (VMADDR_CID_LOCAL, P) can both be
bound.  Because __vsock_find_connected_socket_net() compares only the
destination port (deliberately, since 990454b5a48b), a child accepted
on one CID then matches packets addressed to the other.
vsock_check_source() already treats these CIDs as aliases
(dee44f41f206); bind() does not.

An unprivileged process can use this to reset another process's
connect() when that client is bound to a specific local CID.  Comparing
the destination CID in the lookup instead turns the reset into data
injection, as vsock_check_source() accepts the rewritten source CID.

Reject a bind whose port is already held on another local-transport
CID.  User-visible change: a guest can no longer serve the host on
(guest_cid, P) and local clients on (VMADDR_CID_LOCAL, P) at once; the
second bind fails with EADDRINUSE.

Found with afb-afl, an LLM-driven kernel audit/repro pipeline; the fix
and this changelog were also drafted with LLM assistance.

Fixes: 408624af4c89 ("vsock: use local transport when it is loaded")
Signed-off-by: Yiran Qiu <eritque-arcus@ikuyo.dev>
---
Tested on net/main 4dca97647a3e (vsock unchanged through net/main
5d7f3375cd57 and mainline 65ba8f34ec88) in a QEMU/KVM guest with
vhost-vsock (guest CID 3) and vsock_loopback; 3 runs per case:

  case                                  before      after
  peer on (LOCAL, P), attacker listener ECONNRESET  attacker bind fails,
                                                    victim connects
  same, attacker at another uid         ECONNRESET  victim connects
  peer on (ANY, P), no attacker listen. ECONNRESET  victim connects
  no attacker                           connects    connects

With only a destination-CID compare instead: no reset, but the victim
read 18 attacker bytes (3/3).  vsock_test 44/44 with --peer-cid=1 and 3,
before and after; no WARN/BUG (KASAN off).  Not tested: VMCI, the
host-side alias.  RFC: bind time vs. lookup is your call.  Reproducer
available on request.

 net/vmw_vsock/af_vsock.c | 63 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index d0508887a2fb..47fd00fdac6d 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -294,8 +294,31 @@ static void __vsock_remove_connected(struct vsock_sock *vsk)
 	sock_put(&vsk->sk);
 }
 
+/*
+ * CIDs served by the local transport are aliases of one another on the wire:
+ * an outgoing packet uses transport->get_local_cid() as its source CID, no
+ * matter which of those CIDs the socket was bound to.  Two binds of the same
+ * port on different local CIDs therefore reach the same peer and must
+ * conflict, just like a VMADDR_CID_ANY bind conflicts with any CID.
+ *
+ * @local_cid is the second local CID (the guest->host transport's own CID, or
+ * VMADDR_CID_HOST when there is no guest->host transport), or
+ * VMADDR_CID_ANY when the local transport is not loaded.
+ */
+static bool vsock_addr_aliases_local(const struct sockaddr_vm *a,
+				     const struct sockaddr_vm *b,
+				     u32 local_cid)
+{
+	return local_cid != VMADDR_CID_ANY &&
+	       a->svm_port == b->svm_port &&
+	       a->svm_cid != b->svm_cid &&
+	       (a->svm_cid == VMADDR_CID_LOCAL || a->svm_cid == local_cid) &&
+	       (b->svm_cid == VMADDR_CID_LOCAL || b->svm_cid == local_cid);
+}
+
 static struct sock *__vsock_find_bound_socket_net(struct sockaddr_vm *addr,
-						  struct net *net)
+						  struct net *net,
+						  u32 local_cid)
 {
 	struct vsock_sock *vsk;
 
@@ -311,6 +334,10 @@ static struct sock *__vsock_find_bound_socket_net(struct sockaddr_vm *addr,
 		     addr->svm_cid == VMADDR_CID_ANY) &&
 		     vsock_net_check_mode(sock_net(sk), net))
 			return sk;
+
+		if (vsock_addr_aliases_local(addr, &vsk->local_addr, local_cid) &&
+		    vsock_net_check_mode(sock_net(sk), net))
+			return sk;
 	}
 
 	return NULL;
@@ -383,7 +410,7 @@ struct sock *vsock_find_bound_socket_net(struct sockaddr_vm *addr,
 	struct sock *sk;
 
 	spin_lock_bh(&vsock_table_lock);
-	sk = __vsock_find_bound_socket_net(addr, net);
+	sk = __vsock_find_bound_socket_net(addr, net, VMADDR_CID_ANY);
 	if (sk)
 		sock_hold(sk);
 
@@ -727,6 +754,27 @@ static u32 vsock_registered_transport_cid(const struct vsock_transport **transpo
 	return cid;
 }
 
+/*
+ * Return the CID that the local transport aliases with VMADDR_CID_LOCAL, or
+ * VMADDR_CID_ANY when the local transport is not loaded or has no second CID.
+ * Must be called before taking vsock_table_lock.
+ */
+static u32 vsock_local_alias_cid(void)
+{
+	u32 cid = VMADDR_CID_ANY;
+
+	mutex_lock(&vsock_register_mutex);
+	if (transport_local) {
+		if (transport_g2h)
+			cid = transport_g2h->get_local_cid();
+		else
+			cid = VMADDR_CID_HOST;
+	}
+	mutex_unlock(&vsock_register_mutex);
+
+	return cid;
+}
+
 bool vsock_find_cid(unsigned int cid)
 {
 	if (cid == vsock_registered_transport_cid(&transport_g2h))
@@ -833,7 +881,8 @@ static void vsock_pending_work(struct work_struct *work)
 /**** SOCKET OPERATIONS ****/
 
 static int __vsock_bind_connectible(struct vsock_sock *vsk,
-				    struct sockaddr_vm *addr)
+				    struct sockaddr_vm *addr,
+				    u32 local_cid)
 {
 	struct net *net = sock_net(sk_vsock(vsk));
 	struct sockaddr_vm new_addr;
@@ -854,7 +903,7 @@ static int __vsock_bind_connectible(struct vsock_sock *vsk,
 
 			new_addr.svm_port = net->vsock.port++;
 
-			if (!__vsock_find_bound_socket_net(&new_addr, net)) {
+			if (!__vsock_find_bound_socket_net(&new_addr, net, local_cid)) {
 				found = true;
 				break;
 			}
@@ -871,7 +920,7 @@ static int __vsock_bind_connectible(struct vsock_sock *vsk,
 			return -EACCES;
 		}
 
-		if (__vsock_find_bound_socket_net(&new_addr, net))
+		if (__vsock_find_bound_socket_net(&new_addr, net, local_cid))
 			return -EADDRINUSE;
 	}
 
@@ -897,6 +946,7 @@ static int __vsock_bind_dgram(struct vsock_sock *vsk,
 static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr)
 {
 	struct vsock_sock *vsk = vsock_sk(sk);
+	u32 local_cid;
 	int retval;
 
 	/* First ensure this socket isn't already bound. */
@@ -914,8 +964,9 @@ static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr)
 	switch (sk->sk_socket->type) {
 	case SOCK_STREAM:
 	case SOCK_SEQPACKET:
+		local_cid = vsock_local_alias_cid();
 		spin_lock_bh(&vsock_table_lock);
-		retval = __vsock_bind_connectible(vsk, addr);
+		retval = __vsock_bind_connectible(vsk, addr, local_cid);
 		spin_unlock_bh(&vsock_table_lock);
 		break;
 
-- 
2.56.0

Re: [PATCH RFC net] vsock: make local-transport CIDs conflict on the same port

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vsock: incomplete local CID alias check in vsock_local_alias_cid()
--

commit 5d164f9e8d6139f39bfbdee4f02c52a0bdbf7d5a
Author: Yiran Qiu <eritque-arcus@ikuyo.dev>

vsock: make local-transport CIDs conflict on the same port

This commit updates __vsock_find_bound_socket_net() during connectible
socket bind() so that binding to a port on VMADDR_CID_LOCAL conflicts
with binding to the same port on the local alias CID.

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index d0508887a2fba..47fd00fdac6d8 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -294,8 +294,31 @@ static void __vsock_remove_connected(struct vsock_sock *vsk)
>  	sock_put(&vsk->sk);
>  }
>  
> +/*
> + * CIDs served by the local transport are aliases of one another on the wire:
> + * an outgoing packet uses transport->get_local_cid() as its source CID, no
> + * matter which of those CIDs the socket was bound to.  Two binds of the same
> + * port on different local CIDs therefore reach the same peer and must
> + * conflict, just like a VMADDR_CID_ANY bind conflicts with any CID.
> + *
> + * @local_cid is the second local CID (the guest->host transport's own CID, or
> + * VMADDR_CID_HOST when there is no guest->host transport), or
> + * VMADDR_CID_ANY when the local transport is not loaded.
> + */
> +static bool vsock_addr_aliases_local(const struct sockaddr_vm *a,
> +				     const struct sockaddr_vm *b,
> +				     u32 local_cid)
> +{
> +	return local_cid != VMADDR_CID_ANY &&
> +	       a->svm_port == b->svm_port &&
> +	       a->svm_cid != b->svm_cid &&
> +	       (a->svm_cid == VMADDR_CID_LOCAL || a->svm_cid == local_cid) &&
> +	       (b->svm_cid == VMADDR_CID_LOCAL || b->svm_cid == local_cid);
> +}

[ ... ]

> @@ -727,6 +754,27 @@ static u32 vsock_registered_transport_cid(const struct vsock_transport **transpo
>  	return cid;
>  }
>  
> +/*
> + * Return the CID that the local transport aliases with VMADDR_CID_LOCAL, or
> + * VMADDR_CID_ANY when the local transport is not loaded or has no second CID.
> + * Must be called before taking vsock_table_lock.
> + */
> +static u32 vsock_local_alias_cid(void)
> +{
> +	u32 cid = VMADDR_CID_ANY;
> +
> +	mutex_lock(&vsock_register_mutex);
> +	if (transport_local) {
> +		if (transport_g2h)
> +			cid = transport_g2h->get_local_cid();
> +		else
> +			cid = VMADDR_CID_HOST;
> +	}
> +	mutex_unlock(&vsock_register_mutex);
> +
> +	return cid;
> +}

[Severity: Medium]
Can two sockets still bind to the same port on different CIDs that end up
using transport_local and colliding on the wire when VMADDR_CID_HOST is
bound alongside VMADDR_CID_LOCAL or the guest CID?

First, in vsock_local_alias_cid(), if a guest-to-host transport driver is
registered (for example, virtio_transport built-in or loaded on a host
where no virtio-vsock guest device is present), transport_g2h is non-NULL,
and transport_g2h->get_local_cid() returns VMADDR_CID_ANY:

net/vmw_vsock/virtio_transport.c:virtio_transport_get_local_cid() {
	rcu_read_lock();
	vsock = rcu_dereference(the_virtio_vsock);
	if (!vsock) {
		ret = VMADDR_CID_ANY;
		goto out_rcu;
	}
    ...
}

Because vsock_local_alias_cid() only checks whether transport_g2h is
non-NULL rather than whether get_local_cid() returned VMADDR_CID_ANY, it
returns VMADDR_CID_ANY instead of falling back to VMADDR_CID_HOST. When
transport_h2g and transport_local are also loaded on that host,
__vsock_bind() -> vsock_find_cid() allows binding to both VMADDR_CID_HOST
and VMADDR_CID_LOCAL, while vsock_addr_aliases_local() sees
local_cid == VMADDR_CID_ANY and skips the conflict check completely.

Second, if both transport_g2h (with a valid guest CID) and transport_h2g
are registered alongside transport_local, vsock_find_cid() allows
__vsock_bind() to bind sockets to all three CIDs:

net/vmw_vsock/af_vsock.c:vsock_find_cid() {
	if (cid == vsock_registered_transport_cid(&transport_g2h))
		return true;

	if (transport_h2g && cid == VMADDR_CID_HOST)
		return true;

	if (transport_local && cid == VMADDR_CID_LOCAL)
		return true;

	return false;
}

However, vsock_local_alias_cid() only returns the single G2H CID and
vsock_addr_aliases_local() only checks VMADDR_CID_LOCAL against that single
local_cid. A socket can therefore still bind to (VMADDR_CID_HOST, port)
while another socket is bound to (VMADDR_CID_LOCAL, port) or
(guest_cid, port).

When the socket bound to (VMADDR_CID_HOST, port) connects to
VMADDR_CID_LOCAL (or guest_cid), vsock_connect() -> vsock_assign_transport()
selects transport_local based only on remote_cid:

net/vmw_vsock/af_vsock.c:vsock_assign_transport() {
	case SOCK_STREAM:
	case SOCK_SEQPACKET:
		if (vsock_use_local_transport(remote_cid))
			new_transport = transport_local;
    ...
}

And virtio_transport_send_pkt_info() then sends packets with source CID
VMADDR_CID_LOCAL and vsk->local_addr.svm_port, hitting the same port
collision on the loopback wire.

Should vsock_addr_aliases_local() treat all locally bindable CIDs (where
vsock_find_cid() is true) as conflicting with each other whenever
transport_local is registered, or at least fall back when
transport_g2h->get_local_cid() returns VMADDR_CID_ANY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028065231.655701-1-eritque-arcus@ikuyo.dev?part=1