diff options
author | Martin KaFai Lau <martin.lau@kernel.org> | 2023-12-13 16:21:53 -0800 |
---|---|---|
committer | Martin KaFai Lau <martin.lau@kernel.org> | 2023-12-13 16:33:17 -0800 |
commit | 2f2fee2bf74a7e31d06fc6cb7ba2bd4dd7753c99 (patch) | |
tree | fa6dcb7f84468e7631a8be8d1f4eb3b318d9d66f | |
parent | e307b5a845c5951dabafc48d00b6424ee64716c4 (diff) | |
parent | 50d96f05af6787a34b4eca2ee3fc1993289c4c24 (diff) |
Merge branch ' bpf fix for unconnect af_unix socket'
John Fastabend says:
====================
Eric reported a syzbot splat from a null ptr deref from recent fix to
resolve a use-after-free with af-unix stream sockets and BPF sockmap
usage.
The issue is I missed is we allow unconnected af_unix STREAM sockets to
be added to the sockmap. Fix this by blocking unconnected sockets.
v2: change sk_is_unix to sk_is_stream_unix (Eric) and remove duplicate
ASSERTS in selftests the xsocket helper already marks FAIL (Jakub)
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
-rw-r--r-- | include/net/sock.h | 5 | ||||
-rw-r--r-- | net/core/sock_map.c | 2 | ||||
-rw-r--r-- | tools/testing/selftests/bpf/prog_tests/sockmap_basic.c | 34 |
3 files changed, 41 insertions, 0 deletions
diff --git a/include/net/sock.h b/include/net/sock.h index 1d6931caf0c3..0201136b0b9c 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -2799,6 +2799,11 @@ static inline bool sk_is_tcp(const struct sock *sk) return sk->sk_type == SOCK_STREAM && sk->sk_protocol == IPPROTO_TCP; } +static inline bool sk_is_stream_unix(const struct sock *sk) +{ + return sk->sk_family == AF_UNIX && sk->sk_type == SOCK_STREAM; +} + /** * sk_eat_skb - Release a skb if it is no longer needed * @sk: socket to eat this skb from diff --git a/net/core/sock_map.c b/net/core/sock_map.c index 4292c2ed1828..27d733c0f65e 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -536,6 +536,8 @@ static bool sock_map_sk_state_allowed(const struct sock *sk) { if (sk_is_tcp(sk)) return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN); + if (sk_is_stream_unix(sk)) + return (1 << sk->sk_state) & TCPF_ESTABLISHED; return true; } diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c index f75f84d0b3d7..7c2241fae19a 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c @@ -524,6 +524,37 @@ out: test_sockmap_pass_prog__destroy(pass); } +static void test_sockmap_unconnected_unix(void) +{ + int err, map, stream = 0, dgram = 0, zero = 0; + struct test_sockmap_pass_prog *skel; + + skel = test_sockmap_pass_prog__open_and_load(); + if (!ASSERT_OK_PTR(skel, "open_and_load")) + return; + + map = bpf_map__fd(skel->maps.sock_map_rx); + + stream = xsocket(AF_UNIX, SOCK_STREAM, 0); + if (stream < 0) + return; + + dgram = xsocket(AF_UNIX, SOCK_DGRAM, 0); + if (dgram < 0) { + close(stream); + return; + } + + err = bpf_map_update_elem(map, &zero, &stream, BPF_ANY); + ASSERT_ERR(err, "bpf_map_update_elem(stream)"); + + err = bpf_map_update_elem(map, &zero, &dgram, BPF_ANY); + ASSERT_OK(err, "bpf_map_update_elem(dgram)"); + + close(stream); + close(dgram); +} + void test_sockmap_basic(void) { if (test__start_subtest("sockmap create_update_free")) @@ -566,4 +597,7 @@ void test_sockmap_basic(void) test_sockmap_skb_verdict_fionread(false); if (test__start_subtest("sockmap skb_verdict msg_f_peek")) test_sockmap_skb_verdict_peek(); + + if (test__start_subtest("sockmap unconnected af_unix")) + test_sockmap_unconnected_unix(); } |