On Thu, Aug 20, 2026 at 09:15:17AM +0900, Daehyeon Ko wrote:
virtio_transport_recv_pkt() looks up sockets first by the full source and
destination tuple, then by destination only in the bound table. The
fallback is needed for listening and connecting sockets, but sockets remain
in the bound table after connect(), so it can also return a non-listening
socket.
The fallback does not validate the source address. In TCP_SYN_SENT, a
RESPONSE from an unrelated source can transition the victim socket to
TCP_ESTABLISHED while its stored remote address remains unchanged.
Subsequent RW packets from that source are delivered through the same
destination-only fallback.
This was reproduced with capability-empty processes under different UIDs.
The attacker discovered the target tuple through unprivileged AF_VSOCK
sock_diag and caused the victim socket to read 16 attacker-chosen bytes;
the intended peer-side socket read 0 of those 16 bytes.
After lock_sock(), reject packets for non-listening sockets unless their
source port matches the stored remote port. Require the CID to match too,
except that the loopback transport uses VMADDR_CID_LOCAL as the packet
source for connections addressed through its valid CID aliases.
Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
Closes:
https://lore.kernel.org/netdev/[email protected]/
Cc: [email protected]
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Daehyeon Ko <[email protected]>
---
Changes in v2:
- Preserve valid loopback CID aliases by matching the source port and
accepting VMADDR_CID_LOCAL only for the loopback transport.
- Rewrite the commit message and receive-path comment for clarity.
v1: https://lore.kernel.org/netdev/[email protected]/
net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 8becad812..d8990f5f6 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1764,6 +1764,21 @@ static bool virtio_transport_valid_type(u16 type)
(type == VIRTIO_VSOCK_TYPE_SEQPACKET);
}
+static bool virtio_transport_source_matches(const struct virtio_transport *t,
This doesn't seem virtio-specific, so why introducing this function
here?
IIRC we agreed that also VMCI has the same issue, so what about adding
this function in af_vsock.c?
Also I don't see VMCI changes, why not doing them together in a series
since they are strictly related?
+ const struct sockaddr_vm *src,
+ const struct sockaddr_vm *remote)
+{
What about moving here also the transport check?
So we can have something like this (with some documentation):
bool vsock_check_source(const struct vsock_sock *vsk,
const struct vsock_transport *transport,
const struct sockaddr_vm *src)
{
if (vsk->transport != transport)
return false;
if (src->svm_port != vsk->remote_addr.svm_port)
return false;
etc.
+ if (src->svm_port != remote->svm_port)
+ return false;
+
+ if (src->svm_cid == remote->svm_cid)
+ return true;
+
+ /* The loopback transport represents its peer as VMADDR_CID_LOCAL. */
+ return t->transport.get_local_cid() == VMADDR_CID_LOCAL &&
+ src->svm_cid == VMADDR_CID_LOCAL;
Could this be just `t->transport.get_local_cid() == VMADDR_CID_LOCAL` ?
I guess the source is trusted and if we are in a local transport, it's
always set to it, no?
Thanks,
Stefano
+}
+
/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
* lock.
*/
@@ -1823,10 +1838,15 @@ void virtio_transport_recv_pkt(struct virtio_transport
*t,
lock_sock(sk);
/* Check if sk has been closed or assigned to another transport before
- * lock_sock (note: listener sockets are not assigned to any transport)
+ * lock_sock (note: listener sockets are not assigned to any transport).
+ * The bound-table fallback matches only the destination, so reject
packets
+ * from a peer other than the one stored in the socket.
*/
if (sock_flag(sk, SOCK_DONE) ||
- (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
+ (sk->sk_state != TCP_LISTEN &&
+ (vsk->transport != &t->transport ||
+ !virtio_transport_source_matches(t, &src,
+ &vsk->remote_addr)))) {
(void)virtio_transport_reset_no_sock(t, skb, net);
release_sock(sk);
sock_put(sk);
base-commit: e2466392a0b8496000e12181cb1ee1535eb0da25
--
2.54.0
