[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