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

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