netdev - Re: [PATCH net-next v5 5/5] vsock/test: Add test for an unexpectedly lingering close()

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: <5dc16a14-e66c-4e1d-896f-a8483cdf0f04@rbox.co>
Date: Thu, 22 May 2025 01:17:30 +0200
From: Michal Luczaj <mhal@...x.co>
To: Stefano Garzarella <sgarzare@...hat.com>
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>,
 "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, Stefan Hajnoczi <stefanha@...hat.com>,
 virtualization@...ts.linux.dev, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH net-next v5 5/5] vsock/test: Add test for an unexpectedly
 lingering close()

On 5/21/25 16:56, Stefano Garzarella wrote:
> On Wed, May 21, 2025 at 12:55:23AM +0200, Michal Luczaj wrote:
>> There was an issue with SO_LINGER: instead of blocking until all queued
>> messages for the socket have been successfully sent (or the linger timeout
>> has been reached), close() would block until packets were handled by the
>> peer.
>>
>> Add a test to alert on close() lingering when it should not.
>>
>> Signed-off-by: Michal Luczaj <mhal@...x.co>
>> ---
>> tools/testing/vsock/vsock_test.c | 49 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index f401c6a79495bc7fda97012e5bfeabec7dbfb60a..1040503333cf315e52592c876f2c1809b36fdfdb 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -1839,6 +1839,50 @@ static void test_stream_linger_server(const struct test_opts *opts)
>> 	close(fd);
>> }
>>
>> +static void test_stream_nolinger_client(const struct test_opts *opts)
>> +{
>> +	bool nowait;
>> +	time_t ns;
>> +	int fd;
>> +
>> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>> +	if (fd < 0) {
>> +		perror("connect");
>> +		exit(EXIT_FAILURE);
>> +	}
>> +
>> +	enable_so_linger(fd);
> 
> If we use a parameter for the linger timeout, IMO will be easy to 
> understand this test, defining the timeout in this test, set it and 
> check the value, without defining LINGER_TIMEOUT in util.h.

Yes, you're right. I'll fix that.

>> +	send_byte(fd, 1, 0); /* Left unread to expose incorrect behaviour. */
>> +	nowait = vsock_wait_sent(fd);
>> +
>> +	ns = current_nsec();
>> +	close(fd);
>> +	ns = current_nsec() - ns;
>> +
>> +	if (nowait) {
>> +		fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
>> +	} else if ((ns + NSEC_PER_SEC - 1) / NSEC_PER_SEC >= LINGER_TIMEOUT) {
> 
> Should we define a macro for this conversion?
> 
> Or just use DIV_ROUND_UP:

Arrgh, I was looking for that. If you don't care much for a new macro, I'll
explicitly use DIV_ROUND_UP for now.

Thanks!
Michal