netdev - Re: [PATCH net-next 1/2] vsock/test: retry send() to avoid occasional failure in sigpipe test

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <vheia3vqebshkzajzb6qhxppjvj7onvtjezw5pddfl6qqrnrma@h3dpvkla2pco>
Date: Tue, 13 May 2025 15:46:10 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	virtualization@...ts.linux.dev
Subject: Re: [PATCH net-next 1/2] vsock/test: retry send() to avoid
 occasional failure in sigpipe test

On Tue, May 13, 2025 at 12:37:36PM +0200, Paolo Abeni wrote:
>On 5/8/25 4:20 PM, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@...hat.com>
>>
>> When the other peer calls shutdown(SHUT_RD), there is a chance that
>> the send() call could occur before the message carrying the close
>> information arrives over the transport. In such cases, the send()
>> might still succeed. To avoid this race, let's retry the send() call
>> a few times, ensuring the test is more reliable.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>> ---
>>  tools/testing/vsock/vsock_test.c | 28 ++++++++++++++++++----------
>>  1 file changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index d0f6d253ac72..7de870dee1cf 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -1064,11 +1064,18 @@ 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);
>> -	}
>> +	/* When the other peer calls shutdown(SHUT_RD), there is a chance that
>> +	 * the send() call could occur before the message carrying the close
>> +	 * information arrives over the transport. In such cases, the send()
>> +	 * might still succeed. To avoid this race, let's retry the send() call
>> +	 * a few times, ensuring the test is more reliable.
>> +	 */
>> +	timeout_begin(TIMEOUT);
>> +	do {
>> +		res = send(fd, "A", 1, 0);
>> +		timeout_check("send");
>> +	} while (res != -1);
>
>AFAICS the above could spin on send() for up to 10s, I would say
>considerably more than 'a few times' ;)
>
>In practice that could cause side effect on the timing of other
>concurrent tests (due to one CPU being 100% used for a while).
>
>What if the peer rcvbuf fills-up: will the send fail? That could cause
>false-negative.

Good point!

>
>I *think* it should be better to insert a short sleep in the loop.

Agree, I'll add.

Thanks,
Stefano