Re: [PATCH net v2] vsock/virtio: validate packet source for connected sockets

[Bobby Eshleman <bobbyeshleman@gmail.com>]

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/20260813121236.2328599-1-4ncienth@gmail.com/
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6-sol
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
> 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/20260813121236.2328599-1-4ncienth@gmail.com/
> 
>  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,
> +					    const struct sockaddr_vm *src,
> +					    const struct sockaddr_vm *remote)
> +{
> +	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;

nit: not a strong preference, but I feel this comes out a little more
readable as:

	return src->svm_cid == t->transport.get_local_cid(); 

Otherwise, all looks good to me.

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

> +}
> +
>  /* 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