netdev - Re: [PATCH net] vsock/test: Fix occasional failure in SOCK_STREAM SHUT

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: <2y6v7vog4dylnnu7j625gkijth7lnznvgcjl4kg2q3xy5ht6fe@uikdt45mmocp>
Date: Mon, 26 May 2025 09:54:02 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Konstantin Shkolnyy <kshk@...ux.ibm.com>
Cc: virtualization@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, mjrosato@...ux.ibm.com
Subject: Re: [PATCH net] vsock/test: Fix occasional failure in SOCK_STREAM
 SHUT_RD test

On Sun, May 25, 2025 at 11:32:20PM -0500, Konstantin Shkolnyy wrote:
>The test outputs:
>"SOCK_STREAM SHUT_RD...expected send(2) failure, got 1".
>
>It tests that shutdown(fd, SHUT_RD) on one side causes send() to fail on
>the other side. However, sometimes there is a delay in delivery of the
>SHUT_RD command, send() succeeds and the test fails, even though the
>command is properly delivered and send() starts failing several
>milliseconds later.
>
>The delay occurs in the kernel because the used buffer notification
>callback virtio_vsock_rx_done(), called upon receipt of the SHUT_RD
>command, doesn't immediately disable send(). It delegates that to
>a kernel thread (via vsock->rx_work). Sometimes that thread is delayed
>more than the test expects.
>
>Change the test to keep calling send() until it fails or a timeout occurs.
>
>Fixes: b698bd97c5711 ("test/vsock: shutdowned socket test")
>Signed-off-by: Konstantin Shkolnyy <kshk@...ux.ibm.com>
>---
> tools/testing/vsock/vsock_test.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 613551132a96..c3b90a94a281 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1058,17 +1058,22 @@ static void sigpipe(int signo)
> 	have_sigpipe = 1;
> }
>
>-static void test_stream_check_sigpipe(int fd)
>+static void test_for_send_failure(int fd, int send_flags)
> {
>-	ssize_t res;
>+	timeout_begin(TIMEOUT);
>+	while (true) {
>+		if (send(fd, "A", 1, send_flags) == -1)
>+			return;
>+		timeout_check("expected send(2) failure");
>+	}
>+	timeout_end();
>+}

I'd move this in util.c like we did in 
https://lore.kernel.org/virtualization/20250522-vsock-linger-v6-3-2ad00b0e447e@rbox.co/

And I'd rename following the other functions we have there.

Thanks,
Stefano

>
>+static void test_stream_check_sigpipe(int fd)
>+{
> 	have_sigpipe = 0;
>
>-	res = send(fd, "A", 1, 0);
>-	if (res != -1) {
>-		fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>-		exit(EXIT_FAILURE);
>-	}
>+	test_for_send_failure(fd, 0);
>
> 	if (!have_sigpipe) {
> 		fprintf(stderr, "SIGPIPE expected\n");
>@@ -1077,11 +1082,7 @@ static void test_stream_check_sigpipe(int fd)
>
> 	have_sigpipe = 0;
>
>-	res = send(fd, "A", 1, MSG_NOSIGNAL);
>-	if (res != -1) {
>-		fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>-		exit(EXIT_FAILURE);
>-	}
>+	test_for_send_failure(fd, MSG_NOSIGNAL);
>
> 	if (have_sigpipe) {
> 		fprintf(stderr, "SIGPIPE not expected\n");
>-- 
>2.34.1
>