Re: [PATCH net-next v6 6/8] vsock/virtio: support guest device network namespace

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] vsock/virtio: swept sockets still send packets in
  virtio_transport_send_pkt_info()
--

commit fba2ffbc0db5d9ef1fbbeb2422cacba719ecb282
Author: Bobby Eshleman <bobbyeshleman@meta.com>

vsock/virtio: support guest device network namespace

This commit allows assigning the virtio-vsock guest-to-host device to a
network namespace and enforces namespace reachability on stream and
seqpacket sockets across the receive, handshake, and send paths.

> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 1e762a480df49..c1cbe834e4f59 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -295,6 +295,9 @@ static struct sk_buff *virtio_transport_alloc_skb(struct virtio_vsock_pkt_info *
>   * since a socket assigned to a transport is required.
>   *
>   * Do not use on listener sockets!
> + *
> + * This checks that the socket can reach the g2h device, so device-only replies
> + * such as virtio_transport_reset_no_sock() must keep not using this path.
>   */
>  static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  					  struct virtio_vsock_pkt_info *info)
> @@ -317,6 +320,9 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	if (unlikely(!t_ops))
>  		return -EFAULT;
>  
> +	if (!vsock_g2h_reachable_sk(vsk))
> +		return -ENETUNREACH;
> +
>  	src_cid = t_ops->transport.get_local_cid();
>  	src_port = vsk->local_addr.svm_port;
>  	if (!info->remote_cid) {

[Severity: Medium]
Can a socket that was swept by vsock_g2h_reset_unreachable() still send
packets if the g2h device is later reassigned back to a reachable
namespace?

vsock_g2h_reset_unreachable() removes the socket from the connected table
and sets SOCK_DONE, vsk->peer_shutdown = SHUTDOWN_MASK, and
sk->sk_state = TCP_CLOSING, while vsock_g2h_reachable_sk() only checks
whether the socket's namespace can currently reach vsock_g2h_net. If the
device moves back to a namespace vsk can reach (such as init_net) before
userspace closes the file descriptor, vsock_g2h_reachable_sk(vsk) returns
true again, and close() -> virtio_transport_close() sends a RST before
checking SOCK_DONE:

net/vmw_vsock/virtio_transport_common.c:virtio_transport_close() {
	if (!(sk->sk_state == TCP_ESTABLISHED ||
	      sk->sk_state == TCP_CLOSING))
		return true;

	/* Already received SHUTDOWN from peer, reply with RST */
	if ((vsk->peer_shutdown & SHUTDOWN_MASK) == SHUTDOWN_MASK) {
		(void)virtio_transport_reset(vsk, NULL);
		return true;
	}
    ...
}

Because the swept socket was already removed from vsock_connected_table,
that RST (or a packet from shutdown() or setsockopt(SO_RCVLOWAT)) goes out
with the old 4-tuple and can reset a new connection that reused the port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-vsock-guest-ns-v6-0-3737ce4673c0@meta.com?part=6