On Fri, Mar 07, 2025 at 05:01:11PM +0100, Michal Luczaj wrote:
On 3/7/25 15:35, Stefano Garzarella wrote:
On Fri, Mar 07, 2025 at 10:58:55AM +0100, Michal Luczaj wrote:
Signal delivered during connect() may result in a disconnect of an already
TCP_ESTABLISHED socket. Problem is that such established socket might have
been placed in a sockmap before the connection was closed. We end up with a
SS_UNCONNECTED vsock in a sockmap. And this, combined with the ability to
reassign (unconnected) vsock's transport to NULL, breaks the sockmap
contract. As manifested by WARN_ON_ONCE.
Note that Luigi is currently working on a (vsock test suit) test[1] for a
related bug, which could be neatly adapted to test this bug as well.
[1]: https://lore.kernel.org/netdev/20250306-test_vsock-v1-0-0320b5accf92@xxxxxxxxxx/
Can you work with Luigi to include the changes in that series?
I was just going to wait for Luigi to finish his work (no rush, really) and
then try to parametrize it.
Sure, this is fine, thanks for that!
Stefano
That is unless BPF/sockmap maintainers decide this thread's thing is a
sockmap thing and should be in tools/testing/selftests/bpf.
Below is a repro. If I'm not mistaken, it's basically what Luigi wrote,
just sprinkled with map_update_elem() and recv().
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <pthread.h>
#include <sys/wait.h>
#include <sys/socket.h>
#include <sys/syscall.h>
#include <linux/bpf.h>
#include <linux/vm_sockets.h>
static void die(const char *msg)
{
perror(msg);
exit(-1);
}
static int sockmap_create(void)
{
union bpf_attr attr = {
.map_type = BPF_MAP_TYPE_SOCKMAP,
.key_size = sizeof(int),
.value_size = sizeof(int),
.max_entries = 1
};
int map;
map = syscall(SYS_bpf, BPF_MAP_CREATE, &attr, sizeof(attr));
if (map < 0)
die("map_create");
return map;
}
static void map_update_elem(int fd, int key, int value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = (uint64_t)&key,
.value = (uint64_t)&value,
.flags = BPF_ANY
};
(void)syscall(SYS_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}
static void sighandler(int sig)
{
/* nop */
}
static void *racer(void *c)
{
int map = sockmap_create();
for (;;) {
map_update_elem(map, 0, *(int *)c);
if (kill(0, SIGUSR1) < 0)
die("kill");
}
}
int main(void)
{
struct sockaddr_vm addr = {
.svm_family = AF_VSOCK,
.svm_cid = VMADDR_CID_LOCAL,
.svm_port = VMADDR_PORT_ANY
};
socklen_t alen = sizeof(addr);
struct sockaddr_vm bad_addr;
pthread_t thread;
int s, c;
s = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
if (s < 0)
die("socket s");
if (bind(s, (struct sockaddr *)&addr, alen))
die("bind");
if (listen(s, -1))
die("listen");
if (getsockname(s, (struct sockaddr *)&addr, &alen))
die("getsockname");
bad_addr = addr;
bad_addr.svm_cid = 0x42424242; /* non-existing */
if (signal(SIGUSR1, sighandler) == SIG_ERR)
die("signal");
if (pthread_create(&thread, 0, racer, &c))
die("pthread_create");
for (;;) {
c = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
if (c < 0)
die("socket c");
if (!connect(c, (struct sockaddr *)&addr, alen) ||
errno != EINTR)
goto outro;
if (!connect(c, (struct sockaddr *)&bad_addr, alen) ||
errno != ESOCKTNOSUPPORT)
goto outro;
(void)recv(c, &(char){0}, 1, MSG_DONTWAIT);
outro:
close(accept(s, NULL, NULL));
close(c);
}
return 0;
}
