Re: [PATCH net-next 2/6] vsock: add IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS

[Bobby Eshleman <bobbyeshleman@gmail.com>]

On Wed, Sep 16, 2026 at 02:57:43PM +0200, Stefano Garzarella wrote:
> On Tue, Sep 15, 2026 at 11:50:10AM -0700, Bobby Eshleman wrote:
> > On Tue, Sep 15, 2026 at 05:28:25PM +0200, Stefano Garzarella wrote:
> > > On Wed, Sep 02, 2026 at 04:00:48PM -0700, Bobby Eshleman wrote:
> > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > > >
> > > > Namespaces let a host isolate a VM's vsock traffic to a specific
> > > > namespace, but in a guest vsock traffic cannot be isolated to a
> > > > namespace. The vsock device is hardcoded to global mode and can't be
> > > > moved into a local-mode namespace.
> > > >
> > > > Introduce ioctl IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS on /dev/vsock that
> > > 
> > > We already discussed about netlink, but I'm not sure how much work can
> > > take, here just another alternative, what about adding
> > > /proc/sys/net/vsock/g2h_owner sysctl that can be read/write where:
> > > 0 - no owner
> > > 1 - owner
> > > 
> > > 0 -> 1 transition, move the device in the new nets
> > > 1 -> 0 transition, reset back to init_ns
> > 
> > That seems reasonable, but if multi-device support ever lands, this
> > might be harder to adapt cleanly?
> 
> I see, but maybe we can have a dev0/ dev1/ subfolders in that case.
> 
> > 
> > > 
> > > > gives userspace a way to move the device to the calling pid's namespace.
> > > > The call requires CAP_NET_ADMIN in the root user namespace. A privileged
> > > > user wishing to "unassign" the device can move it to the init_netns,
> > > > which is hardcoded to global mode (so no unassign call is necessary).
> > > >
> > > > A getter to read the current assignment back was considered, returning
> > > > either the namespace's net_cookie or its nsfs inode number, but neither
> > > > seemed useful enough to bake into the uAPI now. It can be added later if
> > > > a user turns up that needs it.
> > > >
> > > > Add a transport hook to indicate support for guest namespacing, so that
> > > > transports may opt in/out. A transport that opts out keeps the
> > > > reachability rules it had before this ioctl existed.
> > > >
> > > > Sockets are reset when the underlying device moves to a different
> > > > namespace, so as to prevent reachability from the previous and now
> > > > disallowed namespace.
> > > >
> > > > Following the approach of netdevs, the device returns to init_net when
> > > > its namespace is removed. Care is taken to not break flows when the
> > > > device is inside a global namespace that is being torn down and alive
> > > > sockets are in a different global namespace. In this scenario, the
> > > > device's netns getter pre-emptively falls back to the init_net (always
> > > > global) so that these flows are not disrupted. If init_netns ever
> > > > supports local-mode in the future, this logic will have to be changed.
> > > >
> > > > Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > Link: https://lore.kernel.org/all/20200427142518.uwssa6dtasrp3bfc@steredhat/
> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > > > ---
> > > > Documentation/admin-guide/sysctl/net.rst |  18 +++
> > > > include/net/af_vsock.h                   |   7 ++
> > > > include/uapi/linux/vm_sockets.h          |   6 +
> > > > net/vmw_vsock/af_vsock.c                 | 198 ++++++++++++++++++++++++++++++-
> > > > 4 files changed, 228 insertions(+), 1 deletion(-)
> > > >
> 
> [...]
> 
> > > > +
> > > > +static void vsock_reset_unreachable_sock(struct sock *sk)
> > > > +{
> > > > +	if (vsock_g2h_net_reachable(sock_net(sk)))
> > > > +		return;
> > > > +
> > > > +	sk->sk_state = TCP_CLOSE;
> > > > +	sk->sk_err = ECONNRESET;
> > > > +	sk_error_report(sk);
> > > 
> > > Should we send the reset to the other peer too?
> > > 
> > > Or avoid to set TCP_CLOSE, so the user will see the error and close it?
> > 
> > Sending the reset sounds good. At least the peer won't be waiting around
> > forever. Will work that into v2.
> 
> If it's too much of work, we can skip it, it's not a strong opinion on my
> side. It was more a question of what we should do ;-)
> 
> > 
> > > 
> > > > +}
> > > > +
> > > > +/* Move the g2h device to @net. Returns -ENODEV if no g2h transport is loaded
> > > > + * and -EOPNOTSUPP if the loaded one cannot be moved.
> > > > + */
> > > > +static int vsock_g2h_net_assign(struct net *net)
> > > > +{
> > > > +	int ret = 0;
> > > > +
> > > > +	mutex_lock(&vsock_register_mutex);
> > > > +	if (!transport_g2h) {
> > > > +		ret = -ENODEV;
> > > > +	} else if (!vsock_g2h_netns_assignable()) {
> > > > +		ret = -EOPNOTSUPP;
> > > > +	} else {
> > > > +		/* See vsock_maybe_set_connected() comment about synchronizing
> > > > +		 * with connecting sockets.
> > > > +		 */
> > > > +		rcu_assign_pointer(vsock_g2h_net, net);
> > > > +		vsock_for_each_connected_socket(transport_g2h,
> > > > +						vsock_reset_unreachable_sock);
> > > > +	}
> > > > +	mutex_unlock(&vsock_register_mutex);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +/* Move the g2h device back to init_net if it lives in @net, which is about to
> > > > + * be destroyed.
> > > > + */
> > > > +static void vsock_g2h_net_reset(struct net *net)
> > > > +{
> > > > +	bool reset = false;
> > > > +
> > > > +	/* Avoid taking the mutex if the namespaces don't match. */
> > > > +	if (likely(rcu_access_pointer(vsock_g2h_net) != net))
> > > > +		return;
> > > > +
> > > > +	mutex_lock(&vsock_register_mutex);
> > > > +	if (rcu_access_pointer(vsock_g2h_net) == net) {
> > > > +		rcu_assign_pointer(vsock_g2h_net, &init_net);
> > > > +		reset = true;
> > > > +	}
> > > > +	mutex_unlock(&vsock_register_mutex);
> > > > +
> > > > +	if (reset)
> > > > +		synchronize_rcu();
> > > > +}
> > > > +
> > > > static struct sock *vsock_dequeue_accept(struct sock *listener)
> > > > {
> > > > 	struct vsock_sock *vlistener;
> > > > @@ -2745,6 +2920,15 @@ static long vsock_dev_do_ioctl(struct file *filp,
> > > > 			retval = -EFAULT;
> > > > 		break;
> > > >
> > > > +	case IOCTL_VM_SOCKETS_ASSIGN_G2H_NETNS:
> > > > +		if (!capable(CAP_NET_ADMIN)) {
> > > > +			retval = -EPERM;
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		retval = vsock_g2h_net_assign(current->nsproxy->net_ns);
> > > > +		break;
> > > > +
> > > > 	default:
> > > > 		retval = -ENOIOCTLCMD;
> > > > 	}
> > > > @@ -2978,6 +3162,7 @@ static __net_init int vsock_sysctl_init_net(struct net *net)
> > > >
> > > > static __net_exit void vsock_sysctl_exit_net(struct net *net)
> > > > {
> > > > +	vsock_g2h_net_reset(net);
> > > 
> > > Why calling this in the sysctl_exit ?
> > 
> > Should we add another pernet_operations? I agree the name of
> > vsock_sysctl_ops does not really fit as a general "do all per-net
> > operations here" location.
> 
> Aaaaa, it's not really related to sysctl.
> 
> Maybe we can rename it or add another pernet_operations.
> 
> > 
> > > 
> > > > 	vsock_sysctl_unregister(net);
> > > > }
> > > >
> > > > @@ -3104,13 +3289,21 @@ EXPORT_SYMBOL_GPL(vsock_core_register);
> > > >
> > > > void vsock_core_unregister(const struct vsock_transport *t)
> > > > {
> > > > +	bool g2h_net_reset = false;
> > > > +
> > > > 	mutex_lock(&vsock_register_mutex);
> > > >
> > > > 	if (transport_h2g == t)
> > > > 		transport_h2g = NULL;
> > > >
> > > > -	if (transport_g2h == t)
> > > > +	if (transport_g2h == t) {
> > > > 		transport_g2h = NULL;
> > > > +		/* The device is gone, so is its namespace assignment. */
> > > > +		if (rcu_access_pointer(vsock_g2h_net) != &init_net) {
> > > > +			rcu_assign_pointer(vsock_g2h_net, &init_net);
> > > > +			g2h_net_reset = true;
> > > > +		}
> > > > +	}
> > > >
> > > > 	if (transport_dgram == t)
> > > > 		transport_dgram = NULL;
> > > > @@ -3119,6 +3312,9 @@ void vsock_core_unregister(const struct vsock_transport *t)
> > > > 		transport_local = NULL;
> > > >
> > > > 	mutex_unlock(&vsock_register_mutex);
> > > > +
> > > > +	if (g2h_net_reset)
> > > > +		synchronize_rcu();
> > > 
> > > Why we need this? (I'd add also a comment with the reason)
> > 
> > We might actually be able to drop it... I do think we need to keep it in
> > vsock_g2h_net_reset() because it makes sure that vsock_g2h_net_get()
> > doesn't dereference a destroyed net.
> > 
> > vsock_core_unregister() isn't on the teardown path, so even if we
> > re-assign to init_net the previous net is still alive.
> 
> Agree.
> 
> > 
> > 
> > One thing I realized when answering your uAPI questions and looking at
> > the code is that IOCTL_VM_SOCKETS_GET_LOCAL_CID is not namespace aware,
> > but I think it probably should be? I remember for host ns we strived for
> > truly strong isolation.
> 
> Can you elabore a bit more?
> 

I'm wondering if IOCTL_VM_SOCKETS_GET_LOCAL_CID should not reveal the
CID of the g2h device if the device has been moved to an inaccessible
namespace? In this case, fallback to the results of h2g/local and
through to returning VMADDR_CID_ANY if neither of those transports are
loaded.

I also considered whether or not /dev/vsock should even be visible from
within an inaccessible namespace in order to not leak even the mere
presence of the vsock device, but it seems to me that /dev/vsock's
presence doesn't leak anything besides the presence of the vsock module.

Best,
Bobby