Re: [PATCH net v3 1/2] vsock/virtio: validate packet source for connected sockets

[Bobby Eshleman <bobbyeshleman@gmail.com>]

On Mon, Aug 24, 2026 at 02:58:57AM +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.
> 
> Add vsock_check_source() to validate the transport, source port and source
> CID against the peer stored in a non-listening socket. The local transport
> is the CID exception because its packets are generated internally with
> VMADDR_CID_LOCAL as their source, including connections using CID aliases.
> 
> Use the helper after lock_sock() in the virtio receive path.
> 
> Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
> Closes: https://lore.kernel.org/netdev/20260813121236.2328599-1-4ncienth@gmail.com/
> Cc: stable@vger.kernel.org
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
>  include/net/af_vsock.h                  |  3 +++
>  net/vmw_vsock/af_vsock.c                | 32 +++++++++++++++++++++++++
>  net/vmw_vsock/virtio_transport_common.c |  3 ++-
>  3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 3357ee62d..5549298c1 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -229,6 +229,9 @@ struct sock *vsock_find_bound_socket_net(struct sockaddr_vm *addr,
>  struct sock *vsock_find_connected_socket_net(struct sockaddr_vm *src,
>  					     struct sockaddr_vm *dst,
>  					     struct net *net);
> +bool vsock_check_source(const struct vsock_sock *vsk,
> +			const struct vsock_transport *transport,
> +			const struct sockaddr_vm *src);
>  void vsock_remove_sock(struct vsock_sock *vsk);
>  void vsock_for_each_connected_socket(struct vsock_transport *transport,
>  				     void (*fn)(struct sock *sk));
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index a33b2a2d3..f840498b5 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -438,6 +438,38 @@ struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
>  }
>  EXPORT_SYMBOL_GPL(vsock_find_connected_socket);
>  
> +/**
> + * vsock_check_source - validate a packet source against a socket peer
> + * @vsk: socket receiving the packet
> + * @transport: transport receiving the packet
> + * @src: source address from the packet
> + *
> + * Return: true if the packet arrived on the socket's assigned transport and
> + * its source matches the stored peer. Loopback packets are generated
> + * internally and always use the local CID as their source, including
> + * connections using a valid CID alias.
> + *
> + * The caller must hold the socket lock and must not call this for listening
> + * sockets, which accept packets from any source and have no assigned
> + * transport.
> + */
> +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;
> +
> +	if (src->svm_cid == vsk->remote_addr.svm_cid)
> +		return true;
> +
> +	return transport->get_local_cid() == VMADDR_CID_LOCAL;
> +}
> +EXPORT_SYMBOL_GPL(vsock_check_source);
> +
>  void vsock_remove_sock(struct vsock_sock *vsk)
>  {
>  	/* Transport reassignment must not remove the binding. */
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index e4ebaa70f..6301c108a 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -1823,7 +1823,8 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>  	 * lock_sock (note: listener sockets are not assigned to any transport)
>  	 */
>  	if (sock_flag(sk, SOCK_DONE) ||
> -	    (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
> +	    (sk->sk_state != TCP_LISTEN &&
> +	     !vsock_check_source(vsk, &t->transport, &src))) {
>  		(void)virtio_transport_reset_no_sock(t, skb, net);
>  		release_sock(sk);
>  		sock_put(sk);
> -- 
> 2.54.0
> 

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>