Re: [PATCH RFC v3 6/8] vsock: allow datagrams to use directional transports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] vsock: local datagrams route to transport_g2h when loopback
  is unloaded
- [Medium] vsock: VMCI datagram sockets misroute CID_HOST and CID_ANY to
  loopback
--

commit 9eab4679f5d6926cf379df182996c2e8cd726902
Author: Sergio Lopez <slp@redhat.com>

vsock: allow datagrams to use directional transports

This commit enables datagram sockets to select directional and loopback
transports per destination while keeping a fallback transport as the
socket owner when needed, and unifies datagram receive queuing in the
core.

Link: https://lore.kernel.org/all/DMGNAV67LK3F.2FNLTD5NVHDEA@gmail.com/

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index aef5389eeca2c..e0d1a3e5d05cc 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
[ ... ]
> @@ -576,11 +579,56 @@ static bool vsock_use_local_transport(unsigned int remote_cid)
>  	if (remote_cid == VMADDR_CID_LOCAL)
>  		return true;
>  
> -	if (transport_g2h) {
> -		return remote_cid == transport_g2h->get_local_cid();
> -	} else {
> -		return remote_cid == VMADDR_CID_HOST;
> +	local_cid = transport_g2h ? transport_g2h->get_local_cid() :
> +		VMADDR_CID_HOST;
> +	/* A virtio device without a CID keeps the host loopback alias.
> +	 * Hyper-V has no numeric CID even while it is active, so CID_HOST
> +	 * must keep using its guest-to-host transport in that case.
> +	 */
> +	if (local_cid == VMADDR_CID_ANY && transport_g2h->dgram_transport_match)
> +		local_cid = VMADDR_CID_HOST;
> +
> +	return remote_cid == local_cid;
> +}
> +
> +/* Select a route without changing socket ownership. The caller holds
> + * vsock_register_mutex and pins the returned transport before releasing it.
> + */
> +static const struct vsock_transport *
> +vsock_select_transport(struct vsock_sock *vsk,
> +		       const struct sockaddr_vm *remote_addr,
> +		       const struct vsock_transport *dgram_owner,
> +		       bool *to_host)
> +{
> +	struct sock *sk = sk_vsock(vsk);
> +	u32 cid = remote_addr->svm_cid;
> +
> +	lockdep_assert_held(&vsock_register_mutex);
> +	*to_host = false;
> +
> +	if (vsock_use_local_transport(cid))
> +		return transport_local;
> +	if (cid <= VMADDR_CID_HOST ||
> +	    (remote_addr->svm_flags & VMADDR_FLAG_TO_HOST))
> +		return transport_g2h;

[Severity: Medium]
Can vsock_select_transport() route VMADDR_CID_LOCAL datagrams to
transport_g2h when transport_local is not registered?

When transport_local is NULL, vsock_use_local_transport(VMADDR_CID_LOCAL)
returns false and cid <= VMADDR_CID_HOST (1 <= 2) matches, returning
transport_g2h instead of NULL. Because vsock_dgram_resolve_cid() also
maps VMADDR_CID_ANY to VMADDR_CID_LOCAL whenever
owner->dgram_transport_match is set, datagram connect() and sendmsg() to
VMADDR_CID_LOCAL or VMADDR_CID_ANY transmit local packets to the host (or
fall back to the VMCI owner) instead of failing with -ENODEV. When
vsock.g2h_fallback is enabled without transport_local, connecting a
datagram socket to the guest's own CID also succeeds via transport_g2h
even though subsequent sends fail in virtio_transport_send_pkt().

[ ... ]

> @@ -1461,6 +1645,27 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
>  	return vsk->transport->read_skb(vsk, read_actor);
>  }
>  
> +static int vsock_dgram_resolve_cid(struct vsock_sock *vsk,
> +				   struct sockaddr_vm *remote_addr,
> +				   bool *vmci_self)
> +{
> +	const struct vsock_transport *owner = vsk->transport;
> +
> +	if (remote_addr->svm_cid != VMADDR_CID_ANY)
> +		return 0;
> +
> +	mutex_lock(&vsock_register_mutex);
> +	if (transport_local || owner->dgram_transport_match) {
> +		remote_addr->svm_cid = VMADDR_CID_LOCAL;
> +	} else {
> +		remote_addr->svm_cid = owner->get_local_cid();
> +		*vmci_self = owner->dgram_fallback;
> +	}
> +	mutex_unlock(&vsock_register_mutex);
> +
> +	return remote_addr->svm_cid == VMADDR_CID_ANY ? -EADDRNOTAVAIL : 0;
> +}

[Severity: Medium]
Does this misroute VMCI datagrams onto loopback when vsock_loopback is
loaded?

On a VMCI guest where virtio_transport is loaded without an active
virtio-vsock device (or transport_g2h is NULL),
vsock_use_local_transport(VMADDR_CID_HOST) returns true, so
vsock_select_transport() routes VMADDR_CID_HOST datagrams to
transport_local instead of falling back to the VMCI owner. In addition, if
a VMCI socket is bound to VMCI's local CID and sends or connects to
VMADDR_CID_ANY while transport_local is registered,
vsock_dgram_resolve_cid() resolves the destination to VMADDR_CID_LOCAL and
vsock_dgram_get_transport() skips the fallback to owner because
transport->get_local_cid() == VMADDR_CID_LOCAL, failing with
-EADDRNOTAVAIL when vsk->local_addr.svm_cid does not match loopback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-vsock-dgram-support-v3-0-1523c8e9b27e@redhat.com?part=6