* [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test
@ 2025-06-11 19:56 Michal Luczaj
2025-06-11 19:56 ` [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper Michal Luczaj
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Michal Luczaj @ 2025-06-11 19:56 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Increase the coverage of a test implemented in commit 301a62dfb0d0
("vsock/test: Add test for UAF due to socket unbinding"). Take this
opportunity to factor out some utility code, drop a redundant sync between
client and server, and introduce a /proc/kallsyms harvesting logic for
auto-detecting registered vsock transports.
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Changes in v3:
- Drop "RFC" prefix, rebase, amend commit logs
- get_transports(): don't look for a symbol that was already found
- Expand testcase comments, clean up the code [Stefano]
- Streamline `enum transport` and `transport_ksyms` [Stefano]
- Move KALLSYMS_* defines from utils.h to utils.c [Stefano]
- Link to v2: https://lore.kernel.org/r/20250528-vsock-test-inc-cov-v2-0-8f655b40d57c@rbox.co
Changes in v2:
- Speed up: don't bother checking EINTR or respecting timeout on connect()s
- Introduce get_transports(), warn on unsupported setup [Stefano]
- Comment the code, drop the sync, introduce vsock_bind_try() [Stefano]
- Link to v1: https://lore.kernel.org/r/20250523-vsock-test-inc-cov-v1-1-fa3507941bbd@rbox.co
---
Michal Luczaj (3):
vsock/test: Introduce vsock_bind_try() helper
vsock/test: Introduce get_transports()
vsock/test: Cover more CIDs in transport_uaf test
tools/testing/vsock/util.c | 80 ++++++++++++++++++++++++++++++++--
tools/testing/vsock/util.h | 30 +++++++++++++
tools/testing/vsock/vsock_test.c | 93 ++++++++++++++++++++++++++++++++--------
3 files changed, 181 insertions(+), 22 deletions(-)
---
base-commit: 0097c4195b1d0ca57d15979626c769c74747b5a0
change-id: 20250326-vsock-test-inc-cov-b823822bdb78
Best regards,
--
Michal Luczaj <mhal@rbox.co>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
@ 2025-06-11 19:56 ` Michal Luczaj
2025-06-13 15:03 ` Luigi Leonardi
2025-06-11 19:56 ` [PATCH net-next v3 2/3] vsock/test: Introduce get_transports() Michal Luczaj
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Michal Luczaj @ 2025-06-11 19:56 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Create a socket and bind() it. If binding failed, gracefully return an
error code while preserving `errno`.
Base vsock_bind() on top of it.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
tools/testing/vsock/util.c | 24 +++++++++++++++++++++---
tools/testing/vsock/util.h | 1 +
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 0c7e9cbcbc85cde9c8764fc3bb623cde2f6c77a6..b7b3fb2221c1682ecde58cf12e2f0b0ded1cff39 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -121,15 +121,17 @@ bool vsock_wait_sent(int fd)
return !ret;
}
-/* Create socket <type>, bind to <cid, port> and return the file descriptor. */
-int vsock_bind(unsigned int cid, unsigned int port, int type)
+/* Create socket <type>, bind to <cid, port>.
+ * Return the file descriptor, or -1 on error.
+ */
+int vsock_bind_try(unsigned int cid, unsigned int port, int type)
{
struct sockaddr_vm sa = {
.svm_family = AF_VSOCK,
.svm_cid = cid,
.svm_port = port,
};
- int fd;
+ int fd, saved_errno;
fd = socket(AF_VSOCK, type, 0);
if (fd < 0) {
@@ -138,6 +140,22 @@ int vsock_bind(unsigned int cid, unsigned int port, int type)
}
if (bind(fd, (struct sockaddr *)&sa, sizeof(sa))) {
+ saved_errno = errno;
+ close(fd);
+ errno = saved_errno;
+ fd = -1;
+ }
+
+ return fd;
+}
+
+/* Create socket <type>, bind to <cid, port> and return the file descriptor. */
+int vsock_bind(unsigned int cid, unsigned int port, int type)
+{
+ int fd;
+
+ fd = vsock_bind_try(cid, port, type);
+ if (fd < 0) {
perror("bind");
exit(EXIT_FAILURE);
}
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 5e2db67072d5053804a9bb93934b625ea78bcd7a..0afe7cbae12e5194172c639ccfbeb8b81f7c25ac 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -44,6 +44,7 @@ int vsock_connect(unsigned int cid, unsigned int port, int type);
int vsock_accept(unsigned int cid, unsigned int port,
struct sockaddr_vm *clientaddrp, int type);
int vsock_stream_connect(unsigned int cid, unsigned int port);
+int vsock_bind_try(unsigned int cid, unsigned int port, int type);
int vsock_bind(unsigned int cid, unsigned int port, int type);
int vsock_bind_connect(unsigned int cid, unsigned int port,
unsigned int bind_port, int type);
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v3 2/3] vsock/test: Introduce get_transports()
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
2025-06-11 19:56 ` [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper Michal Luczaj
@ 2025-06-11 19:56 ` Michal Luczaj
2025-06-13 15:58 ` Luigi Leonardi
2025-06-17 14:12 ` Stefano Garzarella
2025-06-11 19:56 ` [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test Michal Luczaj
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Michal Luczaj @ 2025-06-11 19:56 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Return a bitmap of registered vsock transports. As guesstimated by grepping
/proc/kallsyms (CONFIG_KALLSYMS=y) for known symbols of type `struct
vsock_transport`, or `struct virtio_transport` in case the vsock_transport
is embedded within.
Note that the way `enum transport` and `transport_ksyms[]` are defined
triggers checkpatch.pl:
util.h:11: ERROR: Macros with complex values should be enclosed in parentheses
util.h:20: ERROR: Macros with complex values should be enclosed in parentheses
util.h:20: WARNING: Argument 'symbol' is not used in function-like macro
util.h:28: WARNING: Argument 'name' is not used in function-like macro
While commit 15d4734c7a58 ("checkpatch: qualify do-while-0 advice")
suggests it is known that the ERRORs heuristics are insufficient, I can not
find many other places where preprocessor is used in this
checkpatch-unhappy fashion. Notable exception being bcachefs, e.g.
fs/bcachefs/alloc_background_format.h. WARNINGs regarding unused macro
arguments seem more common, e.g. __ASM_SEL in arch/x86/include/asm/asm.h.
In other words, this might be unnecessarily complex. The same can be
achieved by just telling human to keep the order:
enum transport {
TRANSPORT_LOOPBACK = BIT(0),
TRANSPORT_VIRTIO = BIT(1),
TRANSPORT_VHOST = BIT(2),
TRANSPORT_VMCI = BIT(3),
TRANSPORT_HYPERV = BIT(4),
TRANSPORT_NUM = 5,
};
#define KSYM_ENTRY(sym) "d " sym "_transport"
/* Keep `enum transport` order */
static const char * const transport_ksyms[] = {
KSYM_ENTRY("loopback"),
KSYM_ENTRY("virtio"),
KSYM_ENTRY("vhost"),
KSYM_ENTRY("vmci"),
KSYM_ENTRY("vhs"),
};
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
tools/testing/vsock/util.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
tools/testing/vsock/util.h | 29 ++++++++++++++++++++++++
2 files changed, 85 insertions(+)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index b7b3fb2221c1682ecde58cf12e2f0b0ded1cff39..803f1e075b62228c25f9dffa1eff131b8072a06a 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -7,6 +7,7 @@
* Author: Stefan Hajnoczi <stefanha@redhat.com>
*/
+#include <ctype.h>
#include <errno.h>
#include <stdio.h>
#include <stdint.h>
@@ -23,6 +24,9 @@
#include "control.h"
#include "util.h"
+#define KALLSYMS_PATH "/proc/kallsyms"
+#define KALLSYMS_LINE_LEN 512
+
/* Install signal handlers */
void init_signals(void)
{
@@ -854,3 +858,55 @@ void enable_so_linger(int fd, int timeout)
exit(EXIT_FAILURE);
}
}
+
+static int __get_transports(void)
+{
+ char buf[KALLSYMS_LINE_LEN];
+ const char *ksym;
+ int ret = 0;
+ FILE *f;
+
+ f = fopen(KALLSYMS_PATH, "r");
+ if (!f) {
+ perror("Can't open " KALLSYMS_PATH);
+ exit(EXIT_FAILURE);
+ }
+
+ while (fgets(buf, sizeof(buf), f)) {
+ char *match;
+ int i;
+
+ assert(buf[strlen(buf) - 1] == '\n');
+
+ for (i = 0; i < TRANSPORT_NUM; ++i) {
+ if (ret & BIT(i))
+ continue;
+
+ /* Match should be followed by '\t' or '\n'.
+ * See kallsyms.c:s_show().
+ */
+ ksym = transport_ksyms[i];
+ match = strstr(buf, ksym);
+ if (match && isspace(match[strlen(ksym)])) {
+ ret |= BIT(i);
+ break;
+ }
+ }
+ }
+
+ fclose(f);
+ return ret;
+}
+
+/* Return integer with TRANSPORT_* bit set for every (known) registered vsock
+ * transport.
+ */
+int get_transports(void)
+{
+ static int tr = -1;
+
+ if (tr == -1)
+ tr = __get_transports();
+
+ return tr;
+}
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 0afe7cbae12e5194172c639ccfbeb8b81f7c25ac..71895192cc02313bf52784e2f77aa3b0c28a0c94 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -3,8 +3,36 @@
#define UTIL_H
#include <sys/socket.h>
+#include <linux/bitops.h>
+#include <linux/kernel.h>
#include <linux/vm_sockets.h>
+/* All known vsock transports, see callers of vsock_core_register() */
+#define KNOWN_TRANSPORTS(x) \
+ x(LOOPBACK, "loopback") \
+ x(VIRTIO, "virtio") \
+ x(VHOST, "vhost") \
+ x(VMCI, "vmci") \
+ x(HYPERV, "hvs")
+
+enum transport {
+ TRANSPORT_COUNTER_BASE = __COUNTER__ + 1,
+ #define x(name, symbol) \
+ TRANSPORT_##name = BIT(__COUNTER__ - TRANSPORT_COUNTER_BASE),
+ KNOWN_TRANSPORTS(x)
+ TRANSPORT_NUM = __COUNTER__ - TRANSPORT_COUNTER_BASE,
+ #undef x
+};
+
+static const char * const transport_ksyms[] = {
+ #define x(name, symbol) "d " symbol "_transport",
+ KNOWN_TRANSPORTS(x)
+ #undef x
+};
+
+static_assert(ARRAY_SIZE(transport_ksyms) == TRANSPORT_NUM);
+static_assert(BITS_PER_TYPE(int) >= TRANSPORT_NUM);
+
/* Tests can either run as the client or the server */
enum test_mode {
TEST_MODE_UNSET,
@@ -82,4 +110,5 @@ void setsockopt_timeval_check(int fd, int level, int optname,
struct timeval val, char const *errmsg);
void enable_so_zerocopy_check(int fd);
void enable_so_linger(int fd, int timeout);
+int get_transports(void);
#endif /* UTIL_H */
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
2025-06-11 19:56 ` [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper Michal Luczaj
2025-06-11 19:56 ` [PATCH net-next v3 2/3] vsock/test: Introduce get_transports() Michal Luczaj
@ 2025-06-11 19:56 ` Michal Luczaj
2025-06-17 14:13 ` Stefano Garzarella
2025-06-16 21:57 ` [PATCH net-next v3 0/3] vsock/test: Improve " Jakub Kicinski
2025-06-17 22:20 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 11+ messages in thread
From: Michal Luczaj @ 2025-06-11 19:56 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: virtualization, netdev, linux-kernel, Michal Luczaj
Increase the coverage of test for UAF due to socket unbinding, and losing
transport in general. It's a follow up to commit 301a62dfb0d0 ("vsock/test:
Add test for UAF due to socket unbinding") and discussion in [1].
The idea remains the same: take an unconnected stream socket with a
transport assigned and then attempt to switch the transport by trying (and
failing) to connect to some other CID. Now do this iterating over all the
well known CIDs (plus one).
While at it, drop the redundant synchronization between client and server.
Some single-transport setups can't be tested effectively; a warning is
issued. Depending on transports available, a variety of splats are possible
on unpatched machines. After reverting commit 78dafe1cf3af ("vsock: Orphan
socket after transport release") and commit fcdd2242c023 ("vsock: Keep the
binding until socket destruction"):
BUG: KASAN: slab-use-after-free in __vsock_bind+0x61f/0x720
Read of size 4 at addr ffff88811ff46b54 by task vsock_test/1475
Call Trace:
dump_stack_lvl+0x68/0x90
print_report+0x170/0x53d
kasan_report+0xc2/0x180
__vsock_bind+0x61f/0x720
vsock_connect+0x727/0xc40
__sys_connect+0xe8/0x100
__x64_sys_connect+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
WARNING: CPU: 0 PID: 1475 at net/vmw_vsock/virtio_transport_common.c:37 virtio_transport_send_pkt_info+0xb2b/0x1160
Call Trace:
virtio_transport_connect+0x90/0xb0
vsock_connect+0x782/0xc40
__sys_connect+0xe8/0x100
__x64_sys_connect+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
RIP: 0010:sock_has_perm+0xa7/0x2a0
Call Trace:
selinux_socket_connect_helper.isra.0+0xbc/0x450
selinux_socket_connect+0x3b/0x70
security_socket_connect+0x31/0xd0
__sys_connect_file+0x79/0x1f0
__sys_connect+0xe8/0x100
__x64_sys_connect+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 7 PID: 1518 at lib/refcount.c:25 refcount_warn_saturate+0xdd/0x140
RIP: 0010:refcount_warn_saturate+0xdd/0x140
Call Trace:
__vsock_bind+0x65e/0x720
vsock_connect+0x727/0xc40
__sys_connect+0xe8/0x100
__x64_sys_connect+0x6e/0xc0
do_syscall_64+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 1475 at lib/refcount.c:28 refcount_warn_saturate+0x12b/0x140
RIP: 0010:refcount_warn_saturate+0x12b/0x140
Call Trace:
vsock_remove_bound+0x18f/0x280
__vsock_release+0x371/0x480
vsock_release+0x88/0x120
__sock_release+0xaa/0x260
sock_close+0x14/0x20
__fput+0x35a/0xaa0
task_work_run+0xff/0x1c0
do_exit+0x849/0x24c0
make_task_dead+0xf3/0x110
rewind_stack_and_make_dead+0x16/0x20
[1]: https://lore.kernel.org/netdev/CAGxU2F5zhfWymY8u0hrKksW8PumXAYz-9_qRmW==92oAx1BX3g@mail.gmail.com/
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
tools/testing/vsock/vsock_test.c | 93 ++++++++++++++++++++++++++++++++--------
1 file changed, 74 insertions(+), 19 deletions(-)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index f669baaa0dca3bebc678d00eafa80857d1f0fdd6..eb6f54378667ac7ed324f4823e988ec9846e41a3 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -1718,16 +1718,27 @@ static void test_stream_msgzcopy_leak_zcskb_server(const struct test_opts *opts)
#define MAX_PORT_RETRIES 24 /* net/vmw_vsock/af_vsock.c */
-/* Test attempts to trigger a transport release for an unbound socket. This can
- * lead to a reference count mishandling.
- */
-static void test_stream_transport_uaf_client(const struct test_opts *opts)
+static bool test_stream_transport_uaf(int cid)
{
int sockets[MAX_PORT_RETRIES];
struct sockaddr_vm addr;
- int fd, i, alen;
+ socklen_t alen;
+ int fd, i, c;
+ bool ret;
+
+ /* Probe for a transport by attempting a local CID bind. Unavailable
+ * transport (or more specifically: an unsupported transport/CID
+ * combination) results in EADDRNOTAVAIL, other errnos are fatal.
+ */
+ fd = vsock_bind_try(cid, VMADDR_PORT_ANY, SOCK_STREAM);
+ if (fd < 0) {
+ if (errno != EADDRNOTAVAIL) {
+ perror("Unexpected bind() errno");
+ exit(EXIT_FAILURE);
+ }
- fd = vsock_bind(VMADDR_CID_ANY, VMADDR_PORT_ANY, SOCK_STREAM);
+ return false;
+ }
alen = sizeof(addr);
if (getsockname(fd, (struct sockaddr *)&addr, &alen)) {
@@ -1735,38 +1746,83 @@ static void test_stream_transport_uaf_client(const struct test_opts *opts)
exit(EXIT_FAILURE);
}
+ /* Drain the autobind pool; see __vsock_bind_connectible(). */
for (i = 0; i < MAX_PORT_RETRIES; ++i)
- sockets[i] = vsock_bind(VMADDR_CID_ANY, ++addr.svm_port,
- SOCK_STREAM);
+ sockets[i] = vsock_bind(cid, ++addr.svm_port, SOCK_STREAM);
close(fd);
- fd = socket(AF_VSOCK, SOCK_STREAM, 0);
+
+ /* Setting SOCK_NONBLOCK makes connect() return soon after
+ * (re-)assigning the transport. We are not connecting to anything
+ * anyway, so there is no point entering the main loop in
+ * vsock_connect(); waiting for timeout, checking for signals, etc.
+ */
+ fd = socket(AF_VSOCK, SOCK_STREAM | SOCK_NONBLOCK, 0);
if (fd < 0) {
perror("socket");
exit(EXIT_FAILURE);
}
- if (!vsock_connect_fd(fd, addr.svm_cid, addr.svm_port)) {
- perror("Unexpected connect() #1 success");
+ /* Assign transport, while failing to autobind. Autobind pool was
+ * drained, so EADDRNOTAVAIL coming from __vsock_bind_connectible() is
+ * expected.
+ *
+ * One exception is ENODEV which is thrown by vsock_assign_transport(),
+ * i.e. before vsock_auto_bind(), when the only transport loaded is
+ * vhost.
+ */
+ if (!connect(fd, (struct sockaddr *)&addr, alen)) {
+ fprintf(stderr, "Unexpected connect() success\n");
exit(EXIT_FAILURE);
}
-
- /* Vulnerable system may crash now. */
- if (!vsock_connect_fd(fd, VMADDR_CID_HOST, VMADDR_PORT_ANY)) {
- perror("Unexpected connect() #2 success");
+ if (errno == ENODEV && cid == VMADDR_CID_HOST) {
+ ret = false;
+ goto cleanup;
+ }
+ if (errno != EADDRNOTAVAIL) {
+ perror("Unexpected connect() errno");
exit(EXIT_FAILURE);
}
+ /* Reassign transport, triggering old transport release and
+ * (potentially) unbinding of an unbound socket.
+ *
+ * Vulnerable system may crash now.
+ */
+ for (c = VMADDR_CID_HYPERVISOR; c <= VMADDR_CID_HOST + 1; ++c) {
+ if (c != cid) {
+ addr.svm_cid = c;
+ (void)connect(fd, (struct sockaddr *)&addr, alen);
+ }
+ }
+
+ ret = true;
+cleanup:
close(fd);
while (i--)
close(sockets[i]);
- control_writeln("DONE");
+ return ret;
}
-static void test_stream_transport_uaf_server(const struct test_opts *opts)
+/* Test attempts to trigger a transport release for an unbound socket. This can
+ * lead to a reference count mishandling.
+ */
+static void test_stream_transport_uaf_client(const struct test_opts *opts)
{
- control_expectln("DONE");
+ bool tested = false;
+ int cid, tr;
+
+ for (cid = VMADDR_CID_HYPERVISOR; cid <= VMADDR_CID_HOST + 1; ++cid)
+ tested |= test_stream_transport_uaf(cid);
+
+ tr = get_transports();
+ if (!tr)
+ fprintf(stderr, "No transports detected\n");
+ else if (tr == TRANSPORT_VIRTIO)
+ fprintf(stderr, "Setup unsupported: sole virtio transport\n");
+ else if (!tested)
+ fprintf(stderr, "No transports tested\n");
}
static void test_stream_connect_retry_client(const struct test_opts *opts)
@@ -2034,7 +2090,6 @@ static struct test_case test_cases[] = {
{
.name = "SOCK_STREAM transport release use-after-free",
.run_client = test_stream_transport_uaf_client,
- .run_server = test_stream_transport_uaf_server,
},
{
.name = "SOCK_STREAM retry failed connect()",
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper
2025-06-11 19:56 ` [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper Michal Luczaj
@ 2025-06-13 15:03 ` Luigi Leonardi
0 siblings, 0 replies; 11+ messages in thread
From: Luigi Leonardi @ 2025-06-13 15:03 UTC (permalink / raw)
To: Michal Luczaj; +Cc: Stefano Garzarella, virtualization, netdev, linux-kernel
On Wed, Jun 11, 2025 at 09:56:50PM +0200, Michal Luczaj wrote:
>Create a socket and bind() it. If binding failed, gracefully return an
>error code while preserving `errno`.
>
>Base vsock_bind() on top of it.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/util.c | 24 +++++++++++++++++++++---
> tools/testing/vsock/util.h | 1 +
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 0c7e9cbcbc85cde9c8764fc3bb623cde2f6c77a6..b7b3fb2221c1682ecde58cf12e2f0b0ded1cff39 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -121,15 +121,17 @@ bool vsock_wait_sent(int fd)
> return !ret;
> }
>
>-/* Create socket <type>, bind to <cid, port> and return the file descriptor. */
>-int vsock_bind(unsigned int cid, unsigned int port, int type)
>+/* Create socket <type>, bind to <cid, port>.
>+ * Return the file descriptor, or -1 on error.
>+ */
>+int vsock_bind_try(unsigned int cid, unsigned int port, int type)
> {
> struct sockaddr_vm sa = {
> .svm_family = AF_VSOCK,
> .svm_cid = cid,
> .svm_port = port,
> };
>- int fd;
>+ int fd, saved_errno;
>
> fd = socket(AF_VSOCK, type, 0);
> if (fd < 0) {
>@@ -138,6 +140,22 @@ int vsock_bind(unsigned int cid, unsigned int port, int type)
> }
>
> if (bind(fd, (struct sockaddr *)&sa, sizeof(sa))) {
>+ saved_errno = errno;
>+ close(fd);
>+ errno = saved_errno;
>+ fd = -1;
>+ }
>+
>+ return fd;
>+}
>+
>+/* Create socket <type>, bind to <cid, port> and return the file descriptor. */
>+int vsock_bind(unsigned int cid, unsigned int port, int type)
>+{
>+ int fd;
>+
>+ fd = vsock_bind_try(cid, port, type);
>+ if (fd < 0) {
> perror("bind");
> exit(EXIT_FAILURE);
> }
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index 5e2db67072d5053804a9bb93934b625ea78bcd7a..0afe7cbae12e5194172c639ccfbeb8b81f7c25ac 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -44,6 +44,7 @@ int vsock_connect(unsigned int cid, unsigned int port, int type);
> int vsock_accept(unsigned int cid, unsigned int port,
> struct sockaddr_vm *clientaddrp, int type);
> int vsock_stream_connect(unsigned int cid, unsigned int port);
>+int vsock_bind_try(unsigned int cid, unsigned int port, int type);
> int vsock_bind(unsigned int cid, unsigned int port, int type);
> int vsock_bind_connect(unsigned int cid, unsigned int port,
> unsigned int bind_port, int type);
>
>--
>2.49.0
>
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 2/3] vsock/test: Introduce get_transports()
2025-06-11 19:56 ` [PATCH net-next v3 2/3] vsock/test: Introduce get_transports() Michal Luczaj
@ 2025-06-13 15:58 ` Luigi Leonardi
2025-06-17 14:12 ` Stefano Garzarella
1 sibling, 0 replies; 11+ messages in thread
From: Luigi Leonardi @ 2025-06-13 15:58 UTC (permalink / raw)
To: Michal Luczaj; +Cc: Stefano Garzarella, virtualization, netdev, linux-kernel
Hi Michal,
On Wed, Jun 11, 2025 at 09:56:51PM +0200, Michal Luczaj wrote:
>Return a bitmap of registered vsock transports. As guesstimated by
>grepping
>/proc/kallsyms (CONFIG_KALLSYMS=y) for known symbols of type `struct
>vsock_transport`, or `struct virtio_transport` in case the
>vsock_transport
>is embedded within.
>
>Note that the way `enum transport` and `transport_ksyms[]` are defined
>triggers checkpatch.pl:
>
>util.h:11: ERROR: Macros with complex values should be enclosed in
>parentheses
>util.h:20: ERROR: Macros with complex values should be enclosed in
>parentheses
>util.h:20: WARNING: Argument 'symbol' is not used in function-like
>macro
>util.h:28: WARNING: Argument 'name' is not used in function-like macro
>
>While commit 15d4734c7a58 ("checkpatch: qualify do-while-0 advice")
>suggests it is known that the ERRORs heuristics are insufficient, I can
>not
>find many other places where preprocessor is used in this
>checkpatch-unhappy fashion. Notable exception being bcachefs, e.g.
>fs/bcachefs/alloc_background_format.h. WARNINGs regarding unused macro
>arguments seem more common, e.g. __ASM_SEL in
>arch/x86/include/asm/asm.h.
>
>In other words, this might be unnecessarily complex. The same can be
>achieved by just telling human to keep the order:
>
>enum transport {
> TRANSPORT_LOOPBACK = BIT(0),
> TRANSPORT_VIRTIO = BIT(1),
> TRANSPORT_VHOST = BIT(2),
> TRANSPORT_VMCI = BIT(3),
> TRANSPORT_HYPERV = BIT(4),
> TRANSPORT_NUM = 5,
>};
>
> #define KSYM_ENTRY(sym) "d " sym "_transport"
>
>/* Keep `enum transport` order */
>static const char * const transport_ksyms[] = {
> KSYM_ENTRY("loopback"),
> KSYM_ENTRY("virtio"),
> KSYM_ENTRY("vhost"),
> KSYM_ENTRY("vmci"),
> KSYM_ENTRY("vhs"),
>};
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/util.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/testing/vsock/util.h | 29 ++++++++++++++++++++++++
> 2 files changed, 85 insertions(+)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index b7b3fb2221c1682ecde58cf12e2f0b0ded1cff39..803f1e075b62228c25f9dffa1eff131b8072a06a 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -7,6 +7,7 @@
> * Author: Stefan Hajnoczi <stefanha@redhat.com>
> */
>
>+#include <ctype.h>
> #include <errno.h>
> #include <stdio.h>
> #include <stdint.h>
>@@ -23,6 +24,9 @@
> #include "control.h"
> #include "util.h"
>
>+#define KALLSYMS_PATH "/proc/kallsyms"
>+#define KALLSYMS_LINE_LEN 512
>+
> /* Install signal handlers */
> void init_signals(void)
> {
>@@ -854,3 +858,55 @@ void enable_so_linger(int fd, int timeout)
> exit(EXIT_FAILURE);
> }
> }
>+
>+static int __get_transports(void)
>+{
>+ char buf[KALLSYMS_LINE_LEN];
>+ const char *ksym;
>+ int ret = 0;
>+ FILE *f;
>+
>+ f = fopen(KALLSYMS_PATH, "r");
>+ if (!f) {
>+ perror("Can't open " KALLSYMS_PATH);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ while (fgets(buf, sizeof(buf), f)) {
>+ char *match;
>+ int i;
>+
>+ assert(buf[strlen(buf) - 1] == '\n');
>+
>+ for (i = 0; i < TRANSPORT_NUM; ++i) {
>+ if (ret & BIT(i))
>+ continue;
>+
>+ /* Match should be followed by '\t' or '\n'.
>+ * See kallsyms.c:s_show().
>+ */
>+ ksym = transport_ksyms[i];
>+ match = strstr(buf, ksym);
>+ if (match && isspace(match[strlen(ksym)])) {
>+ ret |= BIT(i);
>+ break;
>+ }
>+ }
>+ }
>+
>+ fclose(f);
>+ return ret;
>+}
>+
>+/* Return integer with TRANSPORT_* bit set for every (known) registered vsock
>+ * transport.
>+ */
>+int get_transports(void)
>+{
>+ static int tr = -1;
>+
>+ if (tr == -1)
>+ tr = __get_transports();
>+
>+ return tr;
>+}
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index 0afe7cbae12e5194172c639ccfbeb8b81f7c25ac..71895192cc02313bf52784e2f77aa3b0c28a0c94 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -3,8 +3,36 @@
> #define UTIL_H
>
> #include <sys/socket.h>
>+#include <linux/bitops.h>
>+#include <linux/kernel.h>
> #include <linux/vm_sockets.h>
>
>+/* All known vsock transports, see callers of vsock_core_register() */
>+#define KNOWN_TRANSPORTS(x) \
>+ x(LOOPBACK, "loopback") \
>+ x(VIRTIO, "virtio") \
>+ x(VHOST, "vhost") \
>+ x(VMCI, "vmci") \
>+ x(HYPERV, "hvs")
>+
>+enum transport {
>+ TRANSPORT_COUNTER_BASE = __COUNTER__ + 1,
>+ #define x(name, symbol) \
>+ TRANSPORT_##name = BIT(__COUNTER__ - TRANSPORT_COUNTER_BASE),
>+ KNOWN_TRANSPORTS(x)
>+ TRANSPORT_NUM = __COUNTER__ - TRANSPORT_COUNTER_BASE,
>+ #undef x
>+};
>+
>+static const char * const transport_ksyms[] = {
>+ #define x(name, symbol) "d " symbol "_transport",
>+ KNOWN_TRANSPORTS(x)
>+ #undef x
>+};
>+
>+static_assert(ARRAY_SIZE(transport_ksyms) == TRANSPORT_NUM);
>+static_assert(BITS_PER_TYPE(int) >= TRANSPORT_NUM);
>+
> /* Tests can either run as the client or the server */
> enum test_mode {
> TEST_MODE_UNSET,
>@@ -82,4 +110,5 @@ void setsockopt_timeval_check(int fd, int level, int optname,
> struct timeval val, char const *errmsg);
> void enable_so_zerocopy_check(int fd);
> void enable_so_linger(int fd, int timeout);
>+int get_transports(void);
> #endif /* UTIL_H */
>
>--
>2.49.0
>
Checked the code and tested `get_transports()`. It works as expected!
I'm not sure about the `checkpatch.pl` errors, but code LGTM to me.
Tested-by: Luigi Leonardi <leonardi@redhat.com>
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Thanks!
Luigi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
` (2 preceding siblings ...)
2025-06-11 19:56 ` [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test Michal Luczaj
@ 2025-06-16 21:57 ` Jakub Kicinski
2025-06-17 8:25 ` Stefano Garzarella
2025-06-17 22:20 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2025-06-16 21:57 UTC (permalink / raw)
To: Michal Luczaj; +Cc: Stefano Garzarella, virtualization, netdev, linux-kernel
On Wed, 11 Jun 2025 21:56:49 +0200 Michal Luczaj wrote:
> Increase the coverage of a test implemented in commit 301a62dfb0d0
> ("vsock/test: Add test for UAF due to socket unbinding"). Take this
> opportunity to factor out some utility code, drop a redundant sync between
> client and server, and introduce a /proc/kallsyms harvesting logic for
> auto-detecting registered vsock transports.
Hi Stefano! Sorry to ping, are these on your radar?
I'm wondering if the delay is because of devconf.cz or you consider
the Suggested-by tags a strong enough indication of support :)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test
2025-06-16 21:57 ` [PATCH net-next v3 0/3] vsock/test: Improve " Jakub Kicinski
@ 2025-06-17 8:25 ` Stefano Garzarella
0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2025-06-17 8:25 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Michal Luczaj, virtualization, netdev, linux-kernel
On Mon, Jun 16, 2025 at 02:57:29PM -0700, Jakub Kicinski wrote:
>On Wed, 11 Jun 2025 21:56:49 +0200 Michal Luczaj wrote:
>> Increase the coverage of a test implemented in commit 301a62dfb0d0
>> ("vsock/test: Add test for UAF due to socket unbinding"). Take this
>> opportunity to factor out some utility code, drop a redundant sync between
>> client and server, and introduce a /proc/kallsyms harvesting logic for
>> auto-detecting registered vsock transports.
>
>Hi Stefano! Sorry to ping, are these on your radar?
>I'm wondering if the delay is because of devconf.cz or you consider
>the Suggested-by tags a strong enough indication of support :)
>
Yeah, it's because devconf.cz. I'm going to review today, thanks for the
reminder :-)
Stefano
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 2/3] vsock/test: Introduce get_transports()
2025-06-11 19:56 ` [PATCH net-next v3 2/3] vsock/test: Introduce get_transports() Michal Luczaj
2025-06-13 15:58 ` Luigi Leonardi
@ 2025-06-17 14:12 ` Stefano Garzarella
1 sibling, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2025-06-17 14:12 UTC (permalink / raw)
To: Michal Luczaj; +Cc: virtualization, netdev, linux-kernel
On Wed, Jun 11, 2025 at 09:56:51PM +0200, Michal Luczaj wrote:
>Return a bitmap of registered vsock transports. As guesstimated by grepping
>/proc/kallsyms (CONFIG_KALLSYMS=y) for known symbols of type `struct
>vsock_transport`, or `struct virtio_transport` in case the vsock_transport
>is embedded within.
>
>Note that the way `enum transport` and `transport_ksyms[]` are defined
>triggers checkpatch.pl:
>
>util.h:11: ERROR: Macros with complex values should be enclosed in parentheses
>util.h:20: ERROR: Macros with complex values should be enclosed in parentheses
>util.h:20: WARNING: Argument 'symbol' is not used in function-like macro
>util.h:28: WARNING: Argument 'name' is not used in function-like macro
>
>While commit 15d4734c7a58 ("checkpatch: qualify do-while-0 advice")
>suggests it is known that the ERRORs heuristics are insufficient, I can not
>find many other places where preprocessor is used in this
>checkpatch-unhappy fashion. Notable exception being bcachefs, e.g.
>fs/bcachefs/alloc_background_format.h. WARNINGs regarding unused macro
>arguments seem more common, e.g. __ASM_SEL in arch/x86/include/asm/asm.h.
>
>In other words, this might be unnecessarily complex. The same can be
>achieved by just telling human to keep the order:
>
>enum transport {
> TRANSPORT_LOOPBACK = BIT(0),
> TRANSPORT_VIRTIO = BIT(1),
> TRANSPORT_VHOST = BIT(2),
> TRANSPORT_VMCI = BIT(3),
> TRANSPORT_HYPERV = BIT(4),
> TRANSPORT_NUM = 5,
>};
>
> #define KSYM_ENTRY(sym) "d " sym "_transport"
>
>/* Keep `enum transport` order */
>static const char * const transport_ksyms[] = {
> KSYM_ENTRY("loopback"),
> KSYM_ENTRY("virtio"),
> KSYM_ENTRY("vhost"),
> KSYM_ENTRY("vmci"),
> KSYM_ENTRY("vhs"),
>};
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/util.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/testing/vsock/util.h | 29 ++++++++++++++++++++++++
> 2 files changed, 85 insertions(+)
LGTM!
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index b7b3fb2221c1682ecde58cf12e2f0b0ded1cff39..803f1e075b62228c25f9dffa1eff131b8072a06a 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -7,6 +7,7 @@
> * Author: Stefan Hajnoczi <stefanha@redhat.com>
> */
>
>+#include <ctype.h>
> #include <errno.h>
> #include <stdio.h>
> #include <stdint.h>
>@@ -23,6 +24,9 @@
> #include "control.h"
> #include "util.h"
>
>+#define KALLSYMS_PATH "/proc/kallsyms"
>+#define KALLSYMS_LINE_LEN 512
>+
> /* Install signal handlers */
> void init_signals(void)
> {
>@@ -854,3 +858,55 @@ void enable_so_linger(int fd, int timeout)
> exit(EXIT_FAILURE);
> }
> }
>+
>+static int __get_transports(void)
>+{
>+ char buf[KALLSYMS_LINE_LEN];
>+ const char *ksym;
>+ int ret = 0;
>+ FILE *f;
>+
>+ f = fopen(KALLSYMS_PATH, "r");
>+ if (!f) {
>+ perror("Can't open " KALLSYMS_PATH);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ while (fgets(buf, sizeof(buf), f)) {
>+ char *match;
>+ int i;
>+
>+ assert(buf[strlen(buf) - 1] == '\n');
>+
>+ for (i = 0; i < TRANSPORT_NUM; ++i) {
>+ if (ret & BIT(i))
>+ continue;
>+
>+ /* Match should be followed by '\t' or '\n'.
>+ * See kallsyms.c:s_show().
>+ */
>+ ksym = transport_ksyms[i];
>+ match = strstr(buf, ksym);
>+ if (match && isspace(match[strlen(ksym)])) {
>+ ret |= BIT(i);
>+ break;
>+ }
>+ }
>+ }
>+
>+ fclose(f);
>+ return ret;
>+}
>+
>+/* Return integer with TRANSPORT_* bit set for every (known) registered vsock
>+ * transport.
>+ */
>+int get_transports(void)
>+{
>+ static int tr = -1;
>+
>+ if (tr == -1)
>+ tr = __get_transports();
>+
>+ return tr;
>+}
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index 0afe7cbae12e5194172c639ccfbeb8b81f7c25ac..71895192cc02313bf52784e2f77aa3b0c28a0c94 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -3,8 +3,36 @@
> #define UTIL_H
>
> #include <sys/socket.h>
>+#include <linux/bitops.h>
>+#include <linux/kernel.h>
> #include <linux/vm_sockets.h>
>
>+/* All known vsock transports, see callers of vsock_core_register() */
>+#define KNOWN_TRANSPORTS(x) \
>+ x(LOOPBACK, "loopback") \
>+ x(VIRTIO, "virtio") \
>+ x(VHOST, "vhost") \
>+ x(VMCI, "vmci") \
>+ x(HYPERV, "hvs")
>+
>+enum transport {
>+ TRANSPORT_COUNTER_BASE = __COUNTER__ + 1,
>+ #define x(name, symbol) \
>+ TRANSPORT_##name = BIT(__COUNTER__ - TRANSPORT_COUNTER_BASE),
>+ KNOWN_TRANSPORTS(x)
>+ TRANSPORT_NUM = __COUNTER__ - TRANSPORT_COUNTER_BASE,
>+ #undef x
>+};
>+
>+static const char * const transport_ksyms[] = {
>+ #define x(name, symbol) "d " symbol "_transport",
>+ KNOWN_TRANSPORTS(x)
>+ #undef x
>+};
>+
>+static_assert(ARRAY_SIZE(transport_ksyms) == TRANSPORT_NUM);
>+static_assert(BITS_PER_TYPE(int) >= TRANSPORT_NUM);
>+
> /* Tests can either run as the client or the server */
> enum test_mode {
> TEST_MODE_UNSET,
>@@ -82,4 +110,5 @@ void setsockopt_timeval_check(int fd, int level, int optname,
> struct timeval val, char const *errmsg);
> void enable_so_zerocopy_check(int fd);
> void enable_so_linger(int fd, int timeout);
>+int get_transports(void);
> #endif /* UTIL_H */
>
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test
2025-06-11 19:56 ` [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test Michal Luczaj
@ 2025-06-17 14:13 ` Stefano Garzarella
0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2025-06-17 14:13 UTC (permalink / raw)
To: Michal Luczaj; +Cc: virtualization, netdev, linux-kernel
On Wed, Jun 11, 2025 at 09:56:52PM +0200, Michal Luczaj wrote:
>Increase the coverage of test for UAF due to socket unbinding, and losing
>transport in general. It's a follow up to commit 301a62dfb0d0 ("vsock/test:
>Add test for UAF due to socket unbinding") and discussion in [1].
>
>The idea remains the same: take an unconnected stream socket with a
>transport assigned and then attempt to switch the transport by trying (and
>failing) to connect to some other CID. Now do this iterating over all the
>well known CIDs (plus one).
>
>While at it, drop the redundant synchronization between client and server.
>
>Some single-transport setups can't be tested effectively; a warning is
>issued. Depending on transports available, a variety of splats are possible
>on unpatched machines. After reverting commit 78dafe1cf3af ("vsock: Orphan
>socket after transport release") and commit fcdd2242c023 ("vsock: Keep the
>binding until socket destruction"):
>
>BUG: KASAN: slab-use-after-free in __vsock_bind+0x61f/0x720
>Read of size 4 at addr ffff88811ff46b54 by task vsock_test/1475
>Call Trace:
> dump_stack_lvl+0x68/0x90
> print_report+0x170/0x53d
> kasan_report+0xc2/0x180
> __vsock_bind+0x61f/0x720
> vsock_connect+0x727/0xc40
> __sys_connect+0xe8/0x100
> __x64_sys_connect+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>WARNING: CPU: 0 PID: 1475 at net/vmw_vsock/virtio_transport_common.c:37 virtio_transport_send_pkt_info+0xb2b/0x1160
>Call Trace:
> virtio_transport_connect+0x90/0xb0
> vsock_connect+0x782/0xc40
> __sys_connect+0xe8/0x100
> __x64_sys_connect+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
>RIP: 0010:sock_has_perm+0xa7/0x2a0
>Call Trace:
> selinux_socket_connect_helper.isra.0+0xbc/0x450
> selinux_socket_connect+0x3b/0x70
> security_socket_connect+0x31/0xd0
> __sys_connect_file+0x79/0x1f0
> __sys_connect+0xe8/0x100
> __x64_sys_connect+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>refcount_t: addition on 0; use-after-free.
>WARNING: CPU: 7 PID: 1518 at lib/refcount.c:25 refcount_warn_saturate+0xdd/0x140
>RIP: 0010:refcount_warn_saturate+0xdd/0x140
>Call Trace:
> __vsock_bind+0x65e/0x720
> vsock_connect+0x727/0xc40
> __sys_connect+0xe8/0x100
> __x64_sys_connect+0x6e/0xc0
> do_syscall_64+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
>refcount_t: underflow; use-after-free.
>WARNING: CPU: 0 PID: 1475 at lib/refcount.c:28 refcount_warn_saturate+0x12b/0x140
>RIP: 0010:refcount_warn_saturate+0x12b/0x140
>Call Trace:
> vsock_remove_bound+0x18f/0x280
> __vsock_release+0x371/0x480
> vsock_release+0x88/0x120
> __sock_release+0xaa/0x260
> sock_close+0x14/0x20
> __fput+0x35a/0xaa0
> task_work_run+0xff/0x1c0
> do_exit+0x849/0x24c0
> make_task_dead+0xf3/0x110
> rewind_stack_and_make_dead+0x16/0x20
>
>[1]: https://lore.kernel.org/netdev/CAGxU2F5zhfWymY8u0hrKksW8PumXAYz-9_qRmW==92oAx1BX3g@mail.gmail.com/
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/vsock_test.c | 93 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 74 insertions(+), 19 deletions(-)
Thanks again for this!
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index f669baaa0dca3bebc678d00eafa80857d1f0fdd6..eb6f54378667ac7ed324f4823e988ec9846e41a3 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1718,16 +1718,27 @@ static void test_stream_msgzcopy_leak_zcskb_server(const struct test_opts *opts)
>
> #define MAX_PORT_RETRIES 24 /* net/vmw_vsock/af_vsock.c */
>
>-/* Test attempts to trigger a transport release for an unbound socket. This can
>- * lead to a reference count mishandling.
>- */
>-static void test_stream_transport_uaf_client(const struct test_opts *opts)
>+static bool test_stream_transport_uaf(int cid)
> {
> int sockets[MAX_PORT_RETRIES];
> struct sockaddr_vm addr;
>- int fd, i, alen;
>+ socklen_t alen;
>+ int fd, i, c;
>+ bool ret;
>+
>+ /* Probe for a transport by attempting a local CID bind. Unavailable
>+ * transport (or more specifically: an unsupported transport/CID
>+ * combination) results in EADDRNOTAVAIL, other errnos are fatal.
>+ */
>+ fd = vsock_bind_try(cid, VMADDR_PORT_ANY, SOCK_STREAM);
>+ if (fd < 0) {
>+ if (errno != EADDRNOTAVAIL) {
>+ perror("Unexpected bind() errno");
>+ exit(EXIT_FAILURE);
>+ }
>
>- fd = vsock_bind(VMADDR_CID_ANY, VMADDR_PORT_ANY, SOCK_STREAM);
>+ return false;
>+ }
>
> alen = sizeof(addr);
> if (getsockname(fd, (struct sockaddr *)&addr, &alen)) {
>@@ -1735,38 +1746,83 @@ static void test_stream_transport_uaf_client(const struct test_opts *opts)
> exit(EXIT_FAILURE);
> }
>
>+ /* Drain the autobind pool; see __vsock_bind_connectible(). */
> for (i = 0; i < MAX_PORT_RETRIES; ++i)
>- sockets[i] = vsock_bind(VMADDR_CID_ANY, ++addr.svm_port,
>- SOCK_STREAM);
>+ sockets[i] = vsock_bind(cid, ++addr.svm_port, SOCK_STREAM);
>
> close(fd);
>- fd = socket(AF_VSOCK, SOCK_STREAM, 0);
>+
>+ /* Setting SOCK_NONBLOCK makes connect() return soon after
>+ * (re-)assigning the transport. We are not connecting to anything
>+ * anyway, so there is no point entering the main loop in
>+ * vsock_connect(); waiting for timeout, checking for signals, etc.
>+ */
>+ fd = socket(AF_VSOCK, SOCK_STREAM | SOCK_NONBLOCK, 0);
> if (fd < 0) {
> perror("socket");
> exit(EXIT_FAILURE);
> }
>
>- if (!vsock_connect_fd(fd, addr.svm_cid, addr.svm_port)) {
>- perror("Unexpected connect() #1 success");
>+ /* Assign transport, while failing to autobind. Autobind pool was
>+ * drained, so EADDRNOTAVAIL coming from __vsock_bind_connectible() is
>+ * expected.
>+ *
>+ * One exception is ENODEV which is thrown by vsock_assign_transport(),
>+ * i.e. before vsock_auto_bind(), when the only transport loaded is
>+ * vhost.
>+ */
>+ if (!connect(fd, (struct sockaddr *)&addr, alen)) {
>+ fprintf(stderr, "Unexpected connect() success\n");
> exit(EXIT_FAILURE);
> }
>-
>- /* Vulnerable system may crash now. */
>- if (!vsock_connect_fd(fd, VMADDR_CID_HOST, VMADDR_PORT_ANY)) {
>- perror("Unexpected connect() #2 success");
>+ if (errno == ENODEV && cid == VMADDR_CID_HOST) {
>+ ret = false;
>+ goto cleanup;
>+ }
>+ if (errno != EADDRNOTAVAIL) {
>+ perror("Unexpected connect() errno");
> exit(EXIT_FAILURE);
> }
>
>+ /* Reassign transport, triggering old transport release and
>+ * (potentially) unbinding of an unbound socket.
>+ *
>+ * Vulnerable system may crash now.
>+ */
>+ for (c = VMADDR_CID_HYPERVISOR; c <= VMADDR_CID_HOST + 1; ++c) {
>+ if (c != cid) {
>+ addr.svm_cid = c;
>+ (void)connect(fd, (struct sockaddr *)&addr, alen);
>+ }
>+ }
>+
>+ ret = true;
>+cleanup:
> close(fd);
> while (i--)
> close(sockets[i]);
>
>- control_writeln("DONE");
>+ return ret;
> }
>
>-static void test_stream_transport_uaf_server(const struct test_opts *opts)
>+/* Test attempts to trigger a transport release for an unbound socket. This can
>+ * lead to a reference count mishandling.
>+ */
>+static void test_stream_transport_uaf_client(const struct test_opts *opts)
> {
>- control_expectln("DONE");
>+ bool tested = false;
>+ int cid, tr;
>+
>+ for (cid = VMADDR_CID_HYPERVISOR; cid <= VMADDR_CID_HOST + 1; ++cid)
>+ tested |= test_stream_transport_uaf(cid);
>+
>+ tr = get_transports();
>+ if (!tr)
>+ fprintf(stderr, "No transports detected\n");
>+ else if (tr == TRANSPORT_VIRTIO)
>+ fprintf(stderr, "Setup unsupported: sole virtio transport\n");
>+ else if (!tested)
>+ fprintf(stderr, "No transports tested\n");
> }
>
> static void test_stream_connect_retry_client(const struct test_opts *opts)
>@@ -2034,7 +2090,6 @@ static struct test_case test_cases[] = {
> {
> .name = "SOCK_STREAM transport release use-after-free",
> .run_client = test_stream_transport_uaf_client,
>- .run_server = test_stream_transport_uaf_server,
> },
> {
> .name = "SOCK_STREAM retry failed connect()",
>
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
` (3 preceding siblings ...)
2025-06-16 21:57 ` [PATCH net-next v3 0/3] vsock/test: Improve " Jakub Kicinski
@ 2025-06-17 22:20 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-17 22:20 UTC (permalink / raw)
To: Michal Luczaj; +Cc: sgarzare, virtualization, netdev, linux-kernel
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 11 Jun 2025 21:56:49 +0200 you wrote:
> Increase the coverage of a test implemented in commit 301a62dfb0d0
> ("vsock/test: Add test for UAF due to socket unbinding"). Take this
> opportunity to factor out some utility code, drop a redundant sync between
> client and server, and introduce a /proc/kallsyms harvesting logic for
> auto-detecting registered vsock transports.
>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>
> [...]
Here is the summary with links:
- [net-next,v3,1/3] vsock/test: Introduce vsock_bind_try() helper
https://git.kernel.org/netdev/net-next/c/d56a8dbff8fe
- [net-next,v3,2/3] vsock/test: Introduce get_transports()
https://git.kernel.org/netdev/net-next/c/3070c05b7afd
- [net-next,v3,3/3] vsock/test: Cover more CIDs in transport_uaf test
https://git.kernel.org/netdev/net-next/c/0cb6db139f39
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-17 22:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 19:56 [PATCH net-next v3 0/3] vsock/test: Improve transport_uaf test Michal Luczaj
2025-06-11 19:56 ` [PATCH net-next v3 1/3] vsock/test: Introduce vsock_bind_try() helper Michal Luczaj
2025-06-13 15:03 ` Luigi Leonardi
2025-06-11 19:56 ` [PATCH net-next v3 2/3] vsock/test: Introduce get_transports() Michal Luczaj
2025-06-13 15:58 ` Luigi Leonardi
2025-06-17 14:12 ` Stefano Garzarella
2025-06-11 19:56 ` [PATCH net-next v3 3/3] vsock/test: Cover more CIDs in transport_uaf test Michal Luczaj
2025-06-17 14:13 ` Stefano Garzarella
2025-06-16 21:57 ` [PATCH net-next v3 0/3] vsock/test: Improve " Jakub Kicinski
2025-06-17 8:25 ` Stefano Garzarella
2025-06-17 22:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).