netdev - Re: [PATCH net 2/2] vsock/test: Add test for SO

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: <vsghmgwurw3rxzw32najvwddolmrbroyryquzsoqt5jr3trzif@4rjr7kwlaowa>
Date: Mon, 10 Feb 2025 11:18:42 +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>, netdev@...r.kernel.org
Subject: Re: [PATCH net 2/2] vsock/test: Add test for SO_LINGER null ptr deref

On Wed, Feb 05, 2025 at 12:20:56PM +0100, Michal Luczaj wrote:
>On 2/4/25 11:48, Stefano Garzarella wrote:
>> On Tue, Feb 04, 2025 at 01:29:53AM +0100, Michal Luczaj wrote:
>>> ...
>>> +static void test_stream_linger_client(const struct test_opts *opts)
>>> +{
>>> +	struct linger optval = {
>>> +		.l_onoff = 1,
>>> +		.l_linger = 1
>>> +	};
>>> +	int fd;
>>> +
>>> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>>> +	if (fd < 0) {
>>> +		perror("connect");
>>> +		exit(EXIT_FAILURE);
>>> +	}
>>> +
>>> +	if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) {
>>> +		perror("setsockopt(SO_LINGER)");
>>> +		exit(EXIT_FAILURE);
>>> +	}
>>
>> Since we are testing SO_LINGER, will also be nice to check if it's
>> working properly, since one of the fixes proposed could break it.
>>
>> To test, we may set a small SO_VM_SOCKETS_BUFFER_SIZE on the receive
>> side and try to send more than that value, obviously without reading
>> anything into the receiver, and check that close() here, returns after
>> the timeout we set in .l_linger.
>
>I may be doing something wrong, but (at least for loopback transport) it

Also with VMs is the same, I think virtio_transport_wait_close() can be 
improved to check if everything is sent, avoiding to wait.

But this is material for another series, so this test should be fine for 
now!

Thanks,
Stefano

>seems that close() lingers until data is received, not sent (without even
>touching SO_VM_SOCKETS_BUFFER_SIZE).
>
>```
>import struct, fcntl, termios, time
>from socket import *
>
>def linger(s, timeout):
>	if s.family == AF_VSOCK:
>		s.setsockopt(SOL_SOCKET, SO_LINGER, (timeout<<32) | 1)
>	elif s.family == AF_INET:
>		s.setsockopt(SOL_SOCKET, SO_LINGER, struct.pack('ii', 1, timeout))
>	else:
>		assert False
>
>def unsent(s):
>	SIOCOUTQ = termios.TIOCOUTQ
>	return struct.unpack('I', fcntl.ioctl(s, SIOCOUTQ, bytes(4)))[0]
>
>def check_lingering(family, addr):
>	lis = socket(family, SOCK_STREAM)
>	lis.bind(addr)
>	lis.listen()
>
>	s = socket(family, SOCK_STREAM)
>	linger(s, 2)
>	s.connect(lis.getsockname())
>
>	for _ in range(1, 1<<8):
>		s.send(b'x')
>
>	while unsent(s) != 0:
>		pass
>
>	print("closing...")
>	ts = time.time()
>	s.close()
>	print(f"done in %ds" % (time.time() - ts))
>
>check_lingering(AF_INET, ('127.0.0.1', 1234))
>check_lingering(AF_VSOCK, (1, 1234)) # VMADDR_CID_LOCAL
>```
>
>Gives me:
>closing...
>done in 0s
>closing...
>done in 2s
>