netdev - vsock broken after connect() returns EINTR (was 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d3a0a4e3-57bd-43f2-8907-af60c18d53ec@rbox.co>
Date: Fri, 11 Apr 2025 16:43:35 +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>,
 netdev@...r.kernel.org
Subject: vsock broken after connect() returns EINTR (was Re: [PATCH net 2/2]
 vsock/test: Add test for SO_LINGER null ptr deref)

On 4/11/25 15:21, Stefano Garzarella wrote:
> On Fri, Apr 04, 2025 at 12:06:36AM +0200, Michal Luczaj wrote:
>> On 4/1/25 12:32, Stefano Garzarella wrote:
>>> On Tue, Mar 25, 2025 at 02:22:45PM +0100, Michal Luczaj wrote:
>>>> ...
>>>> Plus, it appears to be broken: when I hit this condition and I try to
>>>> re-connect to the same listener, I get ETIMEDOUT for loopback and
>>>> ECONNRESET for g2h virtio; see [2].
>>>
>>> Could this be related to the fix I sent some days ago?
>>> https://lore.kernel.org/netdev/20250328141528.420719-1-sgarzare@redhat.com/
>>
>> I've tried that. I've also took a hint from your other mail and attempted
>> flushing the listener queue, but to no avail. Crude code below. Is there
>> something wrong with it?
> 
> I can't see anything wrong, but I'm starting to get confused :-(
> we're talking about too many things in the same thread.

Uhm, that's true, sorry. I've split the thread, hope this helps.

> What issues do you want to highlight?

Once connect() fails with EINTR (e.g. due to a signal delivery), retrying
connect() (to the same listener) fails. That is what the code below was
trying to show.

> #include <stdio.h>
> #include <errno.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <signal.h>
> #include <pthread.h>
> #include <sys/socket.h>
> #include <linux/vm_sockets.h>
>
> static void die(const char *msg)
> {
> 	perror(msg);
> 	exit(-1);
> }
>
> static void barrier(pthread_barrier_t *barr)
> {
> 	errno = pthread_barrier_wait(barr);
> 	if (errno && errno != PTHREAD_BARRIER_SERIAL_THREAD)
> 		die("pthread_barrier_wait");
> }
>
> static void flush_accept(int s)
> {
> 	int p = accept(s, NULL, NULL);
> 	if (p < 0) {
> 		if (errno != EAGAIN)
> 			perror("accept");
> 		return;
> 	}
>
> 	printf("accept: drained\n");
> 	close(p);
> }
>
> static void handler(int signum)
> {
> 	/* nop */
> }
>
> void static set_accept_timeout(int s)
> {
> 	struct timeval tv = { .tv_sec = 1 };
> 	if (setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)))
> 		die("setsockopt SO_RCVTIMEO");
> }
>
> void static set_connect_timeout(int s)
> {
> 	struct timeval tv = { .tv_sec = 1 };
> 	if (setsockopt(s, AF_VSOCK, SO_VM_SOCKETS_CONNECT_TIMEOUT, &tv,
> 		       sizeof(tv)))
> 		die("setsockopt SO_VM_SOCKETS_CONNECT_TIMEOUT");
> }
>
> static void *killer(void *arg)
> {
> 	pthread_barrier_t *barr = (pthread_barrier_t *)arg;
> 	pid_t pid = getpid();
>
> 	if ((errno = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS,
> 					   NULL)))
> 		die("pthread_setcanceltype");
>
> 	for (;;) {
> 		barrier(barr);
> 		if (kill(pid, SIGUSR1))
> 			die("kill");
> 		barrier(barr);
> 	}
>
> 	return NULL;
> }
>
> int main(void)
> {
> 	struct sockaddr_vm addr = {
> 		.svm_family = AF_VSOCK,
> 		.svm_cid = VMADDR_CID_LOCAL,
> 		.svm_port = 1234
> 	};
> 	socklen_t alen = sizeof(addr);
> 	pthread_barrier_t barr;
> 	pthread_t tid;
> 	int s, c;
>
> 	if ((errno = pthread_barrier_init(&barr, NULL, 2)))
> 		die("pthread_barrier_init");
>
> 	if (signal(SIGUSR1, handler) == SIG_ERR)
> 		die("signal");
>
> 	s = socket(AF_VSOCK, SOCK_STREAM, 0);
> 	if (s < 0)
> 		die("socket s");
> 	set_accept_timeout(s);
>
> 	if (bind(s, (struct sockaddr *)&addr, alen))
> 		die("bind");
>
> 	if (listen(s, 64))
> 		die("listen");
>
> 	if ((errno = pthread_create(&tid, NULL, killer, &barr)))
> 		die("pthread_create");
>
> 	for (;;) {
> 		c = socket(AF_VSOCK, SOCK_STREAM, 0);
> 		if (c < 0)
> 			die("socket c");
>
> 		barrier(&barr);
> 		if (connect(c, (struct sockaddr *)&addr, sizeof(addr)) &&
> 		    errno == EINTR) {
> 		    	printf("connect: EINTR\n");
> 			break;
> 		}
> 		barrier(&barr);
>
> 		close(c);
> 		flush_accept(s);
> 	}
>
> 	if ((errno = pthread_cancel(tid)))
> 		die("pthread_cancel");
>
> 	if ((errno = pthread_join(tid, NULL)))
> 		die("pthread_join");
>
> 	flush_accept(s);
> 	set_connect_timeout(c);
> 	if (connect(c, (struct sockaddr *)&addr, sizeof(addr)))
> 		die("re-connect");
>
> 	printf("okay?\n");
>
> 	return 0;
> }
>