[PATCH bpf-next 2/2] selftests/bpf: Add test for connect() racing sockmap update and signal

Michal Luczaj posted 2 patches 2 weeks, 3 days ago
[PATCH bpf-next 2/2] selftests/bpf: Add test for connect() racing sockmap update and signal
Posted by Michal Luczaj 2 weeks, 3 days ago
Attempt to trigger warnings/crashes by racing connect() against sockmap
updates and signals.

Follow-up to the discussion regarding af_vsock connect():
https://lore.kernel.org/netdev/20250311155601.eui5j2lta3q46i6u@gmail.com/

Suggested-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 .../bpf/prog_tests/sockmap_interrupted_connect.c   | 200 +++++++++++++++++++++
 1 file changed, 200 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
new file mode 100644
index 000000000000..aa48ae483dab
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <errno.h>
+#include <error.h>
+#include <stdio.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include <linux/string.h>
+#include <linux/time64.h>
+#include <linux/vm_sockets.h>
+
+#include "linux/const.h"
+#include "test_progs.h"
+#include "sockmap_helpers.h"
+
+#define STR(s)	#s
+#define XSTR(s)	STR(s)
+
+#define TIMEOUT	5	/* seconds */
+#define INVALID	0x4242
+
+struct socket_spec {
+	int domain;
+	int sotype;
+};
+
+struct context {
+	struct sockaddr_storage addr, bad;
+	struct socket_spec *ss;
+	char str[MAX_TEST_NAME];
+	socklen_t alen;
+	int s, c, map;
+};
+
+static void handler(int signum)
+{
+	/* nop */
+}
+
+static void *racer(void *arg)
+{
+	struct context *ctx = arg;
+	int *map = &ctx->map;
+	pid_t pid = getpid();
+
+	for (;;) {
+		if (kill(pid, SIGUSR1)) {
+			FAIL_ERRNO("kill");
+			break;
+		}
+
+		(void)bpf_map_update_elem(*map, &u32(0), &ctx->c, BPF_ANY);
+		pthread_testcancel();
+	}
+
+	return NULL;
+}
+
+static void test_reconnect(struct context *ctx)
+{
+	struct socket_spec *ss = ctx->ss;
+	__u64 tout;
+
+	tout = get_time_ns() + TIMEOUT * NSEC_PER_SEC;
+	do {
+		int c;
+
+		c = xsocket(ss->domain, ss->sotype, 0);
+		if (c < 0)
+			break;
+		ctx->c = c;
+
+		if ((ss->sotype & SOCK_TYPE_MASK) == SOCK_DGRAM) {
+			struct sockaddr_storage ca;
+			socklen_t len;
+
+			init_addr_loopback(ss->domain, &ca, &len);
+			if (xbind(c, sockaddr(&ca), len)) {
+				xclose(c);
+				break;
+			}
+		}
+
+		(void)connect(c, (struct sockaddr *)&ctx->addr, ctx->alen);
+		(void)connect(c, (struct sockaddr *)&ctx->bad, ctx->alen);
+		(void)recv(c, &(char){}, 1, MSG_DONTWAIT);
+
+		for (;;) {
+			int p = accept(ctx->s, NULL, NULL);
+
+			if (p < 0)
+				break;
+			xclose(p);
+		}
+
+		xclose(c);
+	} while (get_time_ns() < tout);
+}
+
+#define __TEST_RECONNECT_ADDR(addr_struct, mangle, mangle_s)			\
+	({									\
+		char str[MAX_TEST_NAME * 2];					\
+										\
+		memcpy(&ctx->bad, &ctx->addr, ctx->alen);			\
+		((struct addr_struct *)&ctx->bad)->mangle;			\
+										\
+		snprintf(str, sizeof(str), "%s %-24.24s ", ctx->str, mangle_s);	\
+		if (test__start_subtest(str))					\
+			test_reconnect(ctx);					\
+	})
+
+#define TEST_RECONNECT_ADDR(addr_struct, mangle)				\
+	__TEST_RECONNECT_ADDR(addr_struct, mangle, XSTR(mangle))
+
+static void test_socket(struct context *ctx)
+{
+	struct socket_spec *ss = ctx->ss;
+	socklen_t alen;
+	int s;
+
+	s = socket_loopback(ss->domain, ss->sotype | SOCK_NONBLOCK);
+	if (s < 0)
+		return;
+
+	alen = sizeof(ctx->addr);
+	if (xgetsockname(s, sockaddr(&ctx->addr), &alen))
+		goto cleanup;
+
+	ctx->s = s;
+	ctx->alen = alen;
+	sprintf(ctx->str + strlen(ctx->str), "%-5s ", socket_kind_to_str(s));
+
+	switch (ss->domain) {
+	case AF_UNIX:
+		TEST_RECONNECT_ADDR(sockaddr_un, sun_family = AF_UNSPEC);
+		TEST_RECONNECT_ADDR(sockaddr_un, sun_path[0] = (char)INVALID);
+		break;
+	case AF_VSOCK:
+		TEST_RECONNECT_ADDR(sockaddr_vm, svm_cid = INVALID);
+		break;
+	default:
+		FAIL("Unknown socket domain %#x", ss->domain);
+	}
+
+cleanup:
+	xclose(s);
+}
+
+static void test_map(struct context *ctx, enum bpf_map_type type)
+{
+	/* Filter by any `struct proto` that defines psock_update_sk_prot() */
+	struct socket_spec *ss, sockets[] = {
+		{ AF_UNIX, SOCK_STREAM },
+		{ AF_UNIX, SOCK_DGRAM },
+		// { AF_UNIX, SOCK_SEQPACKET },	/* see unix_dgram_bpf_update_proto() */
+		{ AF_VSOCK, SOCK_STREAM },
+		// { AF_VSOCK, SOCK_DGRAM },	/* see vsock_bpf_update_proto() */
+		{ AF_VSOCK, SOCK_SEQPACKET },
+	};
+
+	ctx->map = bpf_map_create(type, NULL, sizeof(int), sizeof(int), 1, NULL);
+	if (!ASSERT_OK_FD(ctx->map, "map"))
+		return;
+
+	for (ss = sockets; ss < sockets + ARRAY_SIZE(sockets); ss++) {
+		sprintf(ctx->str, "%-3s ",
+			type == BPF_MAP_TYPE_SOCKMAP ? "map" : "?");
+		ctx->ss = ss;
+
+		test_socket(ctx);
+	}
+
+	xclose(ctx->map);
+}
+
+void serial_test_sockmap_interrupted_connect(void)
+{
+	sighandler_t orig_handler;
+	struct context ctx = {0};
+	pthread_t tid;
+
+	orig_handler = signal(SIGUSR1, handler);
+	if (!ASSERT_NEQ(orig_handler, SIG_ERR, "signal"))
+		return;
+
+	if (xpthread_create(&tid, NULL, racer, &ctx))
+		goto restore;
+
+	test_map(&ctx, BPF_MAP_TYPE_SOCKMAP);
+
+	if (!xpthread_cancel(tid))
+		xpthread_join(tid, NULL);
+restore:
+	ASSERT_NEQ(signal(SIGUSR1, orig_handler), SIG_ERR, "handler restore");
+}

-- 
2.52.0
Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for connect() racing sockmap update and signal
Posted by Alexei Starovoitov 1 week, 6 days ago
On Fri, Jan 23, 2026 at 8:16 AM Michal Luczaj <mhal@rbox.co> wrote:
>
> Attempt to trigger warnings/crashes by racing connect() against sockmap
> updates and signals.
>
> Follow-up to the discussion regarding af_vsock connect():
> https://lore.kernel.org/netdev/20250311155601.eui5j2lta3q46i6u@gmail.com/
>
> Suggested-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
>  .../bpf/prog_tests/sockmap_interrupted_connect.c   | 200 +++++++++++++++++++++
>  1 file changed, 200 insertions(+)

Not sure why, but it seems to be causing reliable failures
in what should have been an unrelated test. See bpf CI.
I suspect there is a cleanup issue with this new test.
Pls investigate.

pw-bot: cr
Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for connect() racing sockmap update and signal
Posted by bot+bpf-ci@kernel.org 2 weeks, 3 days ago
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
> new file mode 100644
> index 000000000000..aa48ae483dab
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0

[ ... ]

> +#include "sockmap_helpers.h"

[ ... ]

> +void serial_test_sockmap_interrupted_connect(void)
> +{
> +	sighandler_t orig_handler;
> +	struct context ctx = {0};
> +	pthread_t tid;
> +
> +	orig_handler = signal(SIGUSR1, handler);
> +	if (!ASSERT_NEQ(orig_handler, SIG_ERR, "signal"))
> +		return;
> +
> +	if (xpthread_create(&tid, NULL, racer, &ctx))
> +		goto restore;
> +
> +	test_map(&ctx, BPF_MAP_TYPE_SOCKMAP);
> +
> +	if (!xpthread_cancel(tid))
              ^^^^^^^^^^^^^^^^

Is xpthread_cancel() defined somewhere? I see sockmap_helpers.h defines
xpthread_create() and xpthread_join(), but there does not appear to be a
definition for xpthread_cancel(). This would cause a compilation failure.

> +		xpthread_join(tid, NULL);
> +restore:
> +	ASSERT_NEQ(signal(SIGUSR1, orig_handler), SIG_ERR, "handler restore");
> +}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21294285592
Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for connect() racing sockmap update and signal
Posted by Michal Luczaj 2 weeks, 3 days ago
On 1/23/26 18:09, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
>> new file mode 100644
>> index 000000000000..aa48ae483dab
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_interrupted_connect.c
>> @@ -0,0 +1,200 @@
>> +// SPDX-License-Identifier: GPL-2.0
> 
> [ ... ]
> 
>> +#include "sockmap_helpers.h"
> 
> [ ... ]
> 
>> +void serial_test_sockmap_interrupted_connect(void)
>> +{
>> +	sighandler_t orig_handler;
>> +	struct context ctx = {0};
>> +	pthread_t tid;
>> +
>> +	orig_handler = signal(SIGUSR1, handler);
>> +	if (!ASSERT_NEQ(orig_handler, SIG_ERR, "signal"))
>> +		return;
>> +
>> +	if (xpthread_create(&tid, NULL, racer, &ctx))
>> +		goto restore;
>> +
>> +	test_map(&ctx, BPF_MAP_TYPE_SOCKMAP);
>> +
>> +	if (!xpthread_cancel(tid))
>               ^^^^^^^^^^^^^^^^
> 
> Is xpthread_cancel() defined somewhere? I see sockmap_helpers.h defines
> xpthread_create() and xpthread_join(), but there does not appear to be a
> definition for xpthread_cancel(). This would cause a compilation failure.

xpthread_cancel() is defined in patch 1/2 of this series.
But apparently patch 1/2 is still missing from https://lore.kernel.org/bpf.
FWIW, it's already here:
https://lore.kernel.org/lkml/20260123-selftest-signal-on-connect-v1-1-b0256e7025b6@rbox.co/