linux-kernel - Re: [PATCH net-next v4 3/3] vsock/test: Expand linger test to ensure close() does not misbehave

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: <720f6986-8b32-4d00-b309-66a6f0c1ca40@rbox.co>
Date: Mon, 12 May 2025 14:23:12 +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 v4 3/3] vsock/test: Expand linger test to ensure
 close() does not misbehave

On 5/7/25 10:26, Stefano Garzarella wrote:
> On Wed, 7 May 2025 at 00:47, Michal Luczaj <mhal@...x.co> wrote:
>>
>> On 5/6/25 11:46, Stefano Garzarella wrote:
>>> On Tue, 6 May 2025 at 11:43, Stefano Garzarella <sgarzare@...hat.com> wrote:
>>>>
>>>> On Thu, May 01, 2025 at 10:05:24AM +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.
>>>>
>>>> This is a new behaviour that only new kernels will follow, so I think
>>>> it is better to add a new test instead of extending a pre-existing test
>>>> that we described as "SOCK_STREAM SO_LINGER null-ptr-deref".
>>>>
>>>> The old test should continue to check the null-ptr-deref also for old
>>>> kernels, while the new test will check the new behaviour, so we can skip
>>>> the new test while testing an old kernel.
>>
>> Right, I'll split it.
>>
>>> I also saw that we don't have any test to verify that actually the
>>> lingering is working, should we add it since we are touching it?
>>
>> Yeah, I agree we should. Do you have any suggestion how this could be done
>> reliably?
> 
> Can we play with SO_VM_SOCKETS_BUFFER_SIZE like in credit-update tests?
> 
> One peer can set it (e.g. to 1k), accept the connection, but without
> read anything. The other peer can set the linger timeout, send more
> bytes than the buffer size set by the receiver.
> At this point the extra bytes should stay on the sender socket buffer,
> so we can do the close() and it should time out, and we can check if
> it happens.
> 
> WDYT?

Haven't we discussed this approach in [1]? I've reported that I can't make
it work. But maybe I'm misunderstanding something, please see the code below.

[1]:
https://lore.kernel.org/netdev/df2d51fd-03e7-477f-8aea-938446f47864@rbox.co/

import termios, time
from socket import *

SIOCOUTQ = termios.TIOCOUTQ
VMADDR_CID_LOCAL = 1
SZ = 1024

def set_linger(s, timeout):
	optval = (timeout << 32) | 1
	s.setsockopt(SOL_SOCKET, SO_LINGER, optval)
	assert s.getsockopt(SOL_SOCKET, SO_LINGER) == optval

def set_bufsz(s, size):
	s.setsockopt(AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE, size)
	assert s.getsockopt(AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE) == size

def check_lingering(addr):
	lis = socket(AF_VSOCK, SOCK_STREAM)
	lis.bind(addr)
	lis.listen()
	set_bufsz(lis, SZ)

	s = socket(AF_VSOCK, SOCK_STREAM)
	set_linger(s, 5)
	s.connect(lis.getsockname())

	p, _ = lis.accept()

	s.send(b'x')
	p.recv(1)

	print("sending...")
	s.send(b'x' * (SZ+1)) # blocks
	print("sent")

	print("closing...")
	ts = time.time()
	s.close()
	print("done in %ds" % (time.time() - ts))

check_lingering((VMADDR_CID_LOCAL, 1234))