niansa-misc/linux
1
0
Fork 0
mirror of synced 2025-03-06 20:59:54 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge branch 'sockmap-vsock-for-connectible-sockets-allow-only-connected'

Browse source 
Michal Luczaj says:

====================
sockmap, vsock: For connectible sockets allow only connected

Series deals with one more case of vsock surprising BPF/sockmap by being
inconsistency about (having an) assigned transport.

KASAN: null-ptr-deref in range [0x0000000000000120-0x0000000000000127]
CPU: 7 UID: 0 PID: 56 Comm: kworker/7:0 Not tainted 6.14.0-rc1+
Workqueue: vsock-loopback vsock_loopback_work
RIP: 0010:vsock_read_skb+0x4b/0x90
Call Trace:
 sk_psock_verdict_data_ready+0xa4/0x2e0
 virtio_transport_recv_pkt+0x1ca8/0x2acc
 vsock_loopback_work+0x27d/0x3f0
 process_one_work+0x846/0x1420
 worker_thread+0x5b3/0xf80
 kthread+0x35a/0x700
 ret_from_fork+0x2d/0x70
 ret_from_fork_asm+0x1a/0x30

This bug, similarly to commit f6abafcd32 ("vsock/bpf: return early if
transport is not assigned"), could be fixed with a single NULL check. But
instead, let's explore another approach: take a hint from
vsock_bpf_update_proto() and teach sockmap to accept only vsocks that are
already connected (no risk of transport being dropped or reassigned). At
the same time straight reject the listeners (vsock listening sockets do not
carry any transport anyway). This way BPF does not have to worry about
vsk->transport becoming NULL.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
====================

Link: https://patch.msgid.link/20250213-vsock-listen-sockmap-nullptr-v1-0-994b7cd2f16b@rbox.co
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni 2025-02-18 12:00:16 +01:00
commit f7b5279b67
4 changed files with 59 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
net/core/sock_map.c
View file

@ -541,6 +541,9 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);
if (sk_is_stream_unix(sk))
return (1 << sk->sk_state) & TCPF_ESTABLISHED;
if (sk_is_vsock(sk) &&
(sk->sk_type == SOCK_STREAM || sk->sk_type == SOCK_SEQPACKET))
return (1 << sk->sk_state) & TCPF_ESTABLISHED;
return true;
}

3
net/vmw_vsock/af_vsock.c
View file

@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
{
struct vsock_sock *vsk = vsock_sk(sk);
if (WARN_ON_ONCE(!vsk->transport))
return -ENODEV;
return vsk->transport->read_skb(vsk, read_actor);
}

2
net/vmw_vsock/vsock_bpf.c
View file

@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
lock_sock(sk);
vsk = vsock_sk(sk);
if (!vsk->transport) {
if (WARN_ON_ONCE(!vsk->transport)) {
copied = -ENODEV;
goto out;
}

70
tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
View file

@ -111,31 +111,35 @@ out:
static void test_sockmap_vsock_delete_on_close(void)
{
int err, c, p, map;
const int zero = 0;
err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
if (!ASSERT_OK(err, "create_pair(AF_VSOCK)"))
return;
int map, c, p, err, zero = 0;
map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
sizeof(int), 1, NULL);
if (!ASSERT_GE(map, 0, "bpf_map_create")) {
close(c);
goto out;
}
if (!ASSERT_OK_FD(map, "bpf_map_create"))
return;
err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
if (!ASSERT_OK(err, "create_pair"))
goto close_map;
if (xbpf_map_update_elem(map, &zero, &c, BPF_NOEXIST))
goto close_socks;
xclose(c);
xclose(p);
err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
if (!ASSERT_OK(err, "create_pair"))
goto close_map;
err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
close(c);
if (!ASSERT_OK(err, "bpf_map_update"))
goto out;
err = bpf_map_update_elem(map, &zero, &p, BPF_NOEXIST);
ASSERT_OK(err, "after close(), bpf_map_update");
out:
close(p);
close(map);
close_socks:
xclose(c);
xclose(p);
close_map:
xclose(map);
}
static void test_skmsg_helpers(enum bpf_map_type map_type)
@ -1061,6 +1065,34 @@ destroy:
test_sockmap_pass_prog__destroy(skel);
}
static void test_sockmap_vsock_unconnected(void)
{
struct sockaddr_storage addr;
int map, s, zero = 0;
socklen_t alen;
map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
sizeof(int), 1, NULL);
if (!ASSERT_OK_FD(map, "bpf_map_create"))
return;
s = xsocket(AF_VSOCK, SOCK_STREAM, 0);
if (s < 0)
goto close_map;
/* Fail connect(), but trigger transport assignment. */
init_addr_loopback(AF_VSOCK, &addr, &alen);
if (!ASSERT_ERR(connect(s, sockaddr(&addr), alen), "connect"))
goto close_sock;
ASSERT_ERR(bpf_map_update_elem(map, &zero, &s, BPF_ANY), "map_update");
close_sock:
xclose(s);
close_map:
xclose(map);
}
void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
@ -1127,4 +1159,6 @@ void test_sockmap_basic(void)
test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKHASH);
if (test__start_subtest("sockmap skb_verdict vsock poll"))
test_sockmap_skb_verdict_vsock_poll();
if (test__start_subtest("sockmap vsock unconnected"))
test_sockmap_vsock_unconnected();
}