[PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H

Bobby Eshleman posted 11 patches 2 weeks ago
There is a newer version of this series
[PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
Posted by Bobby Eshleman 2 weeks ago
From: Bobby Eshleman <bobbyeshleman@meta.com>

Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
is operational. Additionally, reject G2H transport registration if there
already exists a namespace in local mode.

G2H sockets break in local mode because the G2H transports don't support
namespacing yet. The current approach is to coerce packets coming out of
G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
to coerce sockets in the same way because it cannot be deduced which
transport will be used by the socket. Specifically, when bound to
VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
until a packet is received and matched to the bound socket that we
assign the transport. This presents a chicken-and-egg problem, because
we need the namespace to lookup the socket and resolve the transport,
but we need the transport to know how to use the namespace during
lookup.

For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
used on systems that support G2H, even nested systems that also have H2G
transports.

Local mode is blocked based on detecting the presence of G2H devices
(when possible, as hyperv is special). This means that a host kernel
with G2H support compiled in (or has the module loaded), will still
support local mode if there is no G2H (e.g., virtio-vsock) device
detected. This enables using the same kernel in the host and in the
guest, as we do in kselftest.

Systems with only namespace-aware transports (vhost-vsock, loopback) can
still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
intended.

Add supports_local_mode() transport callback to indicate
transport-specific local mode support.

These restrictions can be lifted in a future patch series when G2H
transports gain namespace support.

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v10:
- move this patch before any transports bring online namespacing (Stefano)
- move vsock_net_mode_string into critical section (Stefano)
- add ->supports_local_mode() callback to transports (Stefano)
---
 drivers/vhost/vsock.c            |  6 +++++
 include/net/af_vsock.h           |  5 ++++
 net/vmw_vsock/af_vsock.c         | 50 ++++++++++++++++++++++++++++++++++------
 net/vmw_vsock/hyperv_transport.c |  6 +++++
 net/vmw_vsock/virtio_transport.c | 13 +++++++++++
 net/vmw_vsock/vmci_transport.c   |  7 ++++++
 net/vmw_vsock/vsock_loopback.c   |  6 +++++
 7 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 2c937a2df83b..c8319cd1c232 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
 	return VHOST_VSOCK_DEFAULT_HOST_CID;
 }
 
+static bool vhost_transport_supports_local_mode(void)
+{
+	return true;
+}
+
 /* Callers that dereference the return value must hold vhost_vsock_mutex or the
  * RCU read lock.
  */
@@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = vhost_transport_get_local_cid,
+		.supports_local_mode	  = vhost_transport_supports_local_mode,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 59d97a143204..824d89657d41 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -180,6 +180,11 @@ struct vsock_transport {
 	/* Addressing. */
 	u32 (*get_local_cid)(void);
 
+	/* Return true if this transport supports VSOCK_NET_MODE_LOCAL.
+	 * Otherwise, return false.
+	 */
+	bool (*supports_local_mode)(void);
+
 	/* Read a single skb */
 	int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
 
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 54373ae101c3..7a235bb94437 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -91,6 +91,12 @@
  *   and locked down by a namespace manager. The default is "global". The mode
  *   is set per-namespace.
  *
+ *   Note: LOCAL mode is only supported when using namespace-aware transports
+ *   (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
+ *   hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
+ *   with EOPNOTSUPP, as these transports do not support per-namespace
+ *   isolation.
+ *
  *   The modes affect the allocation and accessibility of CIDs as follows:
  *
  *   - global - access and allocation are all system-wide
@@ -2765,17 +2771,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
 	if (*lenp >= sizeof(data))
 		return -EINVAL;
 
-	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
+	ret = 0;
+	mutex_lock(&vsock_register_mutex);
+	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
 		mode = VSOCK_NET_MODE_GLOBAL;
-	else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
+	} else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
+		if (transport_g2h && transport_g2h->supports_local_mode &&
+		    !transport_g2h->supports_local_mode()) {
+			ret = -EOPNOTSUPP;
+			goto out;
+		}
 		mode = VSOCK_NET_MODE_LOCAL;
-	else
-		return -EINVAL;
+	} else {
+		ret = -EINVAL;
+		goto out;
+	}
 
-	if (!vsock_net_write_mode(net, mode))
-		return -EPERM;
+	if (!vsock_net_write_mode(net, mode)) {
+		ret = -EPERM;
+		goto out;
+	}
 
-	return 0;
+out:
+	mutex_unlock(&vsock_register_mutex);
+	return ret;
 }
 
 static struct ctl_table vsock_table[] = {
@@ -2916,6 +2935,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
 {
 	const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
 	int err = mutex_lock_interruptible(&vsock_register_mutex);
+	struct net *net;
 
 	if (err)
 		return err;
@@ -2938,6 +2958,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
 			err = -EBUSY;
 			goto err_busy;
 		}
+
+		/* G2H sockets break in LOCAL mode namespaces because G2H
+		 * transports don't support them yet. Block registering new G2H
+		 * transports if we already have local mode namespaces on the
+		 * system.
+		 */
+		rcu_read_lock();
+		for_each_net_rcu(net) {
+			if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
+				rcu_read_unlock();
+				err = -EOPNOTSUPP;
+				goto err_busy;
+			}
+		}
+		rcu_read_unlock();
+
 		t_g2h = t;
 	}
 
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 432fcbbd14d4..279f04fcd81a 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
 	return -EOPNOTSUPP;
 }
 
+static bool hvs_supports_local_mode(void)
+{
+	return false;
+}
+
 static struct vsock_transport hvs_transport = {
 	.module                   = THIS_MODULE,
 
 	.get_local_cid            = hvs_get_local_cid,
+	.supports_local_mode      = hvs_supports_local_mode,
 
 	.init                     = hvs_sock_init,
 	.destruct                 = hvs_destruct,
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5d379ccf3770..e585cb66c6f5 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
 	return ret;
 }
 
+static bool virtio_transport_supports_local_mode(void)
+{
+	struct virtio_vsock *vsock;
+
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
+	rcu_read_unlock();
+
+	/* Local mode is supported only when no G2H device is present. */
+	return vsock ? false : true;
+}
+
 /* Caller need to hold vsock->tx_lock on vq */
 static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
 				     struct virtio_vsock *vsock, gfp_t gfp)
@@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = virtio_transport_get_local_cid,
+		.supports_local_mode      = virtio_transport_supports_local_mode,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 7eccd6708d66..da7c52ad7b2a 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
 	return vmci_get_context_id();
 }
 
+static bool vmci_transport_supports_local_mode(void)
+{
+	/* Local mode is supported only when no device is present. */
+	return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
+}
+
 static struct vsock_transport vmci_transport = {
 	.module = THIS_MODULE,
 	.init = vmci_transport_socket_init,
@@ -2062,6 +2068,7 @@ static struct vsock_transport vmci_transport = {
 	.notify_send_post_enqueue = vmci_transport_notify_send_post_enqueue,
 	.shutdown = vmci_transport_shutdown,
 	.get_local_cid = vmci_transport_get_local_cid,
+	.supports_local_mode = vmci_transport_supports_local_mode,
 };
 
 static bool vmci_check_transport(struct vsock_sock *vsk)
diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
index 8722337a4f80..1e25c1a6b43f 100644
--- a/net/vmw_vsock/vsock_loopback.c
+++ b/net/vmw_vsock/vsock_loopback.c
@@ -26,6 +26,11 @@ static u32 vsock_loopback_get_local_cid(void)
 	return VMADDR_CID_LOCAL;
 }
 
+static bool vsock_loopback_supports_local_mode(void)
+{
+	return true;
+}
+
 static int vsock_loopback_send_pkt(struct sk_buff *skb)
 {
 	struct vsock_loopback *vsock = &the_vsock_loopback;
@@ -58,6 +63,7 @@ static struct virtio_transport loopback_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = vsock_loopback_get_local_cid,
+		.supports_local_mode	  = vsock_loopback_supports_local_mode,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,

-- 
2.47.3
Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
Posted by Stefano Garzarella 1 week, 6 days ago
On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
>is operational. Additionally, reject G2H transport registration if there
>already exists a namespace in local mode.
>
>G2H sockets break in local mode because the G2H transports don't support
>namespacing yet. The current approach is to coerce packets coming out of
>G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
>to coerce sockets in the same way because it cannot be deduced which
>transport will be used by the socket. Specifically, when bound to
>VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
>until a packet is received and matched to the bound socket that we
>assign the transport. This presents a chicken-and-egg problem, because
>we need the namespace to lookup the socket and resolve the transport,
>but we need the transport to know how to use the namespace during
>lookup.
>
>For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
>used on systems that support G2H, even nested systems that also have H2G
>transports.
>
>Local mode is blocked based on detecting the presence of G2H devices
>(when possible, as hyperv is special). This means that a host kernel
>with G2H support compiled in (or has the module loaded), will still
>support local mode if there is no G2H (e.g., virtio-vsock) device
>detected. This enables using the same kernel in the host and in the
>guest, as we do in kselftest.
>
>Systems with only namespace-aware transports (vhost-vsock, loopback) can
>still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
>intended.
>
>Add supports_local_mode() transport callback to indicate
>transport-specific local mode support.
>
>These restrictions can be lifted in a future patch series when G2H
>transports gain namespace support.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- move this patch before any transports bring online namespacing (Stefano)
>- move vsock_net_mode_string into critical section (Stefano)
>- add ->supports_local_mode() callback to transports (Stefano)
>---
> drivers/vhost/vsock.c            |  6 +++++
> include/net/af_vsock.h           |  5 ++++
> net/vmw_vsock/af_vsock.c         | 50 ++++++++++++++++++++++++++++++++++------
> net/vmw_vsock/hyperv_transport.c |  6 +++++
> net/vmw_vsock/virtio_transport.c | 13 +++++++++++
> net/vmw_vsock/vmci_transport.c   |  7 ++++++
> net/vmw_vsock/vsock_loopback.c   |  6 +++++
> 7 files changed, 86 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 2c937a2df83b..c8319cd1c232 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
> 	return VHOST_VSOCK_DEFAULT_HOST_CID;
> }
>
>+static bool vhost_transport_supports_local_mode(void)
>+{
>+	return true;

Should we enable this later, when we really add support, or it doesn't
affect anything if vhost-vsock is not really supporting it in this PR
(thinking about bisection issues).

>+}
>+
> /* Callers that dereference the return value must hold vhost_vsock_mutex or the
>  * RCU read lock.
>  */
>@@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
> 		.module                   = THIS_MODULE,
>
> 		.get_local_cid            = vhost_transport_get_local_cid,
>+		.supports_local_mode	  = vhost_transport_supports_local_mode,
>
> 		.init                     = virtio_transport_do_socket_init,
> 		.destruct                 = virtio_transport_destruct,
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 59d97a143204..824d89657d41 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -180,6 +180,11 @@ struct vsock_transport {
> 	/* Addressing. */
> 	u32 (*get_local_cid)(void);
>
>+	/* Return true if this transport supports VSOCK_NET_MODE_LOCAL.

nit: Here I would make it clearer that rather than supporting 
MODE_LOCAL, the transport is not compatible with it, etc.
A summary of the excellent description we have in the commit.

>+	 * Otherwise, return false.
>+	 */
>+	bool (*supports_local_mode)(void);
>+
> 	/* Read a single skb */
> 	int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 54373ae101c3..7a235bb94437 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -91,6 +91,12 @@
>  *   and locked down by a namespace manager. The default is "global". The mode
>  *   is set per-namespace.
>  *
>+ *   Note: LOCAL mode is only supported when using namespace-aware transports
>+ *   (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
>+ *   hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
>+ *   with EOPNOTSUPP, as these transports do not support per-namespace
>+ *   isolation.

Okay, maybe this is fine, so if you don't need to resend, feel free to 
ignore the previous comment.

>+ *
>  *   The modes affect the allocation and accessibility of CIDs as follows:
>  *
>  *   - global - access and allocation are all system-wide
>@@ -2765,17 +2771,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> 	if (*lenp >= sizeof(data))
> 		return -EINVAL;
>
>-	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
>+	ret = 0;

IIUC `ret` should already be 0 at this point, no?

>+	mutex_lock(&vsock_register_mutex);

I honestly don't like to mix the parsing, with this new check, so what
about leaving the parsing as before this patch (also without the mutex),
then just (untested):

	mutex_lock(&vsock_register_mutex);
	if (mode == VSOCK_NET_MODE_LOCAL && transport_g2h &&
	    transport_g2h->supports_local_mode &&
	    !transport_g2h->supports_local_mode()) {
		ret = -EOPNOTSUPP;
		goto out;
	}

	if (!vsock_net_write_mode(net, mode)) {
		ret = -EPERM;
	}
out:
	mutex_unlock(&vsock_register_mutex);
	return ret;
}

>+	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> 		mode = VSOCK_NET_MODE_GLOBAL;
>-	else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
>+	} else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
>+		if (transport_g2h && transport_g2h->supports_local_mode &&
>+		    !transport_g2h->supports_local_mode()) {
>+			ret = -EOPNOTSUPP;
>+			goto out;
>+		}
> 		mode = VSOCK_NET_MODE_LOCAL;
>-	else
>-		return -EINVAL;
>+	} else {
>+		ret = -EINVAL;
>+		goto out;
>+	}
>
>-	if (!vsock_net_write_mode(net, mode))
>-		return -EPERM;
>+	if (!vsock_net_write_mode(net, mode)) {
>+		ret = -EPERM;
>+		goto out;
>+	}
>
>-	return 0;
>+out:
>+	mutex_unlock(&vsock_register_mutex);
>+	return ret;
> }
>
> static struct ctl_table vsock_table[] = {
>@@ -2916,6 +2935,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> {
> 	const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> 	int err = mutex_lock_interruptible(&vsock_register_mutex);
>+	struct net *net;
>
> 	if (err)
> 		return err;
>@@ -2938,6 +2958,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> 			err = -EBUSY;
> 			goto err_busy;
> 		}
>+
>+		/* G2H sockets break in LOCAL mode namespaces because G2H
>+		 * transports don't support them yet. Block registering new G2H
>+		 * transports if we already have local mode namespaces on the
>+		 * system.
>+		 */
>+		rcu_read_lock();
>+		for_each_net_rcu(net) {
>+			if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
>+				rcu_read_unlock();
>+				err = -EOPNOTSUPP;
>+				goto err_busy;
>+			}
>+		}
>+		rcu_read_unlock();
>+
> 		t_g2h = t;
> 	}
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 432fcbbd14d4..279f04fcd81a 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> 	return -EOPNOTSUPP;
> }
>
>+static bool hvs_supports_local_mode(void)
>+{
>+	return false;
>+}
>+
> static struct vsock_transport hvs_transport = {
> 	.module                   = THIS_MODULE,
>
> 	.get_local_cid            = hvs_get_local_cid,
>+	.supports_local_mode      = hvs_supports_local_mode,
>
> 	.init                     = hvs_sock_init,
> 	.destruct                 = hvs_destruct,
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 5d379ccf3770..e585cb66c6f5 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
> 	return ret;
> }
>
>+static bool virtio_transport_supports_local_mode(void)
>+{
>+	struct virtio_vsock *vsock;
>+
>+	rcu_read_lock();
>+	vsock = rcu_dereference(the_virtio_vsock);
>+	rcu_read_unlock();
>+
>+	/* Local mode is supported only when no G2H device is present. */
>+	return vsock ? false : true;
>+}
>+
> /* Caller need to hold vsock->tx_lock on vq */
> static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
> 				     struct virtio_vsock *vsock, gfp_t gfp)
>@@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
> 		.module                   = THIS_MODULE,
>
> 		.get_local_cid            = virtio_transport_get_local_cid,
>+		.supports_local_mode      = virtio_transport_supports_local_mode,
>
> 		.init                     = virtio_transport_do_socket_init,
> 		.destruct                 = virtio_transport_destruct,
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 7eccd6708d66..da7c52ad7b2a 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
> 	return vmci_get_context_id();
> }
>
>+static bool vmci_transport_supports_local_mode(void)
>+{
>+	/* Local mode is supported only when no device is present. */
>+	return vmci_transport_get_local_cid() == VMCI_INVALID_ID;

IIRC vmci can be registered both as H2G and G2H, so should we filter out
the H2G case?

Also, IMO is better to use VMADDR_CID_ANY with get_local_cid().

>+}
>+
> static struct vsock_transport vmci_transport = {
> 	.module = THIS_MODULE,
> 	.init = vmci_transport_socket_init,
>@@ -2062,6 +2068,7 @@ static struct vsock_transport vmci_transport = {
> 	.notify_send_post_enqueue = vmci_transport_notify_send_post_enqueue,
> 	.shutdown = vmci_transport_shutdown,
> 	.get_local_cid = vmci_transport_get_local_cid,
>+	.supports_local_mode = vmci_transport_supports_local_mode,
> };
>
> static bool vmci_check_transport(struct vsock_sock *vsk)
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index 8722337a4f80..1e25c1a6b43f 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -26,6 +26,11 @@ static u32 vsock_loopback_get_local_cid(void)
> 	return VMADDR_CID_LOCAL;
> }
>
>+static bool vsock_loopback_supports_local_mode(void)
>+{
>+	return true;
>+}
>+
> static int vsock_loopback_send_pkt(struct sk_buff *skb)
> {
> 	struct vsock_loopback *vsock = &the_vsock_loopback;
>@@ -58,6 +63,7 @@ static struct virtio_transport loopback_transport = {
> 		.module                   = THIS_MODULE,
>
> 		.get_local_cid            = vsock_loopback_get_local_cid,
>+		.supports_local_mode	  = vsock_loopback_supports_local_mode,
>
> 		.init                     = virtio_transport_do_socket_init,
> 		.destruct                 = virtio_transport_destruct,
>
>-- 
>2.47.3
>
Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
Posted by Bobby Eshleman 1 week, 6 days ago
On Tue, Nov 18, 2025 at 07:10:28PM +0100, Stefano Garzarella wrote:
> On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > 
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index 2c937a2df83b..c8319cd1c232 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
> > 	return VHOST_VSOCK_DEFAULT_HOST_CID;
> > }
> > 
> > +static bool vhost_transport_supports_local_mode(void)
> > +{
> > +	return true;
> 
> Should we enable this later, when we really add support, or it doesn't
> affect anything if vhost-vsock is not really supporting it in this PR
> (thinking about bisection issues).

sgtm!

> 
> > +}
> > +
> > /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> >  * RCU read lock.
> >  */
> > @@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
> > 		.module                   = THIS_MODULE,
> > 
> > 		.get_local_cid            = vhost_transport_get_local_cid,
> > +		.supports_local_mode	  = vhost_transport_supports_local_mode,
> > 
> > 		.init                     = virtio_transport_do_socket_init,
> > 		.destruct                 = virtio_transport_destruct,
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 59d97a143204..824d89657d41 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -180,6 +180,11 @@ struct vsock_transport {
> > 	/* Addressing. */
> > 	u32 (*get_local_cid)(void);
> > 
> > +	/* Return true if this transport supports VSOCK_NET_MODE_LOCAL.
> 
> nit: Here I would make it clearer that rather than supporting MODE_LOCAL,
> the transport is not compatible with it, etc.
> A summary of the excellent description we have in the commit.
> 

sounds good!

> > +	 * Otherwise, return false.
> > +	 */
> > +	bool (*supports_local_mode)(void);
> > +
> > 	/* Read a single skb */
> > 	int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
> > 
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index 54373ae101c3..7a235bb94437 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -91,6 +91,12 @@
> >  *   and locked down by a namespace manager. The default is "global". The mode
> >  *   is set per-namespace.
> >  *
> > + *   Note: LOCAL mode is only supported when using namespace-aware transports
> > + *   (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
> > + *   hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
> > + *   with EOPNOTSUPP, as these transports do not support per-namespace
> > + *   isolation.
> 
> Okay, maybe this is fine, so if you don't need to resend, feel free to
> ignore the previous comment.
> 
> > + *
> >  *   The modes affect the allocation and accessibility of CIDs as follows:
> >  *
> >  *   - global - access and allocation are all system-wide
> > @@ -2765,17 +2771,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> > 	if (*lenp >= sizeof(data))
> > 		return -EINVAL;
> > 
> > -	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
> > +	ret = 0;
> 
> IIUC `ret` should already be 0 at this point, no?
> 
> > +	mutex_lock(&vsock_register_mutex);
> 
> I honestly don't like to mix the parsing, with this new check, so what
> about leaving the parsing as before this patch (also without the mutex),
> then just (untested):
> 
> 	mutex_lock(&vsock_register_mutex);
> 	if (mode == VSOCK_NET_MODE_LOCAL && transport_g2h &&
> 	    transport_g2h->supports_local_mode &&
> 	    !transport_g2h->supports_local_mode()) {
> 		ret = -EOPNOTSUPP;
> 		goto out;
> 	}
> 
> 	if (!vsock_net_write_mode(net, mode)) {
> 		ret = -EPERM;
> 	}
> out:
> 	mutex_unlock(&vsock_register_mutex);
> 	return ret;
> }

Makes sense, I can move that around for next rev.

> 
> > +	if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> > 		mode = VSOCK_NET_MODE_GLOBAL;
> > -	else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
> > +	} else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
> > +		if (transport_g2h && transport_g2h->supports_local_mode &&
> > +		    !transport_g2h->supports_local_mode()) {
> > +			ret = -EOPNOTSUPP;
> > +			goto out;
> > +		}
> > 		mode = VSOCK_NET_MODE_LOCAL;
> > -	else
> > -		return -EINVAL;
> > +	} else {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > 
> > -	if (!vsock_net_write_mode(net, mode))
> > -		return -EPERM;
> > +	if (!vsock_net_write_mode(net, mode)) {
> > +		ret = -EPERM;
> > +		goto out;
> > +	}
> > 
> > -	return 0;
> > +out:
> > +	mutex_unlock(&vsock_register_mutex);
> > +	return ret;
> > }
> > 
> > static struct ctl_table vsock_table[] = {
> > @@ -2916,6 +2935,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > {
> > 	const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > 	int err = mutex_lock_interruptible(&vsock_register_mutex);
> > +	struct net *net;
> > 
> > 	if (err)
> > 		return err;
> > @@ -2938,6 +2958,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > 			err = -EBUSY;
> > 			goto err_busy;
> > 		}
> > +
> > +		/* G2H sockets break in LOCAL mode namespaces because G2H
> > +		 * transports don't support them yet. Block registering new G2H
> > +		 * transports if we already have local mode namespaces on the
> > +		 * system.
> > +		 */
> > +		rcu_read_lock();
> > +		for_each_net_rcu(net) {
> > +			if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
> > +				rcu_read_unlock();
> > +				err = -EOPNOTSUPP;
> > +				goto err_busy;
> > +			}
> > +		}
> > +		rcu_read_unlock();
> > +
> > 		t_g2h = t;
> > 	}
> > 
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 432fcbbd14d4..279f04fcd81a 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> > 	return -EOPNOTSUPP;
> > }
> > 
> > +static bool hvs_supports_local_mode(void)
> > +{
> > +	return false;
> > +}
> > +
> > static struct vsock_transport hvs_transport = {
> > 	.module                   = THIS_MODULE,
> > 
> > 	.get_local_cid            = hvs_get_local_cid,
> > +	.supports_local_mode      = hvs_supports_local_mode,
> > 
> > 	.init                     = hvs_sock_init,
> > 	.destruct                 = hvs_destruct,
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 5d379ccf3770..e585cb66c6f5 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
> > 	return ret;
> > }
> > 
> > +static bool virtio_transport_supports_local_mode(void)
> > +{
> > +	struct virtio_vsock *vsock;
> > +
> > +	rcu_read_lock();
> > +	vsock = rcu_dereference(the_virtio_vsock);
> > +	rcu_read_unlock();
> > +
> > +	/* Local mode is supported only when no G2H device is present. */
> > +	return vsock ? false : true;
> > +}
> > +
> > /* Caller need to hold vsock->tx_lock on vq */
> > static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
> > 				     struct virtio_vsock *vsock, gfp_t gfp)
> > @@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
> > 		.module                   = THIS_MODULE,
> > 
> > 		.get_local_cid            = virtio_transport_get_local_cid,
> > +		.supports_local_mode      = virtio_transport_supports_local_mode,
> > 
> > 		.init                     = virtio_transport_do_socket_init,
> > 		.destruct                 = virtio_transport_destruct,
> > diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> > index 7eccd6708d66..da7c52ad7b2a 100644
> > --- a/net/vmw_vsock/vmci_transport.c
> > +++ b/net/vmw_vsock/vmci_transport.c
> > @@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
> > 	return vmci_get_context_id();
> > }
> > 
> > +static bool vmci_transport_supports_local_mode(void)
> > +{
> > +	/* Local mode is supported only when no device is present. */
> > +	return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
> 
> IIRC vmci can be registered both as H2G and G2H, so should we filter out
> the H2G case?

In fact, I'm realizing now that this should probably just be:

static bool vmci_transport_supports_local_mode(void)
{
	return false;
}


... because even for H2G there is no mechanism for attaching a namespace
to a VM (unlike w/ vhost_vsock device open).

Does that seem right?

Best,
Bobby
Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
Posted by Stefano Garzarella 1 week, 5 days ago
On Tue, Nov 18, 2025 at 05:17:35PM -0800, Bobby Eshleman wrote:
>On Tue, Nov 18, 2025 at 07:10:28PM +0100, Stefano Garzarella wrote:
>> On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
>> > From: Bobby Eshleman <bobbyeshleman@meta.com>

[...]

>> > diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> > index 7eccd6708d66..da7c52ad7b2a 100644
>> > --- a/net/vmw_vsock/vmci_transport.c
>> > +++ b/net/vmw_vsock/vmci_transport.c
>> > @@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
>> > 	return vmci_get_context_id();
>> > }
>> >
>> > +static bool vmci_transport_supports_local_mode(void)
>> > +{
>> > +	/* Local mode is supported only when no device is present. */
>> > +	return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
>>
>> IIRC vmci can be registered both as H2G and G2H, so should we filter out
>> the H2G case?
>
>In fact, I'm realizing now that this should probably just be:
>
>static bool vmci_transport_supports_local_mode(void)
>{
>	return false;
>}
>
>
>... because even for H2G there is no mechanism for attaching a namespace
>to a VM (unlike w/ vhost_vsock device open).
>
>Does that seem right?

tl;dr   yes


vmci_transport.c has MODULE_ALIAS_NETPROTO(PF_VSOCK) for historical 
reasons. This means that the module is automatically loaded the first 
time PF_VSOCK is requested by the user if af_vsock is not loaded.

This was the case before vsock was generalized to support multiple 
transports and has remained so for historical reasons.

So today, we can have that module loaded, registered only for F_DGRAM 
but not registered for F_G2H and F_H2G, so maybe it could work for now 
and if the H2G is also not supporting it, maybe is the right thing to 
do. (with a better comment there on the reason why both G2H and H2G 
doesn't support it).

Sorry for the long reply, maybe just `yes` was fine, but I dumped what I 
thought because I feel it might be useful to you.

Thanks,
Stefano