* [PATCH net-next v2 1/6] vsock: constify the transport in vsock_for_each_connected_socket()
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 2/6] vsock: rename the vsock pernet operations Bobby Eshleman
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
From: Bobby Eshleman <bobbyeshleman@meta.com>
Allow const transports to be passed too. The function only compares the
pointer against vsk->transport, which is itself const, and never writes
through it.
No functional change.
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
include/net/af_vsock.h | 2 +-
net/vmw_vsock/af_vsock.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 5549298c1ec6..370fcd3ddabc 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -233,7 +233,7 @@ bool vsock_check_source(const struct vsock_sock *vsk,
const struct vsock_transport *transport,
const struct sockaddr_vm *src);
void vsock_remove_sock(struct vsock_sock *vsk);
-void vsock_for_each_connected_socket(struct vsock_transport *transport,
+void vsock_for_each_connected_socket(const struct vsock_transport *transport,
void (*fn)(struct sock *sk));
int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk);
bool vsock_find_cid(unsigned int cid);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index f840498b58af..8d6b705f8640 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -480,7 +480,7 @@ void vsock_remove_sock(struct vsock_sock *vsk)
}
EXPORT_SYMBOL_GPL(vsock_remove_sock);
-void vsock_for_each_connected_socket(struct vsock_transport *transport,
+void vsock_for_each_connected_socket(const struct vsock_transport *transport,
void (*fn)(struct sock *sk))
{
int i;
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v2 2/6] vsock: rename the vsock pernet operations
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 1/6] vsock: constify the transport in vsock_for_each_connected_socket() Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns Bobby Eshleman
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
From: Bobby Eshleman <bobbyeshleman@meta.com>
vsock_sysctl_ops and its two callbacks are named for the sysctl tables
they register, but the init callback already does more than that: it
also initialises the per-namespace vsock state through vsock_net_init().
Rename them to vsock_pernet_ops, vsock_pernet_init and
vsock_pernet_exit, so that later per-namespace work has somewhere to go
that does not read as sysctl handling.
No functional change.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- New patch
---
net/vmw_vsock/af_vsock.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 8d6b705f8640..95a435aef512 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2998,7 +2998,7 @@ static void vsock_net_init(struct net *net)
net->vsock.g2h_fallback = 1;
}
-static __net_init int vsock_sysctl_init_net(struct net *net)
+static __net_init int vsock_pernet_init(struct net *net)
{
vsock_net_init(net);
@@ -3008,14 +3008,14 @@ static __net_init int vsock_sysctl_init_net(struct net *net)
return 0;
}
-static __net_exit void vsock_sysctl_exit_net(struct net *net)
+static __net_exit void vsock_pernet_exit(struct net *net)
{
vsock_sysctl_unregister(net);
}
-static struct pernet_operations vsock_sysctl_ops = {
- .init = vsock_sysctl_init_net,
- .exit = vsock_sysctl_exit_net,
+static struct pernet_operations vsock_pernet_ops = {
+ .init = vsock_pernet_init,
+ .exit = vsock_pernet_exit,
};
static int __init vsock_init(void)
@@ -3045,7 +3045,7 @@ static int __init vsock_init(void)
goto err_unregister_proto;
}
- if (register_pernet_subsys(&vsock_sysctl_ops)) {
+ if (register_pernet_subsys(&vsock_pernet_ops)) {
err = -ENOMEM;
goto err_unregister_sock;
}
@@ -3069,7 +3069,7 @@ static void __exit vsock_exit(void)
misc_deregister(&vsock_device);
sock_unregister(AF_VSOCK);
proto_unregister(&vsock_proto);
- unregister_pernet_subsys(&vsock_sysctl_ops);
+ unregister_pernet_subsys(&vsock_pernet_ops);
}
const struct vsock_transport *vsock_core_get_transport(struct vsock_sock *vsk)
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 1/6] vsock: constify the transport in vsock_for_each_connected_socket() Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 2/6] vsock: rename the vsock pernet operations Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-22 1:18 ` [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace Bobby Eshleman
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
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 a vsock generic netlink family whose one command,
VSOCK_CMD_DEV_NETNS_SET, moves the device to the namespace the request
was sent from.
VSOCK_CMD_DEV_NETNS_GET reads the assignment back, reporting the
namespace as an nsid relative to the caller.
The command requires CAP_NET_ADMIN in the initial user namespace. A
privileged user wishing to "unassign" the device can move it to the
init_net, which is hardcoded to global mode (so no unassign call is
necessary).
Add a transport flag 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 command 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.
The device's CID must not be observable from such a namespace either. It
is reached through three paths: IOCTL_VM_SOCKETS_GET_LOCAL_CID reports
it, bind() accepts it because vsock_find_cid() matches it, and connect()
to it selects the loopback transport because vsock_use_local_transport()
compares against it. All three read it through
vsock_registered_transport_cid(), which now takes the namespace asking
and reports VMADDR_CID_ANY for the g2h slot when that namespace cannot
reach the device.
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_net 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>
---
Changes in v2:
- New patch, replaces the ioctl with a genl family and
VSOCK_CMD_DEV_NETNS_SET (Stefano)
- Make netns_assign_allow a bool, not a callback (Stefano)
- One vsock_netns_assignable(t) helper for both call sites (Stefano)
- Make vsock_g2h_reachable_sk() static, drop the export (Stefano)
- RST the peer when socket loses access to the device's net (Stefano)
- Move the netns reset out of the sysctl exit hook, into the renamed
vsock_pernet_ops (Stefano).
- Remove the synchronize_rcu() from the pernet exit path. The
net_namespace documentation states that synchronize_rcu() should be
avoided in the pernet exit path.
- Run the socket reset in the pernet pre_exit() hook, as the
net_namespace management code (and documentation) guarantees
synchronize_rcu() between pre_exit() and exit()
- Drop the conditional synchronize_rcu() in vsock_core_unregister()
(Stefano)
- Specifically state "G2H transport" in the netns_assign_allow comment (Stefano)
- Avoid leaking the g2h CID to a namespace that cannot reach it;
GET_LOCAL_CID, bind() and loopback transport selection all read it
through vsock_registered_transport_cid(), which now takes a netns
- Add VSOCK_CMD_DEV_NETNS_GET
---
Documentation/admin-guide/sysctl/net.rst | 23 ++
Documentation/netlink/specs/vsock.yaml | 68 ++++++
MAINTAINERS | 2 +
drivers/vhost/vsock.c | 6 +-
include/net/af_vsock.h | 17 +-
include/uapi/linux/vsock.h | 28 +++
net/vmw_vsock/Makefile | 2 +-
net/vmw_vsock/af_vsock.c | 358 +++++++++++++++++++++++++++++--
net/vmw_vsock/vsock_nl_gen.c | 36 ++++
net/vmw_vsock/vsock_nl_gen.h | 20 ++
tools/net/ynl/Makefile.deps | 1 +
11 files changed, 543 insertions(+), 18 deletions(-)
diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
index fe43e8595958..f2d8e4e84f89 100644
--- a/Documentation/admin-guide/sysctl/net.rst
+++ b/Documentation/admin-guide/sysctl/net.rst
@@ -529,6 +529,29 @@ their hosts. The behavior of VSOCK sockets in a network namespace is determined
by the namespace's mode (``global`` or ``local``), which controls how CIDs
(Context IDs) are allocated and how sockets interact across namespaces.
+In a guest, the vsock device owned by the guest-to-host (G2H) transport belongs
+to one network namespace at a time. The ``VSOCK_CMD_DEV_NETNS_SET`` command of
+the ``vsock`` netlink family, described in
+Documentation/netlink/specs/vsock.yaml, moves it to the namespace the request
+was sent from, which requires ``CAP_NET_ADMIN`` in the initial user namespace.
+The namespace's mode decides who may then use the device:
+
+- ``global`` - every ``global`` mode namespace may use it.
+- ``local`` - only that namespace may use it, which reserves the connection to
+ the host for it alone.
+
+The device starts out in the initial namespace, so until the command is issued
+nothing has moved and no mode has changed.
+
+Support is transport dependent. A G2H transport that does not implement the
+move refuses the command with ``EOPNOTSUPP``; of the in-tree guest transports
+only virtio-vsock implements it.
+
+Connections made before the move, from a namespace that can no longer reach the
+device, are reset. The device returns to the initial namespace when the
+namespace it was moved to is deleted, so assigning it to the initial namespace
+is how an assignment is undone.
+
ns_mode
-------
diff --git a/Documentation/netlink/specs/vsock.yaml b/Documentation/netlink/specs/vsock.yaml
new file mode 100644
index 000000000000..a33e02cb6f34
--- /dev/null
+++ b/Documentation/netlink/specs/vsock.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+#
+# Copyright (c) 2026 Meta Platforms, Inc. and affiliates
+#
+---
+name: vsock
+
+doc: |
+ Control interface for the vsock (AF_VSOCK) core.
+
+protocol: genetlink
+
+uapi-header: linux/vsock.h
+
+attribute-sets:
+ -
+ name: vsock
+ attributes:
+ -
+ name: netns-id
+ type: s32
+ doc: |
+ Network namespace the guest's vsock device is assigned to, as an
+ nsid relative to the caller.
+
+ Absent when the device is in the caller's own namespace, and
+ NETNSA_NSID_NOT_ASSIGNED when the caller's namespace cannot reach
+ the device under the mode rules, or when no nsid could be
+ allocated for it.
+
+operations:
+ list:
+ -
+ name: dev-netns-set
+ doc: |
+ Set the network namespace of the guest's vsock device, the one owned
+ by the guest-to-host transport.
+
+ The device is moved to the namespace the request was sent from. It
+ starts out in the initial namespace, and moving it back there is how
+ an assignment is undone.
+
+ Support is transport dependent: a guest-to-host transport that does
+ not implement the move refuses the request with EOPNOTSUPP.
+
+ The namespace's mode decides who may then use the device: a global
+ mode namespace shares it with every other global mode namespace,
+ while a local mode namespace reserves it for itself.
+
+ Connections made before the assignment, from a namespace that can no
+ longer reach the device, are reset.
+ attribute-set: vsock
+ flags: [admin-perm]
+ do: {}
+ -
+ name: dev-netns-get
+ doc: |
+ Get the network namespace the guest's vsock device is assigned to.
+
+ The namespace is reported as an nsid relative to the caller, so the
+ attribute is absent when the device is already in the caller's own
+ namespace. A caller whose namespace cannot reach the device is told
+ NETNSA_NSID_NOT_ASSIGNED rather than where the device went.
+ attribute-set: vsock
+ do:
+ reply:
+ attributes:
+ - netns-id
diff --git a/MAINTAINERS b/MAINTAINERS
index df8ab9b82402..e30e6f8d71e2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29143,10 +29143,12 @@ M: Stefano Garzarella <sgarzare@redhat.com>
L: virtualization@lists.linux.dev
L: netdev@vger.kernel.org
S: Maintained
+F: Documentation/netlink/specs/vsock.yaml
F: drivers/net/vsockmon.c
F: include/net/af_vsock.h
F: include/uapi/linux/vm_sockets.h
F: include/uapi/linux/vm_sockets_diag.h
+F: include/uapi/linux/vsock.h
F: include/uapi/linux/vsockmon.h
F: net/vmw_vsock/
F: tools/testing/selftests/vsock/
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index abed1fbcf66c..badc064964b3 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -828,9 +828,11 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
return -EINVAL;
/* Refuse if CID is assigned to the guest->host transport (i.e. nested
- * VM), to make the loopback work.
+ * VM), to make the loopback work. Only when that device is reachable
+ * from this VM's namespace, which is the same test the guest CID
+ * collision check below applies.
*/
- if (vsock_find_cid(guest_cid))
+ if (vsock_find_cid(vsock->net, guest_cid))
return -EADDRINUSE;
/* Refuse if CID is already in use */
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 370fcd3ddabc..dbf1a6aa367f 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -35,6 +35,8 @@ struct vsock_sock {
/* Links for the global tables of bound and connected sockets. */
struct list_head bound_table;
struct list_head connected_table;
+ /* Protected by vsock_register_mutex. */
+ struct list_head pending_reset;
/* Accessed without the socket lock held. This means it can never be
* modified outsided of socket create or destruct.
*/
@@ -190,6 +192,16 @@ struct vsock_transport {
/* Zero-copy. */
bool (*msgzerocopy_allow)(void);
+
+ /* True if the G2H transport honours VSOCK_CMD_DEV_NETNS_SET. A
+ * transport that sets this must also implement reset.
+ */
+ bool netns_assign_allow;
+
+ /* Send a reset to @vsk's peer. @skb is the packet being replied to, or
+ * NULL when the reset is not a reply. May sleep.
+ */
+ int (*reset)(struct vsock_sock *vsk, struct sk_buff *skb);
};
/**** CORE ****/
@@ -236,8 +248,11 @@ void vsock_remove_sock(struct vsock_sock *vsk);
void vsock_for_each_connected_socket(const struct vsock_transport *transport,
void (*fn)(struct sock *sk));
int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk);
-bool vsock_find_cid(unsigned int cid);
+bool vsock_find_cid(struct net *net, unsigned int cid);
void vsock_linger(struct sock *sk);
+struct net *vsock_g2h_net_get(void);
+bool vsock_g2h_net_reachable(struct net *net);
+bool vsock_maybe_set_connected(struct vsock_sock *vsk);
/**** TAP ****/
diff --git a/include/uapi/linux/vsock.h b/include/uapi/linux/vsock.h
new file mode 100644
index 000000000000..803b9aa1f0b3
--- /dev/null
+++ b/include/uapi/linux/vsock.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/vsock.yaml */
+/* YNL-GEN uapi header */
+/* To regenerate run: tools/net/ynl/ynl-regen.sh */
+
+#ifndef _UAPI_LINUX_VSOCK_H
+#define _UAPI_LINUX_VSOCK_H
+
+#define VSOCK_FAMILY_NAME "vsock"
+#define VSOCK_FAMILY_VERSION 1
+
+enum {
+ VSOCK_A_NETNS_ID = 1,
+
+ __VSOCK_A_MAX,
+ VSOCK_A_MAX = (__VSOCK_A_MAX - 1)
+};
+
+enum {
+ VSOCK_CMD_DEV_NETNS_SET = 1,
+ VSOCK_CMD_DEV_NETNS_GET,
+
+ __VSOCK_CMD_MAX,
+ VSOCK_CMD_MAX = (__VSOCK_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_VSOCK_H */
diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile
index 5da74c4a9f1d..97e2a559e2bf 100644
--- a/net/vmw_vsock/Makefile
+++ b/net/vmw_vsock/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_VIRTIO_VSOCKETS_COMMON) += vmw_vsock_virtio_transport_common.o
obj-$(CONFIG_HYPERV_VSOCKETS) += hv_sock.o
obj-$(CONFIG_VSOCKETS_LOOPBACK) += vsock_loopback.o
-vsock-y += af_vsock.o af_vsock_tap.o vsock_addr.o
+vsock-y += af_vsock.o af_vsock_tap.o vsock_addr.o vsock_nl_gen.o
vsock-$(CONFIG_BPF_SYSCALL) += vsock_bpf.o
vsock_diag-y += diag.o
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 95a435aef512..9938dd501019 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -130,6 +130,24 @@
* a different transport that *does* support local mode. For
* example, virtio-vsock may not support local mode, but the socket
* may still accept a connection from vhost-vsock which does.
+ *
+ * - A guest has a single vsock device, owned by the guest->host transport.
+ * The VSOCK_CMD_DEV_NETNS_SET netlink command moves it to the namespace the
+ * request was sent from. It starts out in init_net. The mode rules then
+ * decide who may use it, and which namespace packets from the host are
+ * delivered to:
+ *
+ * - assigned to a global mode namespace - every global mode namespace may
+ * use it. Until the command is issued nothing has moved and no mode has
+ * changed, so the default is the behaviour that predates it.
+ * - assigned to a local mode namespace - only that namespace may use it.
+ * This is how a nested VM is isolated from the rest of the guest.
+ *
+ * Connections made before an assignment, from a namespace that can no
+ * longer reach the device, are reset.
+ *
+ * No reference is taken on the assigned namespace. As is done for netdevs,
+ * the device is moved back to init_net when that namespace is destroyed.
*/
#include <linux/compat.h>
@@ -161,10 +179,14 @@
#include <linux/workqueue.h>
#include <net/sock.h>
#include <net/af_vsock.h>
+#include <linux/net_namespace.h>
+#include <net/genetlink.h>
#include <net/netns/vsock.h>
#include <uapi/linux/vm_sockets.h>
#include <uapi/asm-generic/ioctls.h>
+#include "vsock_nl_gen.h"
+
#define VSOCK_NET_MODE_STR_GLOBAL "global"
#define VSOCK_NET_MODE_STR_LOCAL "local"
@@ -208,6 +230,11 @@ static const struct vsock_transport *transport_dgram;
static const struct vsock_transport *transport_local;
static DEFINE_MUTEX(vsock_register_mutex);
+/* Network namespace of the g2h device. Protected by
+ * vsock_register_mutex/RCU.
+ */
+static struct net __rcu *vsock_g2h_net = RCU_INITIALIZER(&init_net);
+
/**** UTILS ****/
/* Each bound VSocket is stored in the bind hash table and each connected
@@ -553,7 +580,37 @@ void vsock_enqueue_accept(struct sock *listener, struct sock *connected)
}
EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
-static bool vsock_use_local_transport(unsigned int remote_cid)
+/* Return true if @t honours namespace assignment. One that does not keeps the
+ * reachability rules it had before VSOCK_CMD_DEV_NETNS_SET existed.
+ */
+static bool vsock_netns_assignable(const struct vsock_transport *t)
+{
+ return t && t->netns_assign_allow && t->reset;
+}
+
+/* Return the CID of the transport in @transport, as seen from @net.
+ *
+ * The g2h device is the one that can move between namespaces, so a @net that
+ * cannot reach it is told VMADDR_CID_ANY: the same answer it would get if no
+ * g2h transport were registered at all.
+ */
+static u32
+__vsock_registered_transport_cid(const struct vsock_transport **transport,
+ struct net *net)
+{
+ lockdep_assert_held(&vsock_register_mutex);
+
+ if (!*transport)
+ return VMADDR_CID_ANY;
+
+ if (transport == &transport_g2h && vsock_netns_assignable(*transport) &&
+ !vsock_g2h_net_reachable(net))
+ return VMADDR_CID_ANY;
+
+ return (*transport)->get_local_cid();
+}
+
+static bool vsock_use_local_transport(struct net *net, unsigned int remote_cid)
{
lockdep_assert_held(&vsock_register_mutex);
@@ -564,7 +621,12 @@ static bool vsock_use_local_transport(unsigned int remote_cid)
return true;
if (transport_g2h) {
- return remote_cid == transport_g2h->get_local_cid();
+ u32 cid = __vsock_registered_transport_cid(&transport_g2h, net);
+
+ /* The device may be unreachable from @net, in which case
+ * @remote_cid is not the local CID.
+ */
+ return cid != VMADDR_CID_ANY && remote_cid == cid;
} else {
return remote_cid == VMADDR_CID_HOST;
}
@@ -626,7 +688,7 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
break;
case SOCK_STREAM:
case SOCK_SEQPACKET:
- if (vsock_use_local_transport(remote_cid))
+ if (vsock_use_local_transport(sock_net(sk), remote_cid))
new_transport = transport_local;
else if (remote_cid <= VMADDR_CID_HOST ||
(remote_flags & VMADDR_FLAG_TO_HOST))
@@ -654,6 +716,13 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
goto err;
}
+ if (new_transport && new_transport == transport_g2h &&
+ vsock_netns_assignable(new_transport) &&
+ !vsock_g2h_net_reachable(sock_net(sk))) {
+ ret = -ENETUNREACH;
+ goto err;
+ }
+
/* We increase the module refcnt to prevent the transport unloading
* while there are open sockets assigned to it.
*/
@@ -715,21 +784,22 @@ EXPORT_SYMBOL_GPL(vsock_assign_transport);
* Provide safe access to static transport_{h2g,g2h,dgram,local} callbacks.
* Otherwise we may race with module removal. Do not use on `vsk->transport`.
*/
-static u32 vsock_registered_transport_cid(const struct vsock_transport **transport)
+static u32
+vsock_registered_transport_cid(const struct vsock_transport **transport,
+ struct net *net)
{
- u32 cid = VMADDR_CID_ANY;
+ u32 cid;
mutex_lock(&vsock_register_mutex);
- if (*transport)
- cid = (*transport)->get_local_cid();
+ cid = __vsock_registered_transport_cid(transport, net);
mutex_unlock(&vsock_register_mutex);
return cid;
}
-bool vsock_find_cid(unsigned int cid)
+bool vsock_find_cid(struct net *net, unsigned int cid)
{
- if (cid == vsock_registered_transport_cid(&transport_g2h))
+ if (cid == vsock_registered_transport_cid(&transport_g2h, net))
return true;
if (transport_h2g && cid == VMADDR_CID_HOST)
@@ -742,6 +812,173 @@ bool vsock_find_cid(unsigned int cid)
}
EXPORT_SYMBOL_GPL(vsock_find_cid);
+/* Return the namespace the g2h device is assigned to, with a reference held,
+ * or NULL when that namespace is going away and the init_net cannot stand in
+ * for it.
+ */
+struct net *vsock_g2h_net_get(void)
+{
+ struct net *assigned;
+ struct net *net;
+
+ rcu_read_lock();
+ assigned = rcu_dereference(vsock_g2h_net);
+ net = maybe_get_net(assigned);
+
+ /* !net means the net is about to be destroyed, at which point the g2h
+ * device will move to the init_net. If the init_net and the dying net
+ * are both global mode, we use the init_net as a fallback to avoid
+ * disrupting global-mode flows. The per-net destructor hook will
+ * eventually move the g2h device to the init_net anyway.
+ *
+ * vsock_net_check_mode() is safe here because 'assigned' is pointing
+ * to a net that won't be freed until the following rcu grace period.
+ */
+ if (!net && vsock_net_check_mode(&init_net, assigned))
+ net = get_net(&init_net);
+ rcu_read_unlock();
+
+ return net;
+}
+EXPORT_SYMBOL_GPL(vsock_g2h_net_get);
+
+bool vsock_g2h_net_reachable(struct net *net)
+{
+ bool reachable;
+
+ rcu_read_lock();
+ reachable = vsock_net_check_mode(net, rcu_dereference(vsock_g2h_net));
+ rcu_read_unlock();
+
+ return reachable;
+}
+EXPORT_SYMBOL_GPL(vsock_g2h_net_reachable);
+
+static bool vsock_g2h_reachable_sk(struct vsock_sock *vsk)
+{
+ if (!vsock_netns_assignable(vsk->transport))
+ return true;
+
+ return vsock_g2h_net_reachable(sock_net(sk_vsock(vsk)));
+}
+
+/* Move @vsk to TCP_ESTABLISHED and into the connected table, unless the device
+ * has moved to a namespace @vsk cannot reach. Returns false without doing
+ * either in that case.
+ *
+ * The reset sweep walks the same table under the same lock, so an assign
+ * cannot land between the check and the insert: either the sweep finds @vsk
+ * and resets it, or @vsk is never added.
+ */
+bool vsock_maybe_set_connected(struct vsock_sock *vsk)
+{
+ struct list_head *list;
+ bool reachable;
+
+ list = vsock_connected_sockets(&vsk->remote_addr, &vsk->local_addr);
+
+ spin_lock_bh(&vsock_table_lock);
+ reachable = vsock_g2h_reachable_sk(vsk);
+ if (reachable) {
+ sk_vsock(vsk)->sk_state = TCP_ESTABLISHED;
+ __vsock_insert_connected(list, vsk);
+ }
+ spin_unlock_bh(&vsock_table_lock);
+
+ return reachable;
+}
+EXPORT_SYMBOL_GPL(vsock_maybe_set_connected);
+
+/* Reset every connected socket of @t that can no longer reach the g2h device,
+ * and let the transport tell each peer.
+ */
+static void vsock_g2h_reset_unreachable(const struct vsock_transport *t)
+{
+ struct vsock_sock *vsk, *tmp;
+ LIST_HEAD(reset_list);
+ struct sock *sk;
+ int i;
+
+ /* The calling context must hold vsock_register_mutex, which serializes
+ * concurrent netns assignments' use of vsk->pending_reset.
+ */
+ lockdep_assert_held(&vsock_register_mutex);
+
+ spin_lock_bh(&vsock_table_lock);
+
+ for (i = 0; i < ARRAY_SIZE(vsock_connected_table); i++) {
+ list_for_each_entry(vsk, &vsock_connected_table[i],
+ connected_table) {
+ sk = sk_vsock(vsk);
+
+ if (vsk->transport != t ||
+ sk->sk_state == TCP_CLOSE ||
+ vsock_g2h_reachable_sk(vsk))
+ continue;
+
+ sk->sk_state = TCP_CLOSE;
+ sk->sk_err = ECONNRESET;
+ sk_error_report(sk);
+
+ sock_hold(sk);
+ list_add_tail(&vsk->pending_reset, &reset_list);
+ }
+ }
+
+ spin_unlock_bh(&vsock_table_lock);
+
+ /* Reset outside of spinlock because the transport may sleep
+ * (e.g., GFP_KERNEL alloc).
+ */
+ list_for_each_entry_safe(vsk, tmp, &reset_list, pending_reset) {
+ list_del_init(&vsk->pending_reset);
+ t->reset(vsk, NULL);
+ sock_put(sk_vsock(vsk));
+ }
+}
+
+/* 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_netns_assignable(transport_g2h)) {
+ ret = -EOPNOTSUPP;
+ } else {
+ /* See vsock_maybe_set_connected() comment about synchronizing
+ * with connecting sockets.
+ */
+ rcu_assign_pointer(vsock_g2h_net, net);
+ vsock_g2h_reset_unreachable(transport_g2h);
+ }
+ 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.
+ */
+/* Runs as .pre_exit: pernet_operations guarantees a synchronize_rcu()
+ * between pre_exit() and exit(), which drains the readers this drops.
+ */
+static void __net_exit vsock_g2h_net_reset(struct net *net)
+{
+ /* 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);
+ mutex_unlock(&vsock_register_mutex);
+}
+
static struct sock *vsock_dequeue_accept(struct sock *listener)
{
struct vsock_sock *vlistener;
@@ -908,7 +1145,8 @@ static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr)
* like AF_INET prevents binding to a non-local IP address (in most
* cases), we only allow binding to a local CID.
*/
- if (addr->svm_cid != VMADDR_CID_ANY && !vsock_find_cid(addr->svm_cid))
+ if (addr->svm_cid != VMADDR_CID_ANY &&
+ !vsock_find_cid(sock_net(sk_vsock(vsk)), addr->svm_cid))
return -EADDRNOTAVAIL;
switch (sk->sk_socket->type) {
@@ -967,6 +1205,7 @@ static struct sock *__vsock_create(struct net *net,
INIT_LIST_HEAD(&vsk->bound_table);
INIT_LIST_HEAD(&vsk->connected_table);
+ INIT_LIST_HEAD(&vsk->pending_reset);
vsk->listener = NULL;
INIT_LIST_HEAD(&vsk->pending_links);
INIT_LIST_HEAD(&vsk->accept_queue);
@@ -2760,6 +2999,7 @@ static long vsock_dev_do_ioctl(struct file *filp,
{
u32 __user *p = ptr;
int retval = 0;
+ struct net *net;
u32 cid;
switch (cmd) {
@@ -2767,11 +3007,14 @@ static long vsock_dev_do_ioctl(struct file *filp,
/* To be compatible with the VMCI behavior, we prioritize the
* guest CID instead of well-know host CID (VMADDR_CID_HOST).
*/
- cid = vsock_registered_transport_cid(&transport_g2h);
+ net = current->nsproxy->net_ns;
+ cid = vsock_registered_transport_cid(&transport_g2h, net);
if (cid == VMADDR_CID_ANY)
- cid = vsock_registered_transport_cid(&transport_h2g);
+ cid = vsock_registered_transport_cid(&transport_h2g,
+ net);
if (cid == VMADDR_CID_ANY)
- cid = vsock_registered_transport_cid(&transport_local);
+ cid = vsock_registered_transport_cid(&transport_local,
+ net);
if (put_user(cid, p) != 0)
retval = -EFAULT;
@@ -2812,6 +3055,81 @@ static struct miscdevice vsock_device = {
.fops = &vsock_device_ops,
};
+int vsock_nl_dev_netns_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct net *net = genl_info_net(info);
+ struct net *assigned;
+ struct sk_buff *msg;
+ bool report;
+ void *hdr;
+ s32 id;
+ int err;
+
+ mutex_lock(&vsock_register_mutex);
+ if (!transport_g2h) {
+ mutex_unlock(&vsock_register_mutex);
+ NL_SET_ERR_MSG(info->extack,
+ "no guest-to-host transport is loaded");
+ return -ENODEV;
+ }
+
+ rcu_read_lock();
+ assigned = rcu_dereference(vsock_g2h_net);
+ report = !net_eq(net, assigned);
+ if (report) {
+ /* Hide the assignment on the same terms the CID is hidden:
+ * only a transport that honours namespace assignment keeps it
+ * from a namespace that cannot reach the device.
+ */
+ if (vsock_netns_assignable(transport_g2h) &&
+ !vsock_net_check_mode(net, assigned))
+ id = NETNSA_NSID_NOT_ASSIGNED;
+ else
+ id = peernet2id_alloc(net, assigned, GFP_ATOMIC);
+ }
+ rcu_read_unlock();
+ mutex_unlock(&vsock_register_mutex);
+
+ msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(msg, info);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_free;
+ }
+
+ if (report && nla_put_s32(msg, VSOCK_A_NETNS_ID, id)) {
+ err = -EMSGSIZE;
+ goto err_cancel;
+ }
+
+ genlmsg_end(msg, hdr);
+
+ return genlmsg_reply(msg, info);
+
+err_cancel:
+ genlmsg_cancel(msg, hdr);
+err_free:
+ nlmsg_free(msg);
+ return err;
+}
+
+int vsock_nl_dev_netns_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ int err = vsock_g2h_net_assign(genl_info_net(info));
+
+ if (err == -ENODEV)
+ NL_SET_ERR_MSG(info->extack,
+ "no guest-to-host transport is loaded");
+ else if (err == -EOPNOTSUPP)
+ NL_SET_ERR_MSG(info->extack,
+ "the loaded guest-to-host transport does not support namespace assignment");
+
+ return err;
+}
+
static int __vsock_net_mode_string(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos,
enum vsock_net_mode mode,
@@ -3015,6 +3333,7 @@ static __net_exit void vsock_pernet_exit(struct net *net)
static struct pernet_operations vsock_pernet_ops = {
.init = vsock_pernet_init,
+ .pre_exit = vsock_g2h_net_reset,
.exit = vsock_pernet_exit,
};
@@ -3050,10 +3369,18 @@ static int __init vsock_init(void)
goto err_unregister_sock;
}
+ err = genl_register_family(&vsock_nl_family);
+ if (err) {
+ pr_err("Cannot register vsock netlink family: %d\n", err);
+ goto err_unregister_pernet;
+ }
+
vsock_bpf_build_proto();
return 0;
+err_unregister_pernet:
+ unregister_pernet_subsys(&vsock_pernet_ops);
err_unregister_sock:
sock_unregister(AF_VSOCK);
err_unregister_proto:
@@ -3066,6 +3393,7 @@ static int __init vsock_init(void)
static void __exit vsock_exit(void)
{
+ genl_unregister_family(&vsock_nl_family);
misc_deregister(&vsock_device);
sock_unregister(AF_VSOCK);
proto_unregister(&vsock_proto);
@@ -3141,8 +3469,10 @@ void vsock_core_unregister(const struct vsock_transport *t)
if (transport_h2g == t)
transport_h2g = NULL;
- if (transport_g2h == t)
+ if (transport_g2h == t) {
transport_g2h = NULL;
+ rcu_assign_pointer(vsock_g2h_net, &init_net);
+ }
if (transport_dgram == t)
transport_dgram = NULL;
diff --git a/net/vmw_vsock/vsock_nl_gen.c b/net/vmw_vsock/vsock_nl_gen.c
new file mode 100644
index 000000000000..81db4c61d7bb
--- /dev/null
+++ b/net/vmw_vsock/vsock_nl_gen.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/vsock.yaml */
+/* YNL-GEN kernel source */
+/* To regenerate run: tools/net/ynl/ynl-regen.sh */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "vsock_nl_gen.h"
+
+#include <uapi/linux/vsock.h>
+
+/* Ops table for vsock */
+static const struct genl_split_ops vsock_nl_ops[] = {
+ {
+ .cmd = VSOCK_CMD_DEV_NETNS_SET,
+ .doit = vsock_nl_dev_netns_set_doit,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = VSOCK_CMD_DEV_NETNS_GET,
+ .doit = vsock_nl_dev_netns_get_doit,
+ .flags = GENL_CMD_CAP_DO,
+ },
+};
+
+struct genl_family vsock_nl_family __ro_after_init = {
+ .name = VSOCK_FAMILY_NAME,
+ .version = VSOCK_FAMILY_VERSION,
+ .netnsok = true,
+ .parallel_ops = true,
+ .module = THIS_MODULE,
+ .split_ops = vsock_nl_ops,
+ .n_split_ops = ARRAY_SIZE(vsock_nl_ops),
+};
diff --git a/net/vmw_vsock/vsock_nl_gen.h b/net/vmw_vsock/vsock_nl_gen.h
new file mode 100644
index 000000000000..c041195584a0
--- /dev/null
+++ b/net/vmw_vsock/vsock_nl_gen.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/* Documentation/netlink/specs/vsock.yaml */
+/* YNL-GEN kernel header */
+/* To regenerate run: tools/net/ynl/ynl-regen.sh */
+
+#ifndef _LINUX_VSOCK_GEN_H
+#define _LINUX_VSOCK_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/vsock.h>
+
+int vsock_nl_dev_netns_set_doit(struct sk_buff *skb, struct genl_info *info);
+int vsock_nl_dev_netns_get_doit(struct sk_buff *skb, struct genl_info *info);
+
+extern struct genl_family vsock_nl_family;
+
+#endif /* _LINUX_VSOCK_GEN_H */
diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps
index 1e746e25e2bc..fa51108c43d4 100644
--- a/tools/net/ynl/Makefile.deps
+++ b/tools/net/ynl/Makefile.deps
@@ -55,4 +55,5 @@ CFLAGS_tc:= $(call get_hdr_inc,__LINUX_RTNETLINK_H,rtnetlink.h) \
$(call get_hdr_inc,_TC_SKBEDIT_H,tc_act/tc_skbedit.h) \
$(call get_hdr_inc,_TC_TUNNEL_KEY_H,tc_act/tc_tunnel_key.h)
CFLAGS_tcp_metrics:=$(call get_hdr_inc,_LINUX_TCP_METRICS_H,tcp_metrics.h)
+CFLAGS_vsock:=$(call get_hdr_inc,_LINUX_VSOCK_H,vsock.h)
CFLAGS_wireguard:=$(call get_hdr_inc2,_LINUX_WIREGUARD_H,_WG_UAPI_WIREGUARD_H,wireguard.h)
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns
2026-09-22 1:18 ` [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns Bobby Eshleman
@ 2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-24 22:40 ` Bobby Eshleman
0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 19:21 UTC (permalink / raw)
To: bobbyeshleman
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 2 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] vsock_g2h_reset_unreachable() marks a socket
TCP_CLOSE/ECONNRESET but leaves it linked in vsock_connected_table…
- [Medium] vhost_vsock_set_cid()'s collision check against the G2H device
CID became namespace-conditional (`vsock_find_cid(vsock->net,…
- [Medium] vsock_g2h_reset_unreachable() invokes the new sleepable
`t->reset(vsk, NULL)` callback with only vsock_register_mutex held and…
- [Low] DISCARDED (resolved later in the series).
- [Low] DISCARDED (resolved later in the series).
- [Low] DISCARDED (blocked by the transport patch later in the series).
--
vsock: add a netlink command to assign the g2h device to a netns
This adds a vsock generic netlink family with VSOCK_CMD_DEV_NETNS_SET and
VSOCK_CMD_DEV_NETNS_GET, which move the guest-to-host vsock device into the
caller's network namespace and read that assignment back. Reachability of the
device, and visibility of its CID through the local-CID ioctl, bind() and
loopback transport selection, now depend on the namespace asking. Sockets that
can no longer reach the device after a move are reset, and the device returns
to init_net when its namespace is destroyed.
Link: https://lore.kernel.org/all/20200427142518.uwssa6dtasrp3bfc@steredhat/
A few questions below about the new reachability and reset paths.
> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> index fe43e85959583..f2d8e4e84f89b 100644
> --- a/Documentation/admin-guide/sysctl/net.rst
> +++ b/Documentation/admin-guide/sysctl/net.rst
> @@ -529,6 +529,29 @@ their hosts. The behavior of VSOCK sockets in a network namespace is determined
[ ... ]
> +Support is transport dependent. A G2H transport that does not implement the
> +move refuses the command with ``EOPNOTSUPP``; of the in-tree guest transports
> +only virtio-vsock implements it.
[Severity: Low]
At this commit, does any transport set netns_assign_allow and ->reset?
vsock_netns_assignable() requires both, and neither is set by any in-tree
transport here, so VSOCK_CMD_DEV_NETNS_SET can only return -ENODEV or
-EOPNOTSUPP while this text says virtio-vsock implements the move.
vsock_g2h_net_get() and vsock_maybe_set_connected() also have no callers
yet.
The following patch in the series, "vsock/virtio: support guest device
network namespace", adds .netns_assign_allow = true and
.reset = virtio_transport_reset to virtio_transport and wires up both
helpers, so the text becomes accurate at the end of the series. Would it
read better to add this paragraph in that patch?
[ ... ]
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index abed1fbcf66cc..badc064964b31 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -828,9 +828,11 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
> return -EINVAL;
>
> /* Refuse if CID is assigned to the guest->host transport (i.e. nested
> - * VM), to make the loopback work.
> + * VM), to make the loopback work. Only when that device is reachable
> + * from this VM's namespace, which is the same test the guest CID
> + * collision check below applies.
> */
> - if (vsock_find_cid(guest_cid))
> + if (vsock_find_cid(vsock->net, guest_cid))
> return -EADDRINUSE;
[Severity: Medium]
This collision check is now conditional on the g2h device being reachable
from vsock->net, but that condition can change after the CID has been
accepted. Is it re-checked anywhere?
With the device assigned to a local mode namespace A, a VMM in namespace B
sees vsock_find_cid(B, guest_cid) return false, because
__vsock_registered_transport_cid() reports VMADDR_CID_ANY for the g2h slot
when !vsock_g2h_net_reachable(B). So a CID that previously got -EADDRINUSE
is now installed.
The device can become reachable from B afterwards, either by another
VSOCK_CMD_DEV_NETNS_SET or automatically when namespace A is deleted and
vsock_g2h_net_reset() moves the device back to init_net.
vsock_g2h_net_assign() only walks vsock_connected_table, so registered
vhost guest CIDs are never revisited, and then:
- vsock_use_local_transport(B, cid) computes a non-ANY g2h CID and
returns true, so vsock_assign_transport() picks transport_local ahead
of transport_h2g and connects intended for the nested guest land on
vsock_loopback
- vsock_find_cid(B, cid) starts accepting bind() on that CID
Does the nested guest become unreachable from that namespace at that
point, which is what the pre-patch unconditional check prevented?
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 370fcd3ddabc2..dbf1a6aa367fe 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -190,6 +192,16 @@ struct vsock_transport {
>
> /* Zero-copy. */
> bool (*msgzerocopy_allow)(void);
> +
> + /* True if the G2H transport honours VSOCK_CMD_DEV_NETNS_SET. A
> + * transport that sets this must also implement reset.
> + */
> + bool netns_assign_allow;
> +
> + /* Send a reset to @vsk's peer. @skb is the packet being replied to, or
> + * NULL when the reset is not a reply. May sleep.
> + */
> + int (*reset)(struct vsock_sock *vsk, struct sk_buff *skb);
> };
[Severity: Medium]
Which locks may a ->reset() implementation assume are held?
vsock_g2h_reset_unreachable() calls t->reset(vsk, NULL) with only
vsock_register_mutex held and no socket lock. Every existing caller of
virtio_transport_reset(), the implementation wired up later in the series
(close work and close timeout, the recv "destroy" paths, shutdown), runs
under lock_sock() or lock_sock_nested(), which is what serialises
vsk->trans and the addresses the reset reads against close/release and
recv processing.
Taking lock_sock() around the callback does not look available from this
call site, since vsock_connect() holds lock_sock(sk) and then acquires
vsock_register_mutex inside vsock_assign_transport(), making sock lock ->
register mutex the established order.
The sweep also stores sk->sk_state and sk->sk_err under vsock_table_lock
only, so a concurrent lock_sock() holder can overwrite sk_state and the
reset is lost.
Separately, the loop performs one sleeping send per collected socket while
holding vsock_register_mutex, which blocks socket creation, connect and
the CID getters for the duration. Can the resets run outside that mutex,
and can this comment state the locking context the callback is invoked in?
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 95a435aef512b..9938dd5010192 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
[ ... ]
> @@ -654,6 +716,13 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> goto err;
> }
>
> + if (new_transport && new_transport == transport_g2h &&
> + vsock_netns_assignable(new_transport) &&
> + !vsock_g2h_net_reachable(sock_net(sk))) {
> + ret = -ENETUNREACH;
> + goto err;
> + }
> +
[Severity: Low]
This gate sits after the same-transport shortcut a few lines above it:
if (vsk->transport && vsk->transport == new_transport) {
ret = 0;
goto err;
}
so a socket that already holds transport_g2h, for instance one left
assigned by an earlier failed connect, never reaches the -ENETUNREACH
check.
At the end of the series nothing is emitted from an excluded namespace,
because vsock_connect() calls transport->stream_allow() right after
vsock_assign_transport() and the following patch makes
virtio_transport_stream_allow() return vsock_g2h_net_reachable(...). Would
it still be clearer to place this check before the shortcut?
> @@ -742,6 +812,173 @@ bool vsock_find_cid(unsigned int cid)
> }
> EXPORT_SYMBOL_GPL(vsock_find_cid);
[ ... ]
> +/* Move @vsk to TCP_ESTABLISHED and into the connected table, unless the device
> + * has moved to a namespace @vsk cannot reach. Returns false without doing
> + * either in that case.
> + *
> + * The reset sweep walks the same table under the same lock, so an assign
> + * cannot land between the check and the insert: either the sweep finds @vsk
> + * and resets it, or @vsk is never added.
> + */
> +bool vsock_maybe_set_connected(struct vsock_sock *vsk)
> +{
> + struct list_head *list;
> + bool reachable;
> +
> + list = vsock_connected_sockets(&vsk->remote_addr, &vsk->local_addr);
> +
> + spin_lock_bh(&vsock_table_lock);
> + reachable = vsock_g2h_reachable_sk(vsk);
> + if (reachable) {
> + sk_vsock(vsk)->sk_state = TCP_ESTABLISHED;
> + __vsock_insert_connected(list, vsk);
> + }
> + spin_unlock_bh(&vsock_table_lock);
> +
> + return reachable;
> +}
> +EXPORT_SYMBOL_GPL(vsock_maybe_set_connected);
[Severity: Low]
The invariant described here needs every connected-table insertion of an
assignable transport to go through this helper, but nothing calls it in
this commit.
The next patch, "vsock/virtio: support guest device network namespace",
adds the calls in virtio_transport_recv_connecting() and
virtio_transport_recv_listen(), so the invariant holds from there on. Is
the split intentional, or should the helper land with its users?
> +/* Reset every connected socket of @t that can no longer reach the g2h device,
> + * and let the transport tell each peer.
> + */
> +static void vsock_g2h_reset_unreachable(const struct vsock_transport *t)
> +{
[ ... ]
> + if (vsk->transport != t ||
> + sk->sk_state == TCP_CLOSE ||
> + vsock_g2h_reachable_sk(vsk))
> + continue;
> +
> + sk->sk_state = TCP_CLOSE;
> + sk->sk_err = ECONNRESET;
> + sk_error_report(sk);
> +
> + sock_hold(sk);
> + list_add_tail(&vsk->pending_reset, &reset_list);
> + }
> + }
> +
> + spin_unlock_bh(&vsock_table_lock);
[Severity: High]
The sweep marks the socket TCP_CLOSE with ECONNRESET but leaves it linked
in vsock_connected_table and keeps vsk->transport assigned. Can that
socket then be inserted into the table a second time?
A connector blocked in vsock_connect() wakes up, leaves the wait loop and
runs:
net/vmw_vsock/af_vsock.c:vsock_connect() {
...
err = sock_error(sk);
if (err) {
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
}
...
}
so connect() is retryable while the socket is still a member of the
connected table. On the retry vsock_assign_transport() takes the shortcut:
if (vsk->transport && vsk->transport == new_transport) {
ret = 0;
goto err;
}
which skips vsk->transport->release() and vsock_deassign_transport(), so
vsock_remove_connected() is never reached. When the handshake completes
again, virtio_transport_recv_connecting() reaches:
net/vmw_vsock/af_vsock.c:__vsock_insert_connected() {
sock_hold(&vsk->sk);
list_add(&vsk->connected_table, list);
}
on a node that is already linked. Does this corrupt the connected hash
bucket and leak the extra sock_hold()? A self-referential bucket would
make later list_for_each_entry() walks under vsock_table_lock, including
this sweep, vsock_find_connected_socket() and
vsock_for_each_connected_socket(), never terminate, and with
CONFIG_DEBUG_LIST the second list_add() trips the corruption check.
Reaching the reachable-again state does not need a second netlink call:
vsock_g2h_net_reset() moves the device back to init_net when the assigned
namespace is deleted.
Should the sweep also call vsock_remove_connected() on the sockets it
resets, or otherwise deassign the transport, so a retry cannot re-insert
an already linked node?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-vsock-guest-ns-v2-0-693bd78fde9e%40meta.com
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns
2026-09-23 19:21 ` netdev-bot+sashiko
@ 2026-09-24 22:40 ` Bobby Eshleman
0 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-24 22:40 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
On Wed, Sep 23, 2026 at 07:21:01PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 2 · Low: 3
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] vsock_g2h_reset_unreachable() marks a socket
> TCP_CLOSE/ECONNRESET but leaves it linked in vsock_connected_table…
> - [Medium] vhost_vsock_set_cid()'s collision check against the G2H device
> CID became namespace-conditional (`vsock_find_cid(vsock->net,…
> - [Medium] vsock_g2h_reset_unreachable() invokes the new sleepable
> `t->reset(vsk, NULL)` callback with only vsock_register_mutex held and…
> - [Low] DISCARDED (resolved later in the series).
> - [Low] DISCARDED (resolved later in the series).
> - [Low] DISCARDED (blocked by the transport patch later in the series).
>
> --
>
> vsock: add a netlink command to assign the g2h device to a netns
>
> This adds a vsock generic netlink family with VSOCK_CMD_DEV_NETNS_SET and
> VSOCK_CMD_DEV_NETNS_GET, which move the guest-to-host vsock device into the
> caller's network namespace and read that assignment back. Reachability of the
> device, and visibility of its CID through the local-CID ioctl, bind() and
> loopback transport selection, now depend on the namespace asking. Sockets that
> can no longer reach the device after a move are reset, and the device returns
> to init_net when its namespace is destroyed.
>
> Link: https://lore.kernel.org/all/20200427142518.uwssa6dtasrp3bfc@steredhat/
>
> A few questions below about the new reachability and reset paths.
>
> > diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> > index fe43e85959583..f2d8e4e84f89b 100644
> > --- a/Documentation/admin-guide/sysctl/net.rst
> > +++ b/Documentation/admin-guide/sysctl/net.rst
> > @@ -529,6 +529,29 @@ their hosts. The behavior of VSOCK sockets in a network namespace is determined
>
> [ ... ]
>
> > +Support is transport dependent. A G2H transport that does not implement the
> > +move refuses the command with ``EOPNOTSUPP``; of the in-tree guest transports
> > +only virtio-vsock implements it.
>
> [Severity: Low]
>
> At this commit, does any transport set netns_assign_allow and ->reset?
>
> vsock_netns_assignable() requires both, and neither is set by any in-tree
> transport here, so VSOCK_CMD_DEV_NETNS_SET can only return -ENODEV or
> -EOPNOTSUPP while this text says virtio-vsock implements the move.
> vsock_g2h_net_get() and vsock_maybe_set_connected() also have no callers
> yet.
>
> The following patch in the series, "vsock/virtio: support guest device
> network namespace", adds .netns_assign_allow = true and
> .reset = virtio_transport_reset to virtio_transport and wires up both
> helpers, so the text becomes accurate at the end of the series. Would it
> read better to add this paragraph in that patch?
I think this is okay. We add support and then online the transports
later.
>
> [ ... ]
>
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index abed1fbcf66cc..badc064964b31 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -828,9 +828,11 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
> > return -EINVAL;
> >
> > /* Refuse if CID is assigned to the guest->host transport (i.e. nested
> > - * VM), to make the loopback work.
> > + * VM), to make the loopback work. Only when that device is reachable
> > + * from this VM's namespace, which is the same test the guest CID
> > + * collision check below applies.
> > */
> > - if (vsock_find_cid(guest_cid))
> > + if (vsock_find_cid(vsock->net, guest_cid))
> > return -EADDRINUSE;
>
> [Severity: Medium]
>
> This collision check is now conditional on the g2h device being reachable
> from vsock->net, but that condition can change after the CID has been
> accepted. Is it re-checked anywhere?
>
> With the device assigned to a local mode namespace A, a VMM in namespace B
> sees vsock_find_cid(B, guest_cid) return false, because
> __vsock_registered_transport_cid() reports VMADDR_CID_ANY for the g2h slot
> when !vsock_g2h_net_reachable(B). So a CID that previously got -EADDRINUSE
> is now installed.
>
> The device can become reachable from B afterwards, either by another
> VSOCK_CMD_DEV_NETNS_SET or automatically when namespace A is deleted and
> vsock_g2h_net_reset() moves the device back to init_net.
> vsock_g2h_net_assign() only walks vsock_connected_table, so registered
> vhost guest CIDs are never revisited, and then:
>
> - vsock_use_local_transport(B, cid) computes a non-ANY g2h CID and
> returns true, so vsock_assign_transport() picks transport_local ahead
> of transport_h2g and connects intended for the nested guest land on
> vsock_loopback
> - vsock_find_cid(B, cid) starts accepting bind() on that CID
>
> Does the nested guest become unreachable from that namespace at that
> point, which is what the pre-patch unconditional check prevented?
Sashiko doesn't frame this problem that well here IMHO, but it is a real
issue. The issue is about CID collision between g2h and h2g,
particularly if the g2h is in another namespace. In theory we should
allow a CID to be allocated in a local namespace if it is free there.
The issue is then if the user moves that device into a namespace where
that CID is already in-use.
We can reject VSOCK_CMD_DEV_NETNS_SET to handle that case, but the
relocation to init_net upon namespace deletion is more complicated.
In tcp/ip land when a namespace is torn down and a device is
moved to the init_net, it loses its IP address. There is no address
collision. We can't really do that here.
In our case, if the g2h CID 10 is in some other namespace, in theory
don't want to reject vhost trying to allocate CID 10 because that leaks
information about another namespace.
I also don't think we want to reject namespace deletion in this case
either, because then the namespace lifetime will be at the mercy of the
init_net's vmm process that holds the CID.
Right now I'm thinking that just preventing vhost from allocating CIDs
of any g2h (even if in another local namespace) might be the best
solution at hand, even if it exposes a leak.
I also considered the possibility of the g2h device falling back to some
alias_cid when there is contention due to a namespace move... but that
is a can of worms.
Do you have any thoughts on this Stefano?
>
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 370fcd3ddabc2..dbf1a6aa367fe 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -190,6 +192,16 @@ struct vsock_transport {
> >
> > /* Zero-copy. */
> > bool (*msgzerocopy_allow)(void);
> > +
> > + /* True if the G2H transport honours VSOCK_CMD_DEV_NETNS_SET. A
> > + * transport that sets this must also implement reset.
> > + */
> > + bool netns_assign_allow;
> > +
> > + /* Send a reset to @vsk's peer. @skb is the packet being replied to, or
> > + * NULL when the reset is not a reply. May sleep.
> > + */
> > + int (*reset)(struct vsock_sock *vsk, struct sk_buff *skb);
> > };
>
> [Severity: Medium]
>
> Which locks may a ->reset() implementation assume are held?
>
> vsock_g2h_reset_unreachable() calls t->reset(vsk, NULL) with only
> vsock_register_mutex held and no socket lock. Every existing caller of
> virtio_transport_reset(), the implementation wired up later in the series
> (close work and close timeout, the recv "destroy" paths, shutdown), runs
> under lock_sock() or lock_sock_nested(), which is what serialises
> vsk->trans and the addresses the reset reads against close/release and
> recv processing.
>
> Taking lock_sock() around the callback does not look available from this
> call site, since vsock_connect() holds lock_sock(sk) and then acquires
> vsock_register_mutex inside vsock_assign_transport(), making sock lock ->
> register mutex the established order.
>
> The sweep also stores sk->sk_state and sk->sk_err under vsock_table_lock
> only, so a concurrent lock_sock() holder can overwrite sk_state and the
> reset is lost.
>
> Separately, the loop performs one sleeping send per collected socket while
> holding vsock_register_mutex, which blocks socket creation, connect and
> the CID getters for the duration. Can the resets run outside that mutex,
> and can this comment state the locking context the callback is invoked in?
>
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index 95a435aef512b..9938dd5010192 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
>
> [ ... ]
>
> > @@ -654,6 +716,13 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> > goto err;
> > }
> >
> > + if (new_transport && new_transport == transport_g2h &&
> > + vsock_netns_assignable(new_transport) &&
> > + !vsock_g2h_net_reachable(sock_net(sk))) {
> > + ret = -ENETUNREACH;
> > + goto err;
> > + }
> > +
>
> [Severity: Low]
>
> This gate sits after the same-transport shortcut a few lines above it:
>
> if (vsk->transport && vsk->transport == new_transport) {
> ret = 0;
> goto err;
> }
>
> so a socket that already holds transport_g2h, for instance one left
> assigned by an earlier failed connect, never reaches the -ENETUNREACH
> check.
>
> At the end of the series nothing is emitted from an excluded namespace,
> because vsock_connect() calls transport->stream_allow() right after
> vsock_assign_transport() and the following patch makes
> virtio_transport_stream_allow() return vsock_g2h_net_reachable(...). Would
> it still be clearer to place this check before the shortcut?
>
> > @@ -742,6 +812,173 @@ bool vsock_find_cid(unsigned int cid)
> > }
> > EXPORT_SYMBOL_GPL(vsock_find_cid);
>
> [ ... ]
>
> > +/* Move @vsk to TCP_ESTABLISHED and into the connected table, unless the device
> > + * has moved to a namespace @vsk cannot reach. Returns false without doing
> > + * either in that case.
> > + *
> > + * The reset sweep walks the same table under the same lock, so an assign
> > + * cannot land between the check and the insert: either the sweep finds @vsk
> > + * and resets it, or @vsk is never added.
> > + */
> > +bool vsock_maybe_set_connected(struct vsock_sock *vsk)
> > +{
> > + struct list_head *list;
> > + bool reachable;
> > +
> > + list = vsock_connected_sockets(&vsk->remote_addr, &vsk->local_addr);
> > +
> > + spin_lock_bh(&vsock_table_lock);
> > + reachable = vsock_g2h_reachable_sk(vsk);
> > + if (reachable) {
> > + sk_vsock(vsk)->sk_state = TCP_ESTABLISHED;
> > + __vsock_insert_connected(list, vsk);
> > + }
> > + spin_unlock_bh(&vsock_table_lock);
> > +
> > + return reachable;
> > +}
> > +EXPORT_SYMBOL_GPL(vsock_maybe_set_connected);
>
> [Severity: Low]
>
> The invariant described here needs every connected-table insertion of an
> assignable transport to go through this helper, but nothing calls it in
> this commit.
>
> The next patch, "vsock/virtio: support guest device network namespace",
> adds the calls in virtio_transport_recv_connecting() and
> virtio_transport_recv_listen(), so the invariant holds from there on. Is
> the split intentional, or should the helper land with its users?
>
> > +/* Reset every connected socket of @t that can no longer reach the g2h device,
> > + * and let the transport tell each peer.
> > + */
> > +static void vsock_g2h_reset_unreachable(const struct vsock_transport *t)
> > +{
>
> [ ... ]
>
> > + if (vsk->transport != t ||
> > + sk->sk_state == TCP_CLOSE ||
> > + vsock_g2h_reachable_sk(vsk))
> > + continue;
> > +
> > + sk->sk_state = TCP_CLOSE;
> > + sk->sk_err = ECONNRESET;
> > + sk_error_report(sk);
> > +
> > + sock_hold(sk);
> > + list_add_tail(&vsk->pending_reset, &reset_list);
> > + }
> > + }
> > +
> > + spin_unlock_bh(&vsock_table_lock);
>
> [Severity: High]
>
> The sweep marks the socket TCP_CLOSE with ECONNRESET but leaves it linked
> in vsock_connected_table and keeps vsk->transport assigned. Can that
> socket then be inserted into the table a second time?
>
> A connector blocked in vsock_connect() wakes up, leaves the wait loop and
> runs:
>
> net/vmw_vsock/af_vsock.c:vsock_connect() {
> ...
> err = sock_error(sk);
> if (err) {
> sk->sk_state = TCP_CLOSE;
> sock->state = SS_UNCONNECTED;
> }
> ...
> }
>
> so connect() is retryable while the socket is still a member of the
> connected table. On the retry vsock_assign_transport() takes the shortcut:
>
> if (vsk->transport && vsk->transport == new_transport) {
> ret = 0;
> goto err;
> }
>
> which skips vsk->transport->release() and vsock_deassign_transport(), so
> vsock_remove_connected() is never reached. When the handshake completes
> again, virtio_transport_recv_connecting() reaches:
>
> net/vmw_vsock/af_vsock.c:__vsock_insert_connected() {
> sock_hold(&vsk->sk);
> list_add(&vsk->connected_table, list);
> }
>
> on a node that is already linked. Does this corrupt the connected hash
> bucket and leak the extra sock_hold()? A self-referential bucket would
> make later list_for_each_entry() walks under vsock_table_lock, including
> this sweep, vsock_find_connected_socket() and
> vsock_for_each_connected_socket(), never terminate, and with
> CONFIG_DEBUG_LIST the second list_add() trips the corruption check.
>
> Reaching the reachable-again state does not need a second netlink call:
> vsock_g2h_net_reset() moves the device back to init_net when the assigned
> namespace is deleted.
>
> Should the sweep also call vsock_remove_connected() on the sockets it
> resets, or otherwise deassign the transport, so a retry cannot re-insert
> an already linked node?
>
> [ ... ]
Can confirm, this is valid. Will fix.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
` (2 preceding siblings ...)
2026-09-22 1:18 ` [PATCH net-next v2 3/6] vsock: add a netlink command to assign the g2h device to a netns Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-22 1:18 ` [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock " Bobby Eshleman
2026-09-22 1:18 ` [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks Bobby Eshleman
5 siblings, 1 reply; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
From: Bobby Eshleman <bobbyeshleman@meta.com>
virtio-vsock did not have namespace support (the device was always
accessible to any global namespace).
Make the virtio-vsock device assignable to a namespace and initialize it
to init_net. Because virtio-vsock and init_net are both hardcoded to
global mode, nothing changes until the assign command is issued.
When the device's local-mode namespace is being destroyed, received
packets are reset until a new valid namespace has been assigned and/or
automatically returned to, and the next RX batch begins (in
virtio_transport_rx_work). They are reset rather than dropped because
vsock does not retransmit, so a silent drop would leave the host waiting
for a timeout, and a connection request arriving in that window has no
socket whose teardown would tell it otherwise. This requires making
virtio_transport_reset_no_sock() available outside of the common code.
When a device is assigned to a namespace, every already established
vsock socket that is no longer able to reach the device is forcibly
reset. For that reason, adding new sockets to the connected table must
be performed atomically with regards to namespace assignment. This
ensures that when the socket is added to the connected table that it
actually passes the new reachability conditions set by ns assignment. If
it wins the race to the table and does NOT pass the reachability tests,
then it will be reset. This is the purpose of the new helper
'vsock_maybe_set_connected()'.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- Export virtio_transport_reset(), wire it to the new .reset op (Stefano)
- netns_assign_allow is now a bool (Stefano)
- Pass NULL, not &init_net, in virtio_transport_rx_work(), and
comment why (Stefano)
- Drop the if (net) guard around put_net() (Stefano)
- Drop the comments at the vsock_maybe_set_connected() call sites,
the commit msg seems sufficient
---
include/linux/virtio_vsock.h | 3 +++
net/vmw_vsock/virtio_transport.c | 24 ++++++++++++++++++------
net/vmw_vsock/virtio_transport_common.c | 27 ++++++++++++++++++---------
3 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index f91704731057..5d15b6d6bdf7 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -286,6 +286,9 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
void virtio_transport_deliver_tap_pkt(struct sk_buff *skb);
+int virtio_transport_reset(struct vsock_sock *vsk, struct sk_buff *skb);
+int virtio_transport_reset_no_sock(const struct virtio_transport *t,
+ struct sk_buff *skb, struct net *net);
int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head *list);
int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t read_actor);
int virtio_transport_notify_set_rcvlowat(struct vsock_sock *vsk, int val);
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa..5ad93af4bd2b 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -542,7 +542,7 @@ static bool virtio_transport_msgzerocopy_allow(void)
bool virtio_transport_stream_allow(struct vsock_sock *vsk, u32 cid, u32 port)
{
- return vsock_net_mode_global(vsk);
+ return vsock_g2h_net_reachable(sock_net(sk_vsock(vsk)));
}
static bool virtio_transport_seqpacket_allow(struct vsock_sock *vsk,
@@ -587,6 +587,8 @@ static struct virtio_transport virtio_transport = {
.seqpacket_has_data = virtio_transport_seqpacket_has_data,
.msgzerocopy_allow = virtio_transport_msgzerocopy_allow,
+ .netns_assign_allow = true,
+ .reset = virtio_transport_reset,
.notify_poll_in = virtio_transport_notify_poll_in,
.notify_poll_out = virtio_transport_notify_poll_out,
@@ -616,7 +618,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
struct virtio_vsock *vsock;
bool seqpacket_allow;
- if (!vsock_net_mode_global(vsk))
+ if (!vsock_g2h_net_reachable(sock_net(sk_vsock(vsk))))
return false;
seqpacket_allow = false;
@@ -633,7 +635,11 @@ static void virtio_transport_rx_work(struct work_struct *work)
{
struct virtio_vsock *vsock =
container_of(work, struct virtio_vsock, rx_work);
+ struct virtio_transport *t = &virtio_transport;
struct virtqueue *vq;
+ struct net *net;
+
+ net = vsock_g2h_net_get();
mutex_lock(&vsock->rx_lock);
@@ -682,10 +688,14 @@ static void virtio_transport_rx_work(struct work_struct *work)
virtio_transport_deliver_tap_pkt(skb);
- /* Force virtio-transport into global mode since it
- * does not yet support local-mode namespacing.
- */
- virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
+ /* The virtio send path does not use @net. */
+ if (unlikely(!net)) {
+ virtio_transport_reset_no_sock(t, skb, NULL);
+ kfree_skb(skb);
+ continue;
+ }
+
+ virtio_transport_recv_pkt(t, skb, net);
}
} while (!virtqueue_enable_cb(vq));
@@ -694,6 +704,8 @@ static void virtio_transport_rx_work(struct work_struct *work)
virtio_vsock_rx_fill(vsock);
out_nofill:
mutex_unlock(&vsock->rx_lock);
+
+ put_net(net);
}
static int virtio_vsock_vqs_init(struct virtio_vsock *vsock)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f225f53ed4ba..c24049b2a386 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1291,8 +1291,7 @@ ssize_t virtio_transport_unsent_bytes(struct vsock_sock *vsk)
}
EXPORT_SYMBOL_GPL(virtio_transport_unsent_bytes);
-static int virtio_transport_reset(struct vsock_sock *vsk,
- struct sk_buff *skb)
+int virtio_transport_reset(struct vsock_sock *vsk, struct sk_buff *skb)
{
struct virtio_vsock_pkt_info info = {
.op = VIRTIO_VSOCK_OP_RST,
@@ -1307,6 +1306,7 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
return virtio_transport_send_pkt_info(vsk, &info);
}
+EXPORT_SYMBOL_GPL(virtio_transport_reset);
/* Normally packets are associated with a socket. There may be no socket if an
* attempt was made to connect to a socket that does not exist.
@@ -1315,8 +1315,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
* loopback, this is the namespace of the socket. For vhost, this is the
* namespace of the VM (i.e., vhost_vsock).
*/
-static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
- struct sk_buff *skb, struct net *net)
+int virtio_transport_reset_no_sock(const struct virtio_transport *t,
+ struct sk_buff *skb, struct net *net)
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct virtio_vsock_pkt_info info = {
@@ -1355,6 +1355,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
return t->send_pkt(reply, net);
}
+EXPORT_SYMBOL_GPL(virtio_transport_reset_no_sock);
/* This function should be called with sk_lock held and SOCK_DONE set */
static void virtio_transport_remove_sock(struct vsock_sock *vsk)
@@ -1478,9 +1479,13 @@ virtio_transport_recv_connecting(struct sock *sk,
switch (le16_to_cpu(hdr->op)) {
case VIRTIO_VSOCK_OP_RESPONSE:
- sk->sk_state = TCP_ESTABLISHED;
+ if (!vsock_maybe_set_connected(vsk)) {
+ skerr = ECONNRESET;
+ err = -ENETUNREACH;
+ goto destroy;
+ }
+
sk->sk_socket->state = SS_CONNECTED;
- vsock_insert_connected(vsk);
sk->sk_state_change(sk);
break;
case VIRTIO_VSOCK_OP_INVALID:
@@ -1736,8 +1741,6 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
lock_sock_nested(child, SINGLE_DEPTH_NESTING);
- child->sk_state = TCP_ESTABLISHED;
-
vchild = vsock_sk(child);
vsock_addr_init(&vchild->local_addr, le64_to_cpu(hdr->dst_cid),
le32_to_cpu(hdr->dst_port));
@@ -1758,7 +1761,13 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
if (virtio_transport_space_update(child, skb))
child->sk_write_space(child);
- vsock_insert_connected(vchild);
+ if (!vsock_maybe_set_connected(vchild)) {
+ release_sock(child);
+ virtio_transport_reset_no_sock(t, skb, sock_net(sk));
+ sock_put(child);
+ return -ENETUNREACH;
+ }
+
vsock_enqueue_accept(sk, child);
virtio_transport_send_response(vchild, skb);
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace
2026-09-22 1:18 ` [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace Bobby Eshleman
@ 2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-24 1:16 ` Bobby Eshleman
0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 19:21 UTC (permalink / raw)
To: bobbyeshleman
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] virtio_transport_rx_work() samples the g2h namespace once per
work invocation (`net = vsock_g2h_net_get()` at…
- [Medium] By setting `.netns_assign_allow = true` and `.reset =
virtio_transport_reset` (net/vmw_vsock/virtio_transport.c:590-591)…
- [Low] virtio_transport_reset_no_sock() is promoted to an exported
cross-module API (declared in include/linux/virtio_vsock.h,…
--
vsock/virtio: support guest device network namespace
This makes the virtio-vsock guest device assignable to a network namespace
and initializes it to init_net. The RX work now looks up the currently
assigned namespace and resets received packets while no valid namespace is
available, and new connected sockets are inserted via the new
vsock_maybe_set_connected() helper so insertion is atomic with respect to
namespace assignment.
A few questions below.
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index f91704731057e..5d15b6d6bdf7f 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -286,6 +286,9 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> void virtio_transport_deliver_tap_pkt(struct sk_buff *skb);
> +int virtio_transport_reset(struct vsock_sock *vsk, struct sk_buff *skb);
> +int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net);
> int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head *list);
> int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t read_actor);
> int virtio_transport_notify_set_rcvlowat(struct vsock_sock *vsk, int val);
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 4f9aa9c4c3aa5..5ad93af4bd2b1 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
[ ... ]
> @@ -587,6 +587,8 @@ static struct virtio_transport virtio_transport = {
> .seqpacket_has_data = virtio_transport_seqpacket_has_data,
>
> .msgzerocopy_allow = virtio_transport_msgzerocopy_allow,
> + .netns_assign_allow = true,
> + .reset = virtio_transport_reset,
[Severity: Medium]
Setting .netns_assign_allow = true makes vsock_netns_assignable() true for
the g2h transport, which activates the reset sweep in
vsock_g2h_reset_unreachable(). That sweep calls t->reset() while holding
only vsock_register_mutex and a socket reference, never lock_sock(sk):
net/vmw_vsock/af_vsock.c:vsock_g2h_reset_unreachable() {
...
sk->sk_state = TCP_CLOSE;
sk->sk_err = ECONNRESET;
sk_error_report(sk);
...
list_for_each_entry_safe(vsk, tmp, &reset_list, pending_reset) {
list_del_init(&vsk->pending_reset);
t->reset(vsk, NULL);
...
}
virtio_transport_reset() -> virtio_transport_send_pkt_info() addresses the
RST straight out of the socket:
if (!info->remote_cid) {
dst_cid = vsk->remote_addr.svm_cid;
dst_port = vsk->remote_addr.svm_port;
Those fields are normally only stable under lock_sock(sk).
The sk_error_report() above wakes a blocking connect(), which then runs:
net/vmw_vsock/af_vsock.c:vsock_connect() {
err = sock_error(sk);
if (err) {
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
}
}
and returns -ECONNRESET. If userspace retries connect() immediately, the
memcpy(&vsk->remote_addr, remote_addr,
sizeof(vsk->remote_addr));
in vsock_connect() runs before vsock_assign_transport() blocks on
vsock_register_mutex, so it is not serialized against the sweep that is
still walking reset_list.
Can the sweep then read the freshly overwritten remote_addr and send the
RST to the new destination, so the actual peer of the connection that was
just torn down never gets a reset and waits for a timeout instead?
>
> .notify_poll_in = virtio_transport_notify_poll_in,
> .notify_poll_out = virtio_transport_notify_poll_out,
[ ... ]
> @@ -633,7 +635,11 @@ static void virtio_transport_rx_work(struct work_struct *work)
> {
> struct virtio_vsock *vsock =
> container_of(work, struct virtio_vsock, rx_work);
> + struct virtio_transport *t = &virtio_transport;
> struct virtqueue *vq;
> + struct net *net;
> +
> + net = vsock_g2h_net_get();
>
> mutex_lock(&vsock->rx_lock);
>
> @@ -682,10 +688,14 @@ static void virtio_transport_rx_work(struct work_struct *work)
>
> virtio_transport_deliver_tap_pkt(skb);
>
> - /* Force virtio-transport into global mode since it
> - * does not yet support local-mode namespacing.
> - */
> - virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
> + /* The virtio send path does not use @net. */
> + if (unlikely(!net)) {
> + virtio_transport_reset_no_sock(t, skb, NULL);
> + kfree_skb(skb);
> + continue;
> + }
> +
> + virtio_transport_recv_pkt(t, skb, net);
[Severity: Medium]
The namespace is sampled once per work invocation and then pinned with a
hard reference for the whole drain loop:
net = vsock_g2h_net_get();
...
do {
...
virtio_transport_recv_pkt(t, skb, net);
} while (!virtqueue_enable_cb(vq));
Meanwhile the assign path publishes the new namespace and then sweeps the
connected table:
net/vmw_vsock/af_vsock.c:vsock_g2h_net_assign() {
...
rcu_assign_pointer(vsock_g2h_net, net);
vsock_g2h_reset_unreachable(transport_g2h);
...
}
As soon as the new pointer is published, a connect() from the newly
assigned namespace is permitted, because vsock_assign_transport() consults
vsock_g2h_net_reachable() which already reads it. If the host RESPONSE for
that connection is drained by the batch still holding the previous
namespace, vsock_find_connected_socket_net(&src, &dst, net) does not find
the socket (vsock_net_check_mode() of the local-mode new net against the
old net is false), so virtio_transport_recv_pkt() falls into
(void)virtio_transport_reset_no_sock(t, skb, net);
and the just-established connection is reset. Is that acceptable for the
remainder of an in-flight batch, or should the namespace be re-read per
packet?
In the other direction, between the rcu_assign_pointer() above and the
per-socket store in vsock_g2h_reset_unreachable():
sk->sk_state = TCP_CLOSE;
sk->sk_err = ECONNRESET;
can the in-flight batch still resolve packets against the old namespace and
deliver payload plus credit updates into sockets that the device no longer
belongs to?
The commit message describes the batch granularity only for the dying
namespace case:
"received packets are reset until a new valid namespace has been assigned
and/or automatically returned to, and the next RX batch begins"
Should it also describe what happens to packets of the newly assigned
namespace while an older batch is still draining?
> }
> } while (!virtqueue_enable_cb(vq));
>
[ ... ]
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index f225f53ed4bab..c24049b2a3866 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
[ ... ]
> @@ -1315,8 +1315,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> * loopback, this is the namespace of the socket. For vhost, this is the
> * namespace of the VM (i.e., vhost_vsock).
> */
> -static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> - struct sk_buff *skb, struct net *net)
> +int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net)
[Severity: Low]
This isn't a bug, but the comment above still enumerates only the loopback
and vhost meanings of @net, while the helper is now exported via
include/linux/virtio_vsock.h and the new caller in
virtio_transport_rx_work() passes NULL. Whether NULL is fine depends on
the ->send_pkt behind @t: virtio_transport_send_pkt() ignores @net, and
vhost_transport_send_pkt() passes it to vhost_vsock_get() ->
vsock_net_check_mode(), which treats NULL as VSOCK_NET_MODE_GLOBAL. Could
the comment be extended to say that NULL is permitted and what it means?
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct virtio_vsock_pkt_info info = {
> @@ -1355,6 +1355,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>
> return t->send_pkt(reply, net);
> }
> +EXPORT_SYMBOL_GPL(virtio_transport_reset_no_sock);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-vsock-guest-ns-v2-0-693bd78fde9e%40meta.com
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace
2026-09-23 19:21 ` netdev-bot+sashiko
@ 2026-09-24 1:16 ` Bobby Eshleman
0 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-24 1:16 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
On Wed, Sep 23, 2026 at 07:21:03PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] virtio_transport_rx_work() samples the g2h namespace once per
> work invocation (`net = vsock_g2h_net_get()` at…
> - [Medium] By setting `.netns_assign_allow = true` and `.reset =
> virtio_transport_reset` (net/vmw_vsock/virtio_transport.c:590-591)…
> - [Low] virtio_transport_reset_no_sock() is promoted to an exported
> cross-module API (declared in include/linux/virtio_vsock.h,…
>
> --
>
> vsock/virtio: support guest device network namespace
>
> This makes the virtio-vsock guest device assignable to a network namespace
> and initializes it to init_net. The RX work now looks up the currently
> assigned namespace and resets received packets while no valid namespace is
> available, and new connected sockets are inserted via the new
> vsock_maybe_set_connected() helper so insertion is atomic with respect to
> namespace assignment.
>
> A few questions below.
>
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index f91704731057e..5d15b6d6bdf7f 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -286,6 +286,9 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
> > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> > void virtio_transport_deliver_tap_pkt(struct sk_buff *skb);
> > +int virtio_transport_reset(struct vsock_sock *vsk, struct sk_buff *skb);
> > +int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > + struct sk_buff *skb, struct net *net);
> > int virtio_transport_purge_skbs(void *vsk, struct sk_buff_head *list);
> > int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t read_actor);
> > int virtio_transport_notify_set_rcvlowat(struct vsock_sock *vsk, int val);
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 4f9aa9c4c3aa5..5ad93af4bd2b1 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
>
> [ ... ]
>
> > @@ -587,6 +587,8 @@ static struct virtio_transport virtio_transport = {
> > .seqpacket_has_data = virtio_transport_seqpacket_has_data,
> >
> > .msgzerocopy_allow = virtio_transport_msgzerocopy_allow,
> > + .netns_assign_allow = true,
> > + .reset = virtio_transport_reset,
>
> [Severity: Medium]
>
> Setting .netns_assign_allow = true makes vsock_netns_assignable() true for
> the g2h transport, which activates the reset sweep in
> vsock_g2h_reset_unreachable(). That sweep calls t->reset() while holding
> only vsock_register_mutex and a socket reference, never lock_sock(sk):
>
> net/vmw_vsock/af_vsock.c:vsock_g2h_reset_unreachable() {
> ...
> sk->sk_state = TCP_CLOSE;
> sk->sk_err = ECONNRESET;
> sk_error_report(sk);
> ...
> list_for_each_entry_safe(vsk, tmp, &reset_list, pending_reset) {
> list_del_init(&vsk->pending_reset);
> t->reset(vsk, NULL);
> ...
> }
Actually, lock_sock() after register mutex can deadlock on the
virtio_transport_recv_listen() path where the lock order is the exact
opposite, why I did not do this.
We might be able to lift the reset_list into vsock_g2h_net_assign, and
then lock_sock() after releasing the register mutex. Will have to
further evaluate...
>
> virtio_transport_reset() -> virtio_transport_send_pkt_info() addresses the
> RST straight out of the socket:
>
> if (!info->remote_cid) {
> dst_cid = vsk->remote_addr.svm_cid;
> dst_port = vsk->remote_addr.svm_port;
>
> Those fields are normally only stable under lock_sock(sk).
>
> The sk_error_report() above wakes a blocking connect(), which then runs:
>
> net/vmw_vsock/af_vsock.c:vsock_connect() {
> err = sock_error(sk);
> if (err) {
> sk->sk_state = TCP_CLOSE;
> sock->state = SS_UNCONNECTED;
> }
> }
>
> and returns -ECONNRESET. If userspace retries connect() immediately, the
>
> memcpy(&vsk->remote_addr, remote_addr,
> sizeof(vsk->remote_addr));
>
> in vsock_connect() runs before vsock_assign_transport() blocks on
> vsock_register_mutex, so it is not serialized against the sweep that is
> still walking reset_list.
>
> Can the sweep then read the freshly overwritten remote_addr and send the
> RST to the new destination, so the actual peer of the connection that was
> just torn down never gets a reset and waits for a timeout instead?
>
> >
> > .notify_poll_in = virtio_transport_notify_poll_in,
> > .notify_poll_out = virtio_transport_notify_poll_out,
>
> [ ... ]
>
> > @@ -633,7 +635,11 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > {
> > struct virtio_vsock *vsock =
> > container_of(work, struct virtio_vsock, rx_work);
> > + struct virtio_transport *t = &virtio_transport;
> > struct virtqueue *vq;
> > + struct net *net;
> > +
> > + net = vsock_g2h_net_get();
> >
> > mutex_lock(&vsock->rx_lock);
> >
> > @@ -682,10 +688,14 @@ static void virtio_transport_rx_work(struct work_struct *work)
> >
> > virtio_transport_deliver_tap_pkt(skb);
> >
> > - /* Force virtio-transport into global mode since it
> > - * does not yet support local-mode namespacing.
> > - */
> > - virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
> > + /* The virtio send path does not use @net. */
> > + if (unlikely(!net)) {
> > + virtio_transport_reset_no_sock(t, skb, NULL);
> > + kfree_skb(skb);
> > + continue;
> > + }
> > +
> > + virtio_transport_recv_pkt(t, skb, net);
>
> [Severity: Medium]
>
> The namespace is sampled once per work invocation and then pinned with a
> hard reference for the whole drain loop:
>
> net = vsock_g2h_net_get();
> ...
> do {
> ...
> virtio_transport_recv_pkt(t, skb, net);
> } while (!virtqueue_enable_cb(vq));
>
> Meanwhile the assign path publishes the new namespace and then sweeps the
> connected table:
>
> net/vmw_vsock/af_vsock.c:vsock_g2h_net_assign() {
> ...
> rcu_assign_pointer(vsock_g2h_net, net);
> vsock_g2h_reset_unreachable(transport_g2h);
> ...
> }
>
> As soon as the new pointer is published, a connect() from the newly
> assigned namespace is permitted, because vsock_assign_transport() consults
> vsock_g2h_net_reachable() which already reads it. If the host RESPONSE for
> that connection is drained by the batch still holding the previous
> namespace, vsock_find_connected_socket_net(&src, &dst, net) does not find
> the socket (vsock_net_check_mode() of the local-mode new net against the
> old net is false), so virtio_transport_recv_pkt() falls into
>
> (void)virtio_transport_reset_no_sock(t, skb, net);
>
> and the just-established connection is reset. Is that acceptable for the
> remainder of an in-flight batch, or should the namespace be re-read per
> packet?
>
> In the other direction, between the rcu_assign_pointer() above and the
> per-socket store in vsock_g2h_reset_unreachable():
>
> sk->sk_state = TCP_CLOSE;
> sk->sk_err = ECONNRESET;
>
> can the in-flight batch still resolve packets against the old namespace and
> deliver payload plus credit updates into sockets that the device no longer
> belongs to?
>
> The commit message describes the batch granularity only for the dying
> namespace case:
>
> "received packets are reset until a new valid namespace has been assigned
> and/or automatically returned to, and the next RX batch begins"
>
> Should it also describe what happens to packets of the newly assigned
> namespace while an older batch is still draining?
>
> > }
> > } while (!virtqueue_enable_cb(vq));
> >
>
> [ ... ]
>
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index f225f53ed4bab..c24049b2a3866 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
>
> [ ... ]
>
> > @@ -1315,8 +1315,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> > * loopback, this is the namespace of the socket. For vhost, this is the
> > * namespace of the VM (i.e., vhost_vsock).
> > */
> > -static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > - struct sk_buff *skb, struct net *net)
> > +int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > + struct sk_buff *skb, struct net *net)
will fix. might have to grab a reference for every pkt, which I was
trying to avoid.
>
> [Severity: Low]
>
> This isn't a bug, but the comment above still enumerates only the loopback
> and vhost meanings of @net, while the helper is now exported via
> include/linux/virtio_vsock.h and the new caller in
> virtio_transport_rx_work() passes NULL. Whether NULL is fine depends on
> the ->send_pkt behind @t: virtio_transport_send_pkt() ignores @net, and
> vhost_transport_send_pkt() passes it to vhost_vsock_get() ->
> vsock_net_check_mode(), which treats NULL as VSOCK_NET_MODE_GLOBAL. Could
> the comment be extended to say that NULL is permitted and what it means?
>
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct virtio_vsock_pkt_info info = {
> > @@ -1355,6 +1355,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> >
> > return t->send_pkt(reply, net);
> > }
> > +EXPORT_SYMBOL_GPL(virtio_transport_reset_no_sock);
>
> [ ... ]
will correct.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock device network namespace
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
` (3 preceding siblings ...)
2026-09-22 1:18 ` [PATCH net-next v2 4/6] vsock/virtio: support guest device network namespace Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-22 1:18 ` [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks Bobby Eshleman
5 siblings, 1 reply; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
From: Bobby Eshleman <bobbyeshleman@meta.com>
Add tests for guest vsock device namespace assignment, including ns
destruction and the device moving between namespaces.
We do add ynl/cli.py, so have added python3 and PyYAML to the dependency
checks.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- Drive the assign with ynl cli.py, drop the vsock_assign_g2h_netns
ioctl helper and its patch
- Terminate the guest socat sender too, it sometimes outlived the test
and kept holding on to a port the next test binds
---
tools/testing/selftests/vsock/vmtest.sh | 421 +++++++++++++++++++++++++++++++-
1 file changed, 416 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 310dfc2a39ad..e94cc2fd90cc 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -17,6 +17,8 @@ readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)
source "${SCRIPT_DIR}"/../kselftest/ktap_helpers.sh
readonly VSOCK_TEST="${SCRIPT_DIR}"/vsock_test
+readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
+readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
readonly TEST_GUEST_PORT=51000
readonly TEST_HOST_PORT=50000
readonly TEST_HOST_PORT_LISTENER=50001
@@ -73,6 +75,12 @@ readonly TEST_NAMES=(
ns_delete_vm_ok
ns_delete_host_ok
ns_delete_both_ok
+ ns_guest_local_connect_to_host_fails
+ ns_guest_assign_g2h_netns_connect_to_host_ok
+ ns_guest_assign_g2h_netns_init_ns_connect_fails
+ ns_guest_assign_g2h_netns_host_connect_ok
+ ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
+ ns_guest_assign_g2h_netns_old_conn_send_fails
)
readonly TEST_DESCS=(
# vm_server_host_client
@@ -149,12 +157,36 @@ readonly TEST_DESCS=(
# ns_delete_both_ok
"Check that deleting the VM and host's namespaces does not break the socket connection"
+
+ # ns_guest_local_connect_to_host_fails
+ "Check a guest process in a local ns cannot reach the host without the netns assign."
+
+ # ns_guest_assign_g2h_netns_connect_to_host_ok
+ "Check a guest process in a local ns reaches the host once the vsock device is assigned to it."
+
+ # ns_guest_assign_g2h_netns_init_ns_connect_fails
+ "Check the guest's initial ns loses vsock once the device is assigned to another ns."
+
+ # ns_guest_assign_g2h_netns_host_connect_ok
+ "Check the host reaches a guest listener in the ns the vsock device is assigned to."
+
+ # ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
+ "Check the guest's vsock device returns to the initial ns when its ns is deleted."
+
+ # ns_guest_assign_g2h_netns_old_conn_send_fails
+ "Check connections made before the assign stop sending once they lose the device."
)
readonly USE_SHARED_VM=(
vm_server_host_client
vm_client_host_server
vm_loopback
+ ns_guest_local_connect_to_host_fails
+ ns_guest_assign_g2h_netns_connect_to_host_ok
+ ns_guest_assign_g2h_netns_init_ns_connect_fails
+ ns_guest_assign_g2h_netns_host_connect_ok
+ ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
+ ns_guest_assign_g2h_netns_old_conn_send_fails
)
readonly NS_MODES=("local" "global")
@@ -302,7 +334,8 @@ check_args() {
}
check_deps() {
- for dep in vng ${QEMU} busybox pkill ssh ss socat nsenter; do
+ for dep in vng ${QEMU} busybox pkill ssh ss socat nsenter unshare \
+ python3; do
if [[ ! -x $(command -v "${dep}") ]]; then
echo -e "skip: dependency ${dep} not found!\n"
exit "${KSFT_SKIP}"
@@ -314,6 +347,18 @@ check_deps() {
printf " Please build the kselftest vsock target.\n"
exit "${KSFT_SKIP}"
fi
+
+ if ! python3 -c "import yaml" &>/dev/null; then
+ echo -e "skip: python3 yaml module not found!\n"
+ exit "${KSFT_SKIP}"
+ fi
+
+ for dep in "${YNL_CLI}" "${VSOCK_SPEC}"; do
+ if [[ ! -r "${dep}" ]]; then
+ printf "skip: %s not found!\n" "${dep}"
+ exit "${KSFT_SKIP}"
+ fi
+ done
}
check_netns() {
@@ -401,6 +446,17 @@ setup_home() {
mkdir -p "$(dirname "${SSH_KEY_PATH}")"
ssh-keygen -t ed25519 -f "${SSH_KEY_PATH}" -N "" -q
cp "${VSOCK_TEST}" "${TEST_HOME}"/vsock_test
+
+ mkdir -p "${TEST_HOME}"/ynl
+ cp "${YNL_CLI}" "${TEST_HOME}"/ynl/
+ cp -r "$(dirname "${YNL_CLI}")"/lib "${TEST_HOME}"/ynl/
+ cp "${VSOCK_SPEC}" "${TEST_HOME}"/ynl/
+
+ # One of the tests runs the CLI as an unprivileged user, so the CLI
+ # has to be reachable by one.
+ chmod -R a+rX "${TEST_HOME}"/ynl
+
+ chmod 755 "${TEST_HOME}"
}
create_pidfile() {
@@ -528,6 +584,59 @@ vm_wait_for_ssh() {
done
}
+# Create a local mode namespace in the VM and echo the pid holding it open.
+vm_ns_start() {
+ local ns=$1
+
+ vm_ssh "${ns}" -- \
+ "echo local > /proc/sys/net/vsock/child_ns_mode" &>/dev/null
+
+ vm_ssh "${ns}" -- "unshare -n sleep infinity" \
+ '>/dev/null 2>&1 & echo $!'
+}
+
+# Returns once the holder is gone, so that the namespace is unreferenced and
+# the kernel can start tearing it down.
+vm_ns_stop() {
+ local ns=$1
+ local nspid=$2
+
+ vm_ssh "${ns}" <<-EOF &>/dev/null
+ kill ${nspid}
+ for ((i = 0; i < ${WAIT_PERIOD_MAX}; i++)); do
+ kill -0 ${nspid} 2>/dev/null || break
+ sleep 1
+ done
+ EOF
+}
+
+# Runs in the guest's initial namespace when <nspid> is empty. The command must
+# not contain single quotes.
+vm_ns_exec() {
+ local ns=$1
+ local nspid=$2
+ local cmd=$3
+
+ if [[ -z "${nspid}" ]]; then
+ vm_ssh "${ns}" -- "${cmd}"
+ return
+ fi
+
+ vm_ssh "${ns}" -- nsenter -t "${nspid}" -n sh -c "'${cmd}'"
+}
+
+vm_ns_assign_g2h() {
+ local ns=$1
+ local nspid=$2
+
+ vm_ns_exec "${ns}" "${nspid}" "python3 /root/ynl/cli.py --no-schema \
+ --spec /root/ynl/vsock.yaml --do dev-netns-set"
+}
+
+vm_reset_g2h() {
+ vm_ns_assign_g2h "init_ns" "" &>/dev/null
+}
+
# derived from selftests/net/net_helper.sh
wait_for_listener()
{
@@ -564,17 +673,33 @@ wait_for_listener()
done
}
-vm_wait_for_listener() {
+# Runs in the guest's initial namespace when <nspid> is empty.
+vm_ns_wait_for_listener() {
local ns=$1
- local port=$2
- local protocol=$3
+ local nspid=$2
+ local port=$3
+ local protocol=$4
+ local nsenter=
+ local args
+
+ [[ -n "${nspid}" ]] && nsenter="nsenter -t ${nspid} -n"
+ args="${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX} ${protocol}"
vm_ssh "${ns}" <<EOF
$(declare -f wait_for_listener)
-wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX} ${protocol}
+export -f wait_for_listener
+${nsenter} bash -c "wait_for_listener ${args}"
EOF
}
+vm_wait_for_listener() {
+ local ns=$1
+ local port=$2
+ local protocol=$3
+
+ vm_ns_wait_for_listener "${ns}" "" "${port}" "${protocol}"
+}
+
host_wait_for_listener() {
local ns=$1
local port=$2
@@ -1421,6 +1546,292 @@ test_ns_delete_both_ok() {
check_ns_delete_doesnt_break_connection "both"
}
+# Send a string from the guest to a host listener and leave what the host
+# received in <outfile>.
+guest_send_to_host() {
+ local ns=$1
+ local nspid=$2
+ local port=$3
+ local outfile=$4
+ local cmd="echo TEST | socat -u STDIN VSOCK-CONNECT:2:${port}"
+ local pid
+
+ socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
+ pid=$!
+ host_wait_for_listener "${ns}" "${port}" "vsock"
+
+ vm_ns_exec "${ns}" "${nspid}" "${cmd}" 2>/dev/null
+
+ timeout "${WAIT_PERIOD}" \
+ bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
+
+ terminate_pids "${pid}"
+}
+
+# Send a string from the host to a listener in the guest and leave what the
+# guest received in <outfile>.
+host_send_to_guest() {
+ local ns=$1
+ local nspid=$2
+ local port=$3
+ local outfile=$4
+ local cmd="socat -u VSOCK-LISTEN:${port} STDOUT"
+ local dst="VSOCK-CONNECT:${VSOCK_CID}:${port}"
+ local pid
+
+ vm_ns_exec "${ns}" "${nspid}" "${cmd}" > "${outfile}" 2>/dev/null &
+ pid=$!
+ vm_ns_wait_for_listener "${ns}" "${nspid}" "${port}" "vsock"
+
+ echo TEST | socat -u STDIN "${dst}" 2>/dev/null
+
+ timeout "${WAIT_PERIOD}" \
+ bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
+
+ terminate_pids "${pid}"
+}
+
+test_ns_guest_assign_g2h_netns_old_conn_send_fails() {
+ local gap=$(( WAIT_PERIOD * 3 ))
+ local port=12346
+ local outfile
+ local result
+ local sender
+ local nspid
+ local pid
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ outfile=$(mktemp)
+ socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
+ pid=$!
+ host_wait_for_listener "init_ns" "${port}" "vsock"
+
+ # Send a message, wait, then send another. While waiting, assign the
+ # device to a namespace. Confirm the second message does not arrive.
+ vm_ssh "init_ns" -- \
+ "(echo FIRST; sleep ${gap}; echo SECOND) |" \
+ "socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
+ sender=$!
+
+ sleep "${WAIT_PERIOD}"
+
+ if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
+ log_host "failed to assign the vsock device to the guest ns"
+ terminate_pids "${pid}" "${sender}"
+ rm -f "${outfile}"
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+ return "${KSFT_FAIL}"
+ fi
+
+ # Let the second write happen and land, if it is going to.
+ sleep $(( gap + WAIT_PERIOD ))
+
+ terminate_pids "${pid}" "${sender}"
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+
+ if [[ "${result}" != *FIRST* ]]; then
+ log_host "no connection before the assign: [${result}]"
+ return "${KSFT_FAIL}"
+ fi
+
+ if [[ "${result}" == *SECOND* ]]; then
+ log_host "old connection still delivered after the assign"
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_local_connect_to_host_fails() {
+ local port=12345
+ local outfile
+ local result
+ local nspid
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ outfile=$(mktemp)
+ guest_send_to_host "init_ns" "${nspid}" "${port}" "${outfile}"
+
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" == TEST ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_assign_g2h_netns_connect_to_host_ok() {
+ local port=12345
+ local outfile
+ local result
+ local nspid
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
+ log_host "failed to assign the vsock device to the guest ns"
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+ return "${KSFT_FAIL}"
+ fi
+
+ outfile=$(mktemp)
+ guest_send_to_host "init_ns" "${nspid}" "${port}" "${outfile}"
+
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" != TEST ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_assign_g2h_netns_init_ns_connect_fails() {
+ local port=12345
+ local outfile
+ local result
+ local nspid
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
+ log_host "failed to assign the vsock device to the guest ns"
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+ return "${KSFT_FAIL}"
+ fi
+
+ # The device now belongs to a local-mode namespace, so the guest's
+ # initial namespace must no longer reach the host.
+ outfile=$(mktemp)
+ guest_send_to_host "init_ns" "" "${port}" "${outfile}"
+
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" == TEST ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_assign_g2h_netns_host_connect_ok() {
+ local port=12345
+ local outfile
+ local result
+ local nspid
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
+ log_host "failed to assign the vsock device to the guest ns"
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+ return "${KSFT_FAIL}"
+ fi
+
+ outfile=$(mktemp)
+ host_send_to_guest "init_ns" "${nspid}" "${port}" "${outfile}"
+
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" != TEST ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() {
+ local port=12345
+ local outfile
+ local result
+ local nspid
+ local i
+
+ nspid=$(vm_ns_start "init_ns")
+ if [[ -z "${nspid}" ]]; then
+ log_host "failed to create a namespace inside the guest"
+ return "${KSFT_FAIL}"
+ fi
+
+ if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
+ log_host "failed to assign the vsock device to the guest ns"
+ vm_ns_stop "init_ns" "${nspid}"
+ vm_reset_g2h
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_ns_stop "init_ns" "${nspid}"
+
+ # The holder is gone, but the namespace itself is dismantled from a
+ # workqueue, so the device does not come back the same instant. Retry
+ # until it does, rather than expecting the first send to succeed.
+ outfile=$(mktemp)
+ for ((i = 0; i < 5; i++)); do
+ sleep "${WAIT_PERIOD}"
+ guest_send_to_host "init_ns" "" "$(( port + i ))" "${outfile}"
+ result=$(cat "${outfile}")
+ if [[ "${result}" == TEST ]]; then
+ break
+ fi
+ done
+
+ rm -f "${outfile}"
+ vm_reset_g2h
+
+ if [[ "${result}" != TEST ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
shared_vm_test() {
local tname
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock device network namespace
2026-09-22 1:18 ` [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock " Bobby Eshleman
@ 2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-24 0:42 ` Bobby Eshleman
0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 19:21 UTC (permalink / raw)
To: bobbyeshleman
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 4 · Low: 2 (1)
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] check_deps() in tools/testing/selftests/vsock/vmtest.sh now
hard-requires two files that live outside the selftests directory —…
- [Medium] vm_ns_start() (vmtest.sh:588-596) reports success even when
the guest-side namespace setup fails.
- [Medium] test_ns_guest_assign_g2h_netns_old_conn_send_fails()
(vmtest.sh:1594-1653) orders three concurrent actors — the guest…
- [Medium] host_send_to_guest() (vmtest.sh:1582-1591) and
test_ns_guest_assign_g2h_netns_old_conn_send_fails()…
- [Low] guest_send_to_host() (vmtest.sh:1568) and host_send_to_guest()
(vmtest.sh:1591) release their background listeners with…
- [Low] vm_ns_start() (vmtest.sh:594-595) backgrounds `unshare -n sleep
infinity` in the guest and immediately echoes `$!`, with no…
Pre-existing issues:
- [Low] The new helpers splice an unquoted path into a string handed to
an inner shell: `timeout "${WAIT_PERIOD}" bash -c 'while [[ !
--
selftests/vsock: test the guest vsock device network namespace
This adds six new shared-VM tests to vmtest.sh that exercise assigning the
guest vsock device to a network namespace, using tools/net/ynl/pyynl/cli.py
plus Documentation/netlink/specs/vsock.yaml to drive dev-netns-set. The
dependency checks in check_deps() gain unshare, python3 and PyYAML.
A few questions about the new harness code below.
> diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> index 310dfc2a39ad5..e94cc2fd90ccd 100755
> --- a/tools/testing/selftests/vsock/vmtest.sh
> +++ b/tools/testing/selftests/vsock/vmtest.sh
> @@ -17,6 +17,8 @@ readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)
> source "${SCRIPT_DIR}"/../kselftest/ktap_helpers.sh
>
> readonly VSOCK_TEST="${SCRIPT_DIR}"/vsock_test
> +readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
> +readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
[ ... ]
> @@ -314,6 +347,18 @@ check_deps() {
> printf " Please build the kselftest vsock target.\n"
> exit "${KSFT_SKIP}"
> fi
> +
> + if ! python3 -c "import yaml" &>/dev/null; then
> + echo -e "skip: python3 yaml module not found!\n"
> + exit "${KSFT_SKIP}"
> + fi
> +
> + for dep in "${YNL_CLI}" "${VSOCK_SPEC}"; do
> + if [[ ! -r "${dep}" ]]; then
> + printf "skip: %s not found!\n" "${dep}"
> + exit "${KSFT_SKIP}"
> + fi
> + done
> }
[Severity: Medium]
Does this turn the whole suite into a skip when vmtest.sh is run from an
installed tree? Both new dependencies are anchored at KERNEL_CHECKOUT
(SCRIPT_DIR/../../../../), so they only resolve inside a kernel source
tree, and check_deps() exits before any test is selected:
readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
tools/testing/selftests/vsock/Makefile still ships only:
TEST_PROGS += vmtest.sh
TEST_GEN_FILES := vsock_test
There is no TEST_FILES entry for cli.py, its pyynl/lib package, or
vsock.yaml, so after "make -C tools/testing/selftests install" the new
check can never be satisfied, and the 24 pre-existing tests that do not
need ynl at all are skipped too. Would adding these to TEST_FILES (the way
tools/testing/selftests/net/ovpn/Makefile lists
../../../../net/ynl/pyynl/cli.py) work here, or alternatively limiting the
skip to the six new tests instead of the whole script?
[ ... ]
> @@ -528,6 +584,59 @@ vm_wait_for_ssh() {
> done
> }
>
> +# Create a local mode namespace in the VM and echo the pid holding it open.
> +vm_ns_start() {
> + local ns=$1
> +
> + vm_ssh "${ns}" -- \
> + "echo local > /proc/sys/net/vsock/child_ns_mode" &>/dev/null
> +
> + vm_ssh "${ns}" -- "unshare -n sleep infinity" \
> + '>/dev/null 2>&1 & echo $!'
> +}
[Severity: Medium]
Can vm_ns_start() ever report failure? The remote shell runs
unshare -n sleep infinity >/dev/null 2>&1 & echo $!
so the pid is printed at fork time, regardless of whether unshare actually
ran (missing binary, missing privilege, immediate exit). The write to
/proc/sys/net/vsock/child_ns_mode is also discarded with &>/dev/null and
its status is never looked at.
That makes the guard used by all six new tests unable to detect the
condition its message names:
nspid=$(vm_ns_start "init_ns")
if [[ -z "${nspid}" ]]; then
log_host "failed to create a namespace inside the guest"
return "${KSFT_FAIL}"
fi
For the two negative tests, test_ns_guest_local_connect_to_host_fails()
and test_ns_guest_assign_g2h_netns_init_ns_connect_fails(), the pass
condition is an empty outfile:
if [[ "${result}" == TEST ]]; then
return "${KSFT_FAIL}"
fi
return "${KSFT_PASS}"
An empty outfile is also what a broken harness produces, since vm_ns_exec()
discards stderr and does not check the status of nsenter, and
wait_for_listener() always returns 0. Would it make sense to verify the
namespace exists before testing, for example comparing
readlink /proc/<nspid>/ns/net against /proc/self/ns/net, so a setup failure
cannot be reported as ok?
[Severity: Low]
Separately, is there anything ordering the unshare against its first use?
The pid is echoed as soon as the child is forked, and the consumer is a
later, separate ssh invocation:
vm_ssh "${ns}" -- nsenter -t "${nspid}" -n sh -c "'${cmd}'"
If the guest child has not reached unshare(CLONE_NEWNET) yet,
/proc/<nspid>/ns/net is still the guest's initial netns, so dev-netns-set
targets the initial namespace and the _ok tests fail while the _fails tests
assert nothing. The window looks small, because a fresh SSH connection has
to be set up first, but nothing structurally prevents it.
> +
> +# Returns once the holder is gone, so that the namespace is unreferenced and
> +# the kernel can start tearing it down.
> +vm_ns_stop() {
> + local ns=$1
> + local nspid=$2
> +
> + vm_ssh "${ns}" <<-EOF &>/dev/null
> + kill ${nspid}
> + for ((i = 0; i < ${WAIT_PERIOD_MAX}; i++)); do
> + kill -0 ${nspid} 2>/dev/null || break
> + sleep 1
> + done
> + EOF
> +}
> +
> +# Runs in the guest's initial namespace when <nspid> is empty. The command must
> +# not contain single quotes.
> +vm_ns_exec() {
> + local ns=$1
> + local nspid=$2
> + local cmd=$3
> +
> + if [[ -z "${nspid}" ]]; then
> + vm_ssh "${ns}" -- "${cmd}"
> + return
> + fi
> +
> + vm_ssh "${ns}" -- nsenter -t "${nspid}" -n sh -c "'${cmd}'"
> +}
[Severity: Low]
This isn't a bug introduced by this patch, and the same idiom already
exists in vm_start() and check_ns_delete_doesnt_break_connection(), but the
quoting here rests on a comment rather than on the code: vm_ns_exec()
builds a remote root command as sh -c "'${cmd}'" and the new wait loops
splice an unquoted path into an inner shell:
timeout "${WAIT_PERIOD}" \
bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
Whitespace or shell metacharacters in the path (for instance through TMPDIR
affecting mktemp) would be interpreted by the inner shell and the wait loop
would return immediately on a parse error. Would passing the path as a
positional argument to bash -c be preferable?
[ ... ]
> @@ -1421,6 +1546,292 @@ test_ns_delete_both_ok() {
> check_ns_delete_doesnt_break_connection "both"
> }
>
> +# Send a string from the guest to a host listener and leave what the host
> +# received in <outfile>.
> +guest_send_to_host() {
> + local ns=$1
> + local nspid=$2
> + local port=$3
> + local outfile=$4
> + local cmd="echo TEST | socat -u STDIN VSOCK-CONNECT:2:${port}"
> + local pid
> +
> + socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
> + pid=$!
> + host_wait_for_listener "${ns}" "${port}" "vsock"
> +
> + vm_ns_exec "${ns}" "${nspid}" "${cmd}" 2>/dev/null
> +
> + timeout "${WAIT_PERIOD}" \
> + bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
> +
> + terminate_pids "${pid}"
> +}
[Severity: Low]
Can the host listener still hold the port when the next test binds it?
terminate_pids() only signals and returns:
terminate_pids() {
local pid
for pid in "$@"; do
kill -SIGTERM "${pid}" &>/dev/null || :
done
}
There is no wait, and the six new shared-VM tests reuse hard-coded ports
(12345 in four tests, 12345..12349 and 12346 in the last two) back to back
in the same VM. socat's stderr goes to /dev/null so an EADDRINUSE is
invisible, and host_wait_for_listener() would then be satisfied by the
stale listener because wait_for_listener() just greps
ss --listening --vsock for the port, leaving the new test reading an outfile
nothing writes. Would a wait on the pid, a bind-success check, or unique
per-test ports close that?
> +
> +# Send a string from the host to a listener in the guest and leave what the
> +# guest received in <outfile>.
> +host_send_to_guest() {
> + local ns=$1
> + local nspid=$2
> + local port=$3
> + local outfile=$4
> + local cmd="socat -u VSOCK-LISTEN:${port} STDOUT"
> + local dst="VSOCK-CONNECT:${VSOCK_CID}:${port}"
> + local pid
> +
> + vm_ns_exec "${ns}" "${nspid}" "${cmd}" > "${outfile}" 2>/dev/null &
> + pid=$!
> + vm_ns_wait_for_listener "${ns}" "${nspid}" "${port}" "vsock"
> +
> + echo TEST | socat -u STDIN "${dst}" 2>/dev/null
> +
> + timeout "${WAIT_PERIOD}" \
> + bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
> +
> + terminate_pids "${pid}"
> +}
[Severity: Medium]
Which process does pid=$! name here? vm_ns_exec() is a shell function with
several commands, so backgrounding it makes bash fork a subshell, and $! is
that subshell rather than ssh. terminate_pids() then SIGTERMs only the
wrapper, while the ssh child and the guest-side
socat -u VSOCK-LISTEN:<port> STDOUT keep running.
The same applies to the sender in
test_ns_guest_assign_g2h_netns_old_conn_send_fails():
vm_ssh "init_ns" -- \
"(echo FIRST; sleep ${gap}; echo SECOND) |" \
"socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
sender=$!
If that is right, then the v2 changelog entry
"Terminate the guest socat sender too, it sometimes outlived the
test and kept holding on to a port the next test binds"
does not have the described effect, since killing a local wrapper subshell
cannot terminate the remote process.
A leftover guest socat also keeps a reference on the namespace created by
vm_ns_start(), so the postcondition documented on vm_ns_stop()
# Returns once the holder is gone, so that the namespace is unreferenced and
# the kernel can start tearing it down.
may not hold, which is exactly what
test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() relies on. Would
killing the remote command explicitly (or using ssh with a process group /
pkill inside the guest) be more reliable?
> +
> +test_ns_guest_assign_g2h_netns_old_conn_send_fails() {
> + local gap=$(( WAIT_PERIOD * 3 ))
> + local port=12346
[ ... ]
> + outfile=$(mktemp)
> + socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
> + pid=$!
> + host_wait_for_listener "init_ns" "${port}" "vsock"
> +
> + # Send a message, wait, then send another. While waiting, assign the
> + # device to a namespace. Confirm the second message does not arrive.
> + vm_ssh "init_ns" -- \
> + "(echo FIRST; sleep ${gap}; echo SECOND) |" \
> + "socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
> + sender=$!
> +
> + sleep "${WAIT_PERIOD}"
> +
> + if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
[Severity: Medium]
Is a fixed sleep enough to order these three actors? The guest sender, the
host listener and the assign are only sequenced by sleeps, and nothing
confirms FIRST actually arrived before the assign (unlike
guest_send_to_host(), which polls the outfile).
If ssh plus socat startup in the VM takes longer than WAIT_PERIOD (3s), the
assign happens first, the connection is never established, the outfile stays
empty, and the test reports
log_host "no connection before the assign: [${result}]"
against a correctly behaving kernel.
> + log_host "failed to assign the vsock device to the guest ns"
> + terminate_pids "${pid}" "${sender}"
> + rm -f "${outfile}"
> + vm_ns_stop "init_ns" "${nspid}"
> + vm_reset_g2h
> + return "${KSFT_FAIL}"
> + fi
> +
> + # Let the second write happen and land, if it is going to.
> + sleep $(( gap + WAIT_PERIOD ))
[Severity: Medium]
And in the other direction, can the assign lose the race against the
sender's gap? vm_ns_assign_g2h() is an ssh connection plus nsenter plus a
python3 interpreter start and a PyYAML spec parse. If that exceeds the
sender's sleep of 9s, SECOND is delivered before the device moves and the
test reports "old connection still delivered after the assign" for a kernel
that is behaving as intended.
Could the same polling already used in guest_send_to_host() be applied
here, waiting for FIRST in the outfile before the assign and for the assign
to complete before the second write is expected?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-vsock-guest-ns-v2-0-693bd78fde9e%40meta.com
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock device network namespace
2026-09-23 19:21 ` netdev-bot+sashiko
@ 2026-09-24 0:42 ` Bobby Eshleman
0 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-24 0:42 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
On Wed, Sep 23, 2026 at 07:21:05PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 4 · Low: 2 (1)
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] check_deps() in tools/testing/selftests/vsock/vmtest.sh now
> hard-requires two files that live outside the selftests directory —…
> - [Medium] vm_ns_start() (vmtest.sh:588-596) reports success even when
> the guest-side namespace setup fails.
> - [Medium] test_ns_guest_assign_g2h_netns_old_conn_send_fails()
> (vmtest.sh:1594-1653) orders three concurrent actors — the guest…
> - [Medium] host_send_to_guest() (vmtest.sh:1582-1591) and
> test_ns_guest_assign_g2h_netns_old_conn_send_fails()…
> - [Low] guest_send_to_host() (vmtest.sh:1568) and host_send_to_guest()
> (vmtest.sh:1591) release their background listeners with…
> - [Low] vm_ns_start() (vmtest.sh:594-595) backgrounds `unshare -n sleep
> infinity` in the guest and immediately echoes `$!`, with no…
>
> Pre-existing issues:
> - [Low] The new helpers splice an unquoted path into a string handed to
> an inner shell: `timeout "${WAIT_PERIOD}" bash -c 'while [[ !
>
> --
>
> selftests/vsock: test the guest vsock device network namespace
>
> This adds six new shared-VM tests to vmtest.sh that exercise assigning the
> guest vsock device to a network namespace, using tools/net/ynl/pyynl/cli.py
> plus Documentation/netlink/specs/vsock.yaml to drive dev-netns-set. The
> dependency checks in check_deps() gain unshare, python3 and PyYAML.
>
> A few questions about the new harness code below.
>
> > diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> > index 310dfc2a39ad5..e94cc2fd90ccd 100755
> > --- a/tools/testing/selftests/vsock/vmtest.sh
> > +++ b/tools/testing/selftests/vsock/vmtest.sh
> > @@ -17,6 +17,8 @@ readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)
> > source "${SCRIPT_DIR}"/../kselftest/ktap_helpers.sh
> >
> > readonly VSOCK_TEST="${SCRIPT_DIR}"/vsock_test
> > +readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
> > +readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
>
> [ ... ]
>
> > @@ -314,6 +347,18 @@ check_deps() {
> > printf " Please build the kselftest vsock target.\n"
> > exit "${KSFT_SKIP}"
> > fi
> > +
> > + if ! python3 -c "import yaml" &>/dev/null; then
> > + echo -e "skip: python3 yaml module not found!\n"
> > + exit "${KSFT_SKIP}"
> > + fi
> > +
> > + for dep in "${YNL_CLI}" "${VSOCK_SPEC}"; do
> > + if [[ ! -r "${dep}" ]]; then
> > + printf "skip: %s not found!\n" "${dep}"
> > + exit "${KSFT_SKIP}"
> > + fi
> > + done
> > }
>
> [Severity: Medium]
> Does this turn the whole suite into a skip when vmtest.sh is run from an
> installed tree? Both new dependencies are anchored at KERNEL_CHECKOUT
> (SCRIPT_DIR/../../../../), so they only resolve inside a kernel source
> tree, and check_deps() exits before any test is selected:
>
> readonly YNL_CLI="${KERNEL_CHECKOUT}"/tools/net/ynl/pyynl/cli.py
> readonly VSOCK_SPEC="${KERNEL_CHECKOUT}"/Documentation/netlink/specs/vsock.yaml
>
> tools/testing/selftests/vsock/Makefile still ships only:
>
> TEST_PROGS += vmtest.sh
> TEST_GEN_FILES := vsock_test
>
> There is no TEST_FILES entry for cli.py, its pyynl/lib package, or
> vsock.yaml, so after "make -C tools/testing/selftests install" the new
> check can never be satisfied, and the 24 pre-existing tests that do not
> need ynl at all are skipped too. Would adding these to TEST_FILES (the way
> tools/testing/selftests/net/ovpn/Makefile lists
> ../../../../net/ynl/pyynl/cli.py) work here, or alternatively limiting the
> skip to the six new tests instead of the whole script?
>
> [ ... ]
valid, will fix.
>
> > @@ -528,6 +584,59 @@ vm_wait_for_ssh() {
> > done
> > }
> >
> > +# Create a local mode namespace in the VM and echo the pid holding it open.
> > +vm_ns_start() {
> > + local ns=$1
> > +
> > + vm_ssh "${ns}" -- \
> > + "echo local > /proc/sys/net/vsock/child_ns_mode" &>/dev/null
> > +
> > + vm_ssh "${ns}" -- "unshare -n sleep infinity" \
> > + '>/dev/null 2>&1 & echo $!'
> > +}
>
> [Severity: Medium]
> Can vm_ns_start() ever report failure? The remote shell runs
>
> unshare -n sleep infinity >/dev/null 2>&1 & echo $!
>
> so the pid is printed at fork time, regardless of whether unshare actually
> ran (missing binary, missing privilege, immediate exit). The write to
> /proc/sys/net/vsock/child_ns_mode is also discarded with &>/dev/null and
> its status is never looked at.
>
> That makes the guard used by all six new tests unable to detect the
> condition its message names:
>
> nspid=$(vm_ns_start "init_ns")
> if [[ -z "${nspid}" ]]; then
> log_host "failed to create a namespace inside the guest"
> return "${KSFT_FAIL}"
> fi
valid, will fix.
>
> For the two negative tests, test_ns_guest_local_connect_to_host_fails()
> and test_ns_guest_assign_g2h_netns_init_ns_connect_fails(), the pass
> condition is an empty outfile:
>
> if [[ "${result}" == TEST ]]; then
> return "${KSFT_FAIL}"
> fi
>
> return "${KSFT_PASS}"
>
> An empty outfile is also what a broken harness produces, since vm_ns_exec()
> discards stderr and does not check the status of nsenter, and
> wait_for_listener() always returns 0. Would it make sense to verify the
> namespace exists before testing, for example comparing
> readlink /proc/<nspid>/ns/net against /proc/self/ns/net, so a setup failure
> cannot be reported as ok?
Sure, if that is the only way the test could break. Will look into and
fix / find a better way.
>
> [Severity: Low]
> Separately, is there anything ordering the unshare against its first use?
> The pid is echoed as soon as the child is forked, and the consumer is a
> later, separate ssh invocation:
>
> vm_ssh "${ns}" -- nsenter -t "${nspid}" -n sh -c "'${cmd}'"
>
> If the guest child has not reached unshare(CLONE_NEWNET) yet,
> /proc/<nspid>/ns/net is still the guest's initial netns, so dev-netns-set
> targets the initial namespace and the _ok tests fail while the _fails tests
> assert nothing. The window looks small, because a fresh SSH connection has
> to be set up first, but nothing structurally prevents it.
ssh is unlikely to win this race, but wierder things have happend. I
guess we can wait until the ns exists before proceeding.
>
> > +
> > +# Returns once the holder is gone, so that the namespace is unreferenced and
> > +# the kernel can start tearing it down.
> > +vm_ns_stop() {
> > + local ns=$1
> > + local nspid=$2
> > +
> > + vm_ssh "${ns}" <<-EOF &>/dev/null
> > + kill ${nspid}
> > + for ((i = 0; i < ${WAIT_PERIOD_MAX}; i++)); do
> > + kill -0 ${nspid} 2>/dev/null || break
> > + sleep 1
> > + done
> > + EOF
> > +}
> > +
> > +# Runs in the guest's initial namespace when <nspid> is empty. The command must
> > +# not contain single quotes.
> > +vm_ns_exec() {
> > + local ns=$1
> > + local nspid=$2
> > + local cmd=$3
> > +
> > + if [[ -z "${nspid}" ]]; then
> > + vm_ssh "${ns}" -- "${cmd}"
> > + return
> > + fi
> > +
> > + vm_ssh "${ns}" -- nsenter -t "${nspid}" -n sh -c "'${cmd}'"
> > +}
>
> [Severity: Low]
> This isn't a bug introduced by this patch, and the same idiom already
> exists in vm_start() and check_ns_delete_doesnt_break_connection(), but the
> quoting here rests on a comment rather than on the code: vm_ns_exec()
> builds a remote root command as sh -c "'${cmd}'" and the new wait loops
> splice an unquoted path into an inner shell:
>
> timeout "${WAIT_PERIOD}" \
> bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
>
> Whitespace or shell metacharacters in the path (for instance through TMPDIR
> affecting mktemp) would be interpreted by the inner shell and the wait loop
> would return immediately on a parse error. Would passing the path as a
> positional argument to bash -c be preferable?
won't fix in this series.
>
> [ ... ]
>
> > @@ -1421,6 +1546,292 @@ test_ns_delete_both_ok() {
> > check_ns_delete_doesnt_break_connection "both"
> > }
> >
> > +# Send a string from the guest to a host listener and leave what the host
> > +# received in <outfile>.
> > +guest_send_to_host() {
> > + local ns=$1
> > + local nspid=$2
> > + local port=$3
> > + local outfile=$4
> > + local cmd="echo TEST | socat -u STDIN VSOCK-CONNECT:2:${port}"
> > + local pid
> > +
> > + socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
> > + pid=$!
> > + host_wait_for_listener "${ns}" "${port}" "vsock"
> > +
> > + vm_ns_exec "${ns}" "${nspid}" "${cmd}" 2>/dev/null
> > +
> > + timeout "${WAIT_PERIOD}" \
> > + bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
> > +
> > + terminate_pids "${pid}"
> > +}
>
> [Severity: Low]
> Can the host listener still hold the port when the next test binds it?
> terminate_pids() only signals and returns:
>
> terminate_pids() {
> local pid
>
> for pid in "$@"; do
> kill -SIGTERM "${pid}" &>/dev/null || :
> done
> }
>
> There is no wait, and the six new shared-VM tests reuse hard-coded ports
> (12345 in four tests, 12345..12349 and 12346 in the last two) back to back
> in the same VM. socat's stderr goes to /dev/null so an EADDRINUSE is
> invisible, and host_wait_for_listener() would then be satisfied by the
> stale listener because wait_for_listener() just greps
> ss --listening --vsock for the port, leaving the new test reading an outfile
> nothing writes. Would a wait on the pid, a bind-success check, or unique
> per-test ports close that?
will fix by waiting on pid.
>
> > +
> > +# Send a string from the host to a listener in the guest and leave what the
> > +# guest received in <outfile>.
> > +host_send_to_guest() {
> > + local ns=$1
> > + local nspid=$2
> > + local port=$3
> > + local outfile=$4
> > + local cmd="socat -u VSOCK-LISTEN:${port} STDOUT"
> > + local dst="VSOCK-CONNECT:${VSOCK_CID}:${port}"
> > + local pid
> > +
> > + vm_ns_exec "${ns}" "${nspid}" "${cmd}" > "${outfile}" 2>/dev/null &
> > + pid=$!
> > + vm_ns_wait_for_listener "${ns}" "${nspid}" "${port}" "vsock"
> > +
> > + echo TEST | socat -u STDIN "${dst}" 2>/dev/null
> > +
> > + timeout "${WAIT_PERIOD}" \
> > + bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done'
> > +
> > + terminate_pids "${pid}"
> > +}
>
> [Severity: Medium]
> Which process does pid=$! name here? vm_ns_exec() is a shell function with
> several commands, so backgrounding it makes bash fork a subshell, and $! is
> that subshell rather than ssh. terminate_pids() then SIGTERMs only the
> wrapper, while the ssh child and the guest-side
> socat -u VSOCK-LISTEN:<port> STDOUT keep running.
>
> The same applies to the sender in
> test_ns_guest_assign_g2h_netns_old_conn_send_fails():
>
> vm_ssh "init_ns" -- \
> "(echo FIRST; sleep ${gap}; echo SECOND) |" \
> "socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
> sender=$!
>
> If that is right, then the v2 changelog entry
>
> "Terminate the guest socat sender too, it sometimes outlived the
> test and kept holding on to a port the next test binds"
>
> does not have the described effect, since killing a local wrapper subshell
> cannot terminate the remote process.
>
> A leftover guest socat also keeps a reference on the namespace created by
> vm_ns_start(), so the postcondition documented on vm_ns_stop()
>
> # Returns once the holder is gone, so that the namespace is unreferenced and
> # the kernel can start tearing it down.
>
> may not hold, which is exactly what
> test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() relies on. Would
> killing the remote command explicitly (or using ssh with a process group /
> pkill inside the guest) be more reliable?
>
> > +
> > +test_ns_guest_assign_g2h_netns_old_conn_send_fails() {
> > + local gap=$(( WAIT_PERIOD * 3 ))
> > + local port=12346
>
> [ ... ]
>
> > + outfile=$(mktemp)
> > + socat -u VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
> > + pid=$!
> > + host_wait_for_listener "init_ns" "${port}" "vsock"
> > +
> > + # Send a message, wait, then send another. While waiting, assign the
> > + # device to a namespace. Confirm the second message does not arrive.
> > + vm_ssh "init_ns" -- \
> > + "(echo FIRST; sleep ${gap}; echo SECOND) |" \
> > + "socat -u STDIN VSOCK-CONNECT:2:${port}" &>/dev/null &
> > + sender=$!
> > +
> > + sleep "${WAIT_PERIOD}"
> > +
> > + if ! vm_ns_assign_g2h "init_ns" "${nspid}"; then
>
> [Severity: Medium]
> Is a fixed sleep enough to order these three actors? The guest sender, the
> host listener and the assign are only sequenced by sleeps, and nothing
> confirms FIRST actually arrived before the assign (unlike
> guest_send_to_host(), which polls the outfile).
>
> If ssh plus socat startup in the VM takes longer than WAIT_PERIOD (3s), the
> assign happens first, the connection is never established, the outfile stays
> empty, and the test reports
>
> log_host "no connection before the assign: [${result}]"
>
> against a correctly behaving kernel.
>
> > + log_host "failed to assign the vsock device to the guest ns"
> > + terminate_pids "${pid}" "${sender}"
> > + rm -f "${outfile}"
> > + vm_ns_stop "init_ns" "${nspid}"
> > + vm_reset_g2h
> > + return "${KSFT_FAIL}"
> > + fi
> > +
> > + # Let the second write happen and land, if it is going to.
> > + sleep $(( gap + WAIT_PERIOD ))
>
> [Severity: Medium]
> And in the other direction, can the assign lose the race against the
> sender's gap? vm_ns_assign_g2h() is an ssh connection plus nsenter plus a
> python3 interpreter start and a PyYAML spec parse. If that exceeds the
> sender's sleep of 9s, SECOND is delivered before the device moves and the
> test reports "old connection still delivered after the assign" for a kernel
> that is behaving as intended.
>
> Could the same polling already used in guest_send_to_host() be applied
> here, waiting for FIRST in the outfile before the assign and for the assign
> to complete before the second write is expected?
>
This is probably the right thing to do... though I'm not a big fan of
having a custom wait_for_X function for every single type of event.
I'll explore if its possible to have more generic/reusable coordination,
or otherwise fix this as is.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks
2026-09-22 1:18 [PATCH net-next v2 0/6] vsock: assign the guest vsock device to a network namespace Bobby Eshleman
` (4 preceding siblings ...)
2026-09-22 1:18 ` [PATCH net-next v2 5/6] selftests/vsock: test the guest vsock " Bobby Eshleman
@ 2026-09-22 1:18 ` Bobby Eshleman
2026-09-23 19:21 ` netdev-bot+sashiko
5 siblings, 1 reply; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-22 1:18 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Shuah Khan, Randy Dunlap,
Donald Hunter
Cc: virtualization, netdev, linux-kernel, linux-doc, kvm,
linux-kselftest, sargun, jlinbox, Stanislav Fomichev,
Bobby Eshleman
From: Bobby Eshleman <bobbyeshleman@meta.com>
VSOCK_CMD_DEV_NETNS_SET refuses callers without CAP_NET_ADMIN in the
init user namespace.
Add two tests: one confirms that CAP_NET_ADMIN is required even by a
privileged user and the other confirms that CAP_NET_ADMIN in an
unprivileged user ns alone is insufficient.
CONFIG_USER_NS is needed to test the CAP_NET_ADMIN + unprivileged user
ns case.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- Assert only that the command fails, not EPERM: ynl's cli.py does not
report the errno (Sashiko)
- Use ynl cli.py for the assign
---
tools/testing/selftests/vsock/config | 1 +
tools/testing/selftests/vsock/vmtest.sh | 54 ++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vsock/config b/tools/testing/selftests/vsock/config
index 5f0a4f17dfc9..4b31085558fa 100644
--- a/tools/testing/selftests/vsock/config
+++ b/tools/testing/selftests/vsock/config
@@ -109,3 +109,4 @@ CONFIG_FS_DAX=y
CONFIG_MEMORY_HOTPLUG=y
CONFIG_MEMORY_HOTREMOVE=y
CONFIG_ZONE_DEVICE=y
+CONFIG_USER_NS=y
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index e94cc2fd90cc..4f42bcdac3f6 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -81,6 +81,8 @@ readonly TEST_NAMES=(
ns_guest_assign_g2h_netns_host_connect_ok
ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
ns_guest_assign_g2h_netns_old_conn_send_fails
+ ns_guest_assign_g2h_netns_no_cap_net_admin_fails
+ ns_guest_assign_g2h_netns_unpriv_user_ns_fails
)
readonly TEST_DESCS=(
# vm_server_host_client
@@ -175,6 +177,12 @@ readonly TEST_DESCS=(
# ns_guest_assign_g2h_netns_old_conn_send_fails
"Check connections made before the assign stop sending once they lose the device."
+
+ # ns_guest_assign_g2h_netns_no_cap_net_admin_fails
+ "Check assigning the guest's vsock device to a namespace needs CAP_NET_ADMIN."
+
+ # ns_guest_assign_g2h_netns_unpriv_user_ns_fails
+ "Check an unprivileged user cannot claim the guest's vsock device via a user ns."
)
readonly USE_SHARED_VM=(
@@ -187,6 +195,8 @@ readonly USE_SHARED_VM=(
ns_guest_assign_g2h_netns_host_connect_ok
ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
ns_guest_assign_g2h_netns_old_conn_send_fails
+ ns_guest_assign_g2h_netns_no_cap_net_admin_fails
+ ns_guest_assign_g2h_netns_unpriv_user_ns_fails
)
readonly NS_MODES=("local" "global")
@@ -335,7 +345,7 @@ check_args() {
check_deps() {
for dep in vng ${QEMU} busybox pkill ssh ss socat nsenter unshare \
- python3; do
+ setpriv python3; do
if [[ ! -x $(command -v "${dep}") ]]; then
echo -e "skip: dependency ${dep} not found!\n"
exit "${KSFT_SKIP}"
@@ -1832,6 +1842,48 @@ test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() {
return "${KSFT_PASS}"
}
+test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails() {
+ local cmd="unshare -n setpriv --bounding-set=-net_admin"
+ local rc
+
+ vm_ssh "init_ns" -- "${cmd}" python3 /root/ynl/cli.py --no-schema \
+ --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
+ rc=$?
+
+ if [[ "${rc}" -eq 0 ]]; then
+ log_host "assign unexpectedly succeeded without CAP_NET_ADMIN"
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_guest_assign_g2h_netns_unpriv_user_ns_fails() {
+ local unpriv_uid=65534
+ local unpriv
+ local rc
+
+ unpriv="setpriv --reuid=${unpriv_uid} --regid=${unpriv_uid}"
+ unpriv="${unpriv} --clear-groups"
+
+ if ! vm_ssh "init_ns" -- "${unpriv} unshare -U true"; then
+ log_host "unprivileged user namespaces unavailable, skipping"
+ return "${KSFT_SKIP}"
+ fi
+
+ vm_ssh "init_ns" -- "${unpriv} unshare -Urn" \
+ python3 /root/ynl/cli.py --no-schema \
+ --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
+ rc=$?
+
+ if [[ "${rc}" -eq 0 ]]; then
+ log_host "assign unexpectedly succeeded for unprivileged user"
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
shared_vm_test() {
local tname
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks
2026-09-22 1:18 ` [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks Bobby Eshleman
@ 2026-09-23 19:21 ` netdev-bot+sashiko
2026-09-24 0:24 ` Bobby Eshleman
0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 19:21 UTC (permalink / raw)
To: bobbyeshleman
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] Both new selftests
(`test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails`,…
- [Low] The two new shared-VM tests omit the `vm_reset_g2h()` cleanup
that every other ns_guest_assign_g2h_netns_* test in USE_SHARED_VM…
--
selftests/vsock: test the netns assign privilege checks
Adds two vmtest.sh cases that attempt VSOCK_CMD_DEV_NETNS_SET without
CAP_NET_ADMIN and from an unprivileged user namespace, and enables
CONFIG_USER_NS in the vsock selftest config.
> diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> index e94cc2fd90ccd..4f42bcdac3f6c 100755
> --- a/tools/testing/selftests/vsock/vmtest.sh
> +++ b/tools/testing/selftests/vsock/vmtest.sh
[ ... ]
> @@ -187,6 +195,8 @@ readonly TEST_DESCS=(
> ns_guest_assign_g2h_netns_host_connect_ok
> ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
> ns_guest_assign_g2h_netns_old_conn_send_fails
> + ns_guest_assign_g2h_netns_no_cap_net_admin_fails
> + ns_guest_assign_g2h_netns_unpriv_user_ns_fails
> )
> readonly NS_MODES=("local" "global")
[Severity: Low]
Both new tests are added to USE_SHARED_VM but neither calls
vm_reset_g2h() on any exit path, unlike every other
ns_guest_assign_g2h_netns_* test in that list.
Should these follow the sibling pattern? If the privilege check under
test is broken, the assign succeeds and the guest's single g2h device is
left owned by the transient namespace created by unshare, while the
harness moves on to the next test in the same VM.
The kernel does restore this by itself once that namespace is reaped:
net/vmw_vsock/af_vsock.c:vsock_g2h_net_reset() {
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);
...
}
registered as .pre_exit in vsock_pernet_ops, and the next test's
vm_ns_assign_g2h() overwrites the pointer anyway. The residual window is
the asynchronous teardown that ..._reset_on_ns_delete_ok already documents
("the namespace itself is dismantled from a workqueue, so the device does
not come back the same instant"). Since the new tests are appended last
in TEST_NAMES and USE_SHARED_VM, a default run is unaffected; this only
shows up with selective or reordered invocation.
[ ... ]
> @@ -1832,6 +1842,48 @@ test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() {
> return "${KSFT_PASS}"
> }
>
> +test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails() {
> + local cmd="unshare -n setpriv --bounding-set=-net_admin"
> + local rc
> +
> + vm_ssh "init_ns" -- "${cmd}" python3 /root/ynl/cli.py --no-schema \
> + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> + rc=$?
> +
> + if [[ "${rc}" -eq 0 ]]; then
> + log_host "assign unexpectedly succeeded without CAP_NET_ADMIN"
> + return "${KSFT_FAIL}"
> + fi
> +
> + return "${KSFT_PASS}"
> +}
[Severity: Medium]
Can this test pass without the capability check ever being reached? The
only assertion is rc -eq 0 => FAIL, output is discarded with &>/dev/null,
and there is no positive control, so any nonzero status reads as
KSFT_PASS.
A few concrete sources of a nonzero status that are not -EPERM:
net/vmw_vsock/af_vsock.c:vsock_nl_dev_netns_set_doit() {
int err = vsock_g2h_net_assign(genl_info_net(info));
if (err == -ENODEV)
NL_SET_ERR_MSG(info->extack,
"no guest-to-host transport is loaded");
else if (err == -EOPNOTSUPP)
...
}
and cli.py collapses all of those to the same status as -EPERM:
tools/net/ynl/pyynl/cli.py:main() {
except NlError as e:
print(e)
sys.exit(1)
}
vm_ssh() also returns ssh's own status, so a transport failure (255) or a
remote shell failure (127, e.g. an installed util-linux that does not
accept setpriv --bounding-set=-net_admin) is indistinguishable from the
expected refusal. A python-side failure such as a missing
/root/ynl/vsock.yaml exits 1 too.
Would it be worth asserting the privileged case succeeds first? The file
already has the helper the sibling tests use as a precondition:
vm_ns_assign_g2h() {
local ns=$1
local nspid=$2
vm_ns_exec "${ns}" "${nspid}" "python3 /root/ynl/cli.py --no-schema \
--spec /root/ynl/vsock.yaml --do dev-netns-set"
}
On the changelog note "ynl's cli.py does not report the errno": is that
accurate? NlError.__str__() in tools/net/ynl/pyynl/lib/ynl.py does:
msg += os.strerror(self.error)
so the error string is printed and the test could match "Operation not
permitted" instead of sending everything to /dev/null.
> +
> +test_ns_guest_assign_g2h_netns_unpriv_user_ns_fails() {
> + local unpriv_uid=65534
> + local unpriv
> + local rc
> +
> + unpriv="setpriv --reuid=${unpriv_uid} --regid=${unpriv_uid}"
> + unpriv="${unpriv} --clear-groups"
> +
> + if ! vm_ssh "init_ns" -- "${unpriv} unshare -U true"; then
> + log_host "unprivileged user namespaces unavailable, skipping"
> + return "${KSFT_SKIP}"
> + fi
> +
> + vm_ssh "init_ns" -- "${unpriv} unshare -Urn" \
> + python3 /root/ynl/cli.py --no-schema \
> + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> + rc=$?
> +
> + if [[ "${rc}" -eq 0 ]]; then
> + log_host "assign unexpectedly succeeded for unprivileged user"
> + return "${KSFT_FAIL}"
> + fi
> +
> + return "${KSFT_PASS}"
> +}
[Severity: Medium]
Same rc != 0 => KSFT_PASS inference here, plus the availability probe is
not the command being measured:
probe: ${unpriv} unshare -U true
measured: ${unpriv} unshare -Urn python3 /root/ynl/cli.py ...
If the extra -r/-n setup or the python startup under uid 65534 fails, the
result is KSFT_PASS rather than KSFT_SKIP or KSFT_FAIL, and the child user
namespace never actually holds CAP_NET_ADMIN, which is the condition the
commit message says is being tested:
the other confirms that CAP_NET_ADMIN in an unprivileged user ns
alone is insufficient.
Would checking the reported error string, or asserting the privileged
assign works in the same test, make these two cases fail if
GENL_ADMIN_PERM is ever dropped from VSOCK_CMD_DEV_NETNS_SET?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-vsock-guest-ns-v2-0-693bd78fde9e%40meta.com
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks
2026-09-23 19:21 ` netdev-bot+sashiko
@ 2026-09-24 0:24 ` Bobby Eshleman
2026-09-24 0:55 ` Bobby Eshleman
0 siblings, 1 reply; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-24 0:24 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
On Wed, Sep 23, 2026 at 07:21:07PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] Both new selftests
> (`test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails`,…
> - [Low] The two new shared-VM tests omit the `vm_reset_g2h()` cleanup
> that every other ns_guest_assign_g2h_netns_* test in USE_SHARED_VM…
>
> --
>
> selftests/vsock: test the netns assign privilege checks
>
> Adds two vmtest.sh cases that attempt VSOCK_CMD_DEV_NETNS_SET without
> CAP_NET_ADMIN and from an unprivileged user namespace, and enables
> CONFIG_USER_NS in the vsock selftest config.
>
> > diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> > index e94cc2fd90ccd..4f42bcdac3f6c 100755
> > --- a/tools/testing/selftests/vsock/vmtest.sh
> > +++ b/tools/testing/selftests/vsock/vmtest.sh
>
> [ ... ]
>
> > @@ -187,6 +195,8 @@ readonly TEST_DESCS=(
> > ns_guest_assign_g2h_netns_host_connect_ok
> > ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
> > ns_guest_assign_g2h_netns_old_conn_send_fails
> > + ns_guest_assign_g2h_netns_no_cap_net_admin_fails
> > + ns_guest_assign_g2h_netns_unpriv_user_ns_fails
> > )
> > readonly NS_MODES=("local" "global")
>
> [Severity: Low]
> Both new tests are added to USE_SHARED_VM but neither calls
> vm_reset_g2h() on any exit path, unlike every other
> ns_guest_assign_g2h_netns_* test in that list.
>
> Should these follow the sibling pattern? If the privilege check under
> test is broken, the assign succeeds and the guest's single g2h device is
> left owned by the transient namespace created by unshare, while the
> harness moves on to the next test in the same VM.
>
> The kernel does restore this by itself once that namespace is reaped:
>
> net/vmw_vsock/af_vsock.c:vsock_g2h_net_reset() {
> 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);
> ...
> }
>
> registered as .pre_exit in vsock_pernet_ops, and the next test's
> vm_ns_assign_g2h() overwrites the pointer anyway. The residual window is
> the asynchronous teardown that ..._reset_on_ns_delete_ok already documents
> ("the namespace itself is dismantled from a workqueue, so the device does
> not come back the same instant"). Since the new tests are appended last
> in TEST_NAMES and USE_SHARED_VM, a default run is unaffected; this only
> shows up with selective or reordered invocation.
Yes, these tests should call it regardless. Relying on cleanup makes the
pass/fail race-y.
>
> [ ... ]
>
> > @@ -1832,6 +1842,48 @@ test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() {
> > return "${KSFT_PASS}"
> > }
> >
> > +test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails() {
> > + local cmd="unshare -n setpriv --bounding-set=-net_admin"
> > + local rc
> > +
> > + vm_ssh "init_ns" -- "${cmd}" python3 /root/ynl/cli.py --no-schema \
> > + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> > + rc=$?
> > +
> > + if [[ "${rc}" -eq 0 ]]; then
> > + log_host "assign unexpectedly succeeded without CAP_NET_ADMIN"
> > + return "${KSFT_FAIL}"
> > + fi
> > +
> > + return "${KSFT_PASS}"
> > +}
>
> [Severity: Medium]
> Can this test pass without the capability check ever being reached? The
> only assertion is rc -eq 0 => FAIL, output is discarded with &>/dev/null,
> and there is no positive control, so any nonzero status reads as
> KSFT_PASS.
>
> A few concrete sources of a nonzero status that are not -EPERM:
>
> net/vmw_vsock/af_vsock.c:vsock_nl_dev_netns_set_doit() {
> int err = vsock_g2h_net_assign(genl_info_net(info));
>
> if (err == -ENODEV)
> NL_SET_ERR_MSG(info->extack,
> "no guest-to-host transport is loaded");
> else if (err == -EOPNOTSUPP)
> ...
> }
>
> and cli.py collapses all of those to the same status as -EPERM:
>
> tools/net/ynl/pyynl/cli.py:main() {
> except NlError as e:
> print(e)
> sys.exit(1)
> }
>
> vm_ssh() also returns ssh's own status, so a transport failure (255) or a
> remote shell failure (127, e.g. an installed util-linux that does not
> accept setpriv --bounding-set=-net_admin) is indistinguishable from the
> expected refusal. A python-side failure such as a missing
> /root/ynl/vsock.yaml exits 1 too.
>
> Would it be worth asserting the privileged case succeeds first? The file
> already has the helper the sibling tests use as a precondition:
>
> vm_ns_assign_g2h() {
> local ns=$1
> local nspid=$2
>
> vm_ns_exec "${ns}" "${nspid}" "python3 /root/ynl/cli.py --no-schema \
> --spec /root/ynl/vsock.yaml --do dev-netns-set"
> }
>
> On the changelog note "ynl's cli.py does not report the errno": is that
> accurate? NlError.__str__() in tools/net/ynl/pyynl/lib/ynl.py does:
>
> msg += os.strerror(self.error)
>
> so the error string is printed and the test could match "Operation not
> permitted" instead of sending everything to /dev/null.
I considered this but felt like grepping stderr/stdout is somewhat
unnatural. My plan was to later propose that cli.py returns the err via
exit code, and then update these tests.
First checking the privileged assign sounds like a reasonable middle
ground to rule out the other error states.
>
> > +
> > +test_ns_guest_assign_g2h_netns_unpriv_user_ns_fails() {
> > + local unpriv_uid=65534
> > + local unpriv
> > + local rc
> > +
> > + unpriv="setpriv --reuid=${unpriv_uid} --regid=${unpriv_uid}"
> > + unpriv="${unpriv} --clear-groups"
> > +
> > + if ! vm_ssh "init_ns" -- "${unpriv} unshare -U true"; then
> > + log_host "unprivileged user namespaces unavailable, skipping"
> > + return "${KSFT_SKIP}"
> > + fi
> > +
> > + vm_ssh "init_ns" -- "${unpriv} unshare -Urn" \
> > + python3 /root/ynl/cli.py --no-schema \
> > + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> > + rc=$?
> > +
> > + if [[ "${rc}" -eq 0 ]]; then
> > + log_host "assign unexpectedly succeeded for unprivileged user"
> > + return "${KSFT_FAIL}"
> > + fi
> > +
> > + return "${KSFT_PASS}"
> > +}
>
> [Severity: Medium]
> Same rc != 0 => KSFT_PASS inference here, plus the availability probe is
> not the command being measured:
>
> probe: ${unpriv} unshare -U true
> measured: ${unpriv} unshare -Urn python3 /root/ynl/cli.py ...
>
> If the extra -r/-n setup or the python startup under uid 65534 fails, the
> result is KSFT_PASS rather than KSFT_SKIP or KSFT_FAIL, and the child user
> namespace never actually holds CAP_NET_ADMIN, which is the condition the
> commit message says is being tested:
>
> the other confirms that CAP_NET_ADMIN in an unprivileged user ns
> alone is insufficient.
>
> Would checking the reported error string, or asserting the privileged
> assign works in the same test, make these two cases fail if
> GENL_ADMIN_PERM is ever dropped from VSOCK_CMD_DEV_NETNS_SET?
Yep, can follow the same approach as above.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 6/6] selftests/vsock: test the netns assign privilege checks
2026-09-24 0:24 ` Bobby Eshleman
@ 2026-09-24 0:55 ` Bobby Eshleman
0 siblings, 0 replies; 16+ messages in thread
From: Bobby Eshleman @ 2026-09-24 0:55 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: sgarzare, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
stefanha, mst, jasowangio, xuanzhuo, eperezma, shuah, rdunlap,
donald.hunter, virtualization, netdev, linux-kernel, linux-doc,
kvm, linux-kselftest, sargun, jlinbox, sdf.kernel, bobbyeshleman
On Wed, Sep 23, 2026 at 05:24:21PM -0700, Bobby Eshleman wrote:
> On Wed, Sep 23, 2026 at 07:21:07PM +0000, netdev-bot+sashiko@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 1
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] Both new selftests
> > (`test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails`,…
> > - [Low] The two new shared-VM tests omit the `vm_reset_g2h()` cleanup
> > that every other ns_guest_assign_g2h_netns_* test in USE_SHARED_VM…
> >
> > --
> >
> > selftests/vsock: test the netns assign privilege checks
> >
> > Adds two vmtest.sh cases that attempt VSOCK_CMD_DEV_NETNS_SET without
> > CAP_NET_ADMIN and from an unprivileged user namespace, and enables
> > CONFIG_USER_NS in the vsock selftest config.
> >
> > > diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> > > index e94cc2fd90ccd..4f42bcdac3f6c 100755
> > > --- a/tools/testing/selftests/vsock/vmtest.sh
> > > +++ b/tools/testing/selftests/vsock/vmtest.sh
> >
> > [ ... ]
> >
> > > @@ -187,6 +195,8 @@ readonly TEST_DESCS=(
> > > ns_guest_assign_g2h_netns_host_connect_ok
> > > ns_guest_assign_g2h_netns_reset_on_ns_delete_ok
> > > ns_guest_assign_g2h_netns_old_conn_send_fails
> > > + ns_guest_assign_g2h_netns_no_cap_net_admin_fails
> > > + ns_guest_assign_g2h_netns_unpriv_user_ns_fails
> > > )
> > > readonly NS_MODES=("local" "global")
> >
> > [Severity: Low]
> > Both new tests are added to USE_SHARED_VM but neither calls
> > vm_reset_g2h() on any exit path, unlike every other
> > ns_guest_assign_g2h_netns_* test in that list.
> >
> > Should these follow the sibling pattern? If the privilege check under
> > test is broken, the assign succeeds and the guest's single g2h device is
> > left owned by the transient namespace created by unshare, while the
> > harness moves on to the next test in the same VM.
> >
> > The kernel does restore this by itself once that namespace is reaped:
> >
> > net/vmw_vsock/af_vsock.c:vsock_g2h_net_reset() {
> > 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);
> > ...
> > }
> >
> > registered as .pre_exit in vsock_pernet_ops, and the next test's
> > vm_ns_assign_g2h() overwrites the pointer anyway. The residual window is
> > the asynchronous teardown that ..._reset_on_ns_delete_ok already documents
> > ("the namespace itself is dismantled from a workqueue, so the device does
> > not come back the same instant"). Since the new tests are appended last
> > in TEST_NAMES and USE_SHARED_VM, a default run is unaffected; this only
> > shows up with selective or reordered invocation.
>
> Yes, these tests should call it regardless. Relying on cleanup makes the
> pass/fail race-y.
>
> >
> > [ ... ]
> >
> > > @@ -1832,6 +1842,48 @@ test_ns_guest_assign_g2h_netns_reset_on_ns_delete_ok() {
> > > return "${KSFT_PASS}"
> > > }
> > >
> > > +test_ns_guest_assign_g2h_netns_no_cap_net_admin_fails() {
> > > + local cmd="unshare -n setpriv --bounding-set=-net_admin"
> > > + local rc
> > > +
> > > + vm_ssh "init_ns" -- "${cmd}" python3 /root/ynl/cli.py --no-schema \
> > > + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> > > + rc=$?
> > > +
> > > + if [[ "${rc}" -eq 0 ]]; then
> > > + log_host "assign unexpectedly succeeded without CAP_NET_ADMIN"
> > > + return "${KSFT_FAIL}"
> > > + fi
> > > +
> > > + return "${KSFT_PASS}"
> > > +}
> >
> > [Severity: Medium]
> > Can this test pass without the capability check ever being reached? The
> > only assertion is rc -eq 0 => FAIL, output is discarded with &>/dev/null,
> > and there is no positive control, so any nonzero status reads as
> > KSFT_PASS.
> >
> > A few concrete sources of a nonzero status that are not -EPERM:
> >
> > net/vmw_vsock/af_vsock.c:vsock_nl_dev_netns_set_doit() {
> > int err = vsock_g2h_net_assign(genl_info_net(info));
> >
> > if (err == -ENODEV)
> > NL_SET_ERR_MSG(info->extack,
> > "no guest-to-host transport is loaded");
> > else if (err == -EOPNOTSUPP)
> > ...
> > }
> >
> > and cli.py collapses all of those to the same status as -EPERM:
> >
> > tools/net/ynl/pyynl/cli.py:main() {
> > except NlError as e:
> > print(e)
> > sys.exit(1)
> > }
> >
> > vm_ssh() also returns ssh's own status, so a transport failure (255) or a
> > remote shell failure (127, e.g. an installed util-linux that does not
> > accept setpriv --bounding-set=-net_admin) is indistinguishable from the
> > expected refusal. A python-side failure such as a missing
> > /root/ynl/vsock.yaml exits 1 too.
> >
> > Would it be worth asserting the privileged case succeeds first? The file
> > already has the helper the sibling tests use as a precondition:
> >
> > vm_ns_assign_g2h() {
> > local ns=$1
> > local nspid=$2
> >
> > vm_ns_exec "${ns}" "${nspid}" "python3 /root/ynl/cli.py --no-schema \
> > --spec /root/ynl/vsock.yaml --do dev-netns-set"
> > }
> >
> > On the changelog note "ynl's cli.py does not report the errno": is that
> > accurate? NlError.__str__() in tools/net/ynl/pyynl/lib/ynl.py does:
> >
> > msg += os.strerror(self.error)
> >
> > so the error string is printed and the test could match "Operation not
> > permitted" instead of sending everything to /dev/null.
>
> I considered this but felt like grepping stderr/stdout is somewhat
> unnatural. My plan was to later propose that cli.py returns the err via
> exit code, and then update these tests.
>
> First checking the privileged assign sounds like a reasonable middle
> ground to rule out the other error states.
>
> >
> > > +
> > > +test_ns_guest_assign_g2h_netns_unpriv_user_ns_fails() {
> > > + local unpriv_uid=65534
> > > + local unpriv
> > > + local rc
> > > +
> > > + unpriv="setpriv --reuid=${unpriv_uid} --regid=${unpriv_uid}"
> > > + unpriv="${unpriv} --clear-groups"
> > > +
> > > + if ! vm_ssh "init_ns" -- "${unpriv} unshare -U true"; then
> > > + log_host "unprivileged user namespaces unavailable, skipping"
> > > + return "${KSFT_SKIP}"
> > > + fi
> > > +
> > > + vm_ssh "init_ns" -- "${unpriv} unshare -Urn" \
> > > + python3 /root/ynl/cli.py --no-schema \
> > > + --spec /root/ynl/vsock.yaml --do dev-netns-set &>/dev/null
> > > + rc=$?
> > > +
> > > + if [[ "${rc}" -eq 0 ]]; then
> > > + log_host "assign unexpectedly succeeded for unprivileged user"
> > > + return "${KSFT_FAIL}"
> > > + fi
> > > +
> > > + return "${KSFT_PASS}"
> > > +}
> >
> > [Severity: Medium]
> > Same rc != 0 => KSFT_PASS inference here, plus the availability probe is
> > not the command being measured:
> >
> > probe: ${unpriv} unshare -U true
> > measured: ${unpriv} unshare -Urn python3 /root/ynl/cli.py ...
> >
> > If the extra -r/-n setup or the python startup under uid 65534 fails, the
> > result is KSFT_PASS rather than KSFT_SKIP or KSFT_FAIL, and the child user
> > namespace never actually holds CAP_NET_ADMIN, which is the condition the
> > commit message says is being tested:
> >
> > the other confirms that CAP_NET_ADMIN in an unprivileged user ns
> > alone is insufficient.
> >
> > Would checking the reported error string, or asserting the privileged
> > assign works in the same test, make these two cases fail if
> > GENL_ADMIN_PERM is ever dropped from VSOCK_CMD_DEV_NETNS_SET?
>
> Yep, can follow the same approach as above.
pw-bot: cr
^ permalink raw reply [flat|nested] 16+ messages in thread