netdev - Re: [PATCH net 5/5] vsock/test: Add test for connect() retries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <pyun3hl67vjel7gc7k67nvelx5bmgw664gvkzauqqv6nkkt5sc@x6hzsedofchl>
Date: Mon, 20 Jan 2025 11:24:30 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
Cc: "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	George Zhang <georgezhang@...are.com>, Dmitry Torokhov <dtor@...are.com>, Andy King <acking@...are.com>, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net 5/5] vsock/test: Add test for connect() retries

On Fri, Jan 17, 2025 at 10:59:45PM +0100, Michal Luczaj wrote:
>Deliberately fail a connect() attempt; expect error. Then verify that
>subsequent attempt (using the same socket) can still succeed, rather than
>fail outright.
>
>Signed-off-by: Michal Luczaj <mhal@...x.co>
>---
> tools/testing/vsock/vsock_test.c | 52 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 7f1916e23858b5ba407c34742a05b7bd6cfdcc10..712650f993e9df68ceb68ae02334c2775be09c7c 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1520,6 +1520,53 @@ static void test_seqpacket_transport_uaf_server(const struct test_opts *opts)
> 	control_expectln("DONE");
> }
>
>+static void test_stream_connect_retry_client(const struct test_opts *opts)
>+{
>+	struct sockaddr_vm addr = {
>+		.svm_family = AF_VSOCK,
>+		.svm_cid = opts->peer_cid,
>+		.svm_port = opts->peer_port
>+	};
>+	int s, alen = sizeof(addr);
>+
>+	s = socket(AF_VSOCK, SOCK_STREAM, 0);
>+	if (s < 0) {
>+		perror("socket");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	if (!connect(s, (struct sockaddr *)&addr, alen)) {
>+		fprintf(stderr, "Unexpected connect() #1 success\n");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	control_writeln("LISTEN");
>+	control_expectln("LISTENING");
>+
>+	if (connect(s, (struct sockaddr *)&addr, alen)) {
>+		perror("connect() #2");
>+		exit(EXIT_FAILURE);
>+	}

What about using the timeout_begin()/timeout_end() we used in all other
places?

>+
>+	close(s);
>+}
>+
>+static void test_stream_connect_retry_server(const struct test_opts *opts)
>+{
>+	int fd;
>+
>+	control_expectln("LISTEN");
>+
>+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+	if (fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	vsock_wait_remote_close(fd);
>+	close(fd);
>+}
>+
> static struct test_case test_cases[] = {
> 	{
> 		.name = "SOCK_STREAM connection reset",
>@@ -1655,6 +1702,11 @@ static struct test_case test_cases[] = {
> 		.run_client = test_seqpacket_transport_uaf_client,
> 		.run_server = test_seqpacket_transport_uaf_server,
> 	},
>+	{
>+		.name = "connectible retry failed connect()",

"SOCK_STREAM retry failed connect()"

(and if you want add a comment on top of the test the in the current
implementation the same path is shared with other connectible socket
like SOCK_SEQPACKET)

The rest LGTM.

Thanks,
Stefano

>+		.run_client = test_stream_connect_retry_client,
>+		.run_server = test_stream_connect_retry_server,
>+	},
> 	{},
> };
>
>
>-- 
>2.47.1
>