Skip to content

Commit 8d66506

Browse files
jrfastabMartin KaFai Lau
authored andcommitted
bpf: syzkaller found null ptr deref in unix_bpf proto add
I added logic to track the sock pair for stream_unix sockets so that we ensure lifetime of the sock matches the time a sockmap could reference the sock (see fixes tag). I forgot though that we allow af_unix unconnected sockets into a sock{map|hash} map. This is problematic because previous fixed expected sk_pair() to exist and did not NULL check it. Because unconnected sockets have a NULL sk_pair this resulted in the NULL ptr dereference found by syzkaller. BUG: KASAN: null-ptr-deref in unix_stream_bpf_update_proto+0x72/0x430 net/unix/unix_bpf.c:171 Write of size 4 at addr 0000000000000080 by task syz-executor360/5073 Call Trace: <TASK> ... sock_hold include/net/sock.h:777 [inline] unix_stream_bpf_update_proto+0x72/0x430 net/unix/unix_bpf.c:171 sock_map_init_proto net/core/sock_map.c:190 [inline] sock_map_link+0xb87/0x1100 net/core/sock_map.c:294 sock_map_update_common+0xf6/0x870 net/core/sock_map.c:483 sock_map_update_elem_sys+0x5b6/0x640 net/core/sock_map.c:577 bpf_map_update_value+0x3af/0x820 kernel/bpf/syscall.c:167 We considered just checking for the null ptr and skipping taking a ref on the NULL peer sock. But, if the socket is then connected() after being added to the sockmap we can cause the original issue again. So instead this patch blocks adding af_unix sockets that are not in the ESTABLISHED state. Reported-by: Eric Dumazet <[email protected]> Reported-by: [email protected] Fixes: 8866730 ("bpf, sockmap: af_unix stream sockets need to hold ref for pair sock") Acked-by: Jakub Sitnicki <[email protected]> Signed-off-by: John Fastabend <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Martin KaFai Lau <[email protected]>
1 parent e307b5a commit 8d66506

File tree

2 files changed

+7
-0
lines changed

2 files changed

+7
-0
lines changed

include/net/sock.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,6 +2799,11 @@ static inline bool sk_is_tcp(const struct sock *sk)
27992799
return sk->sk_type == SOCK_STREAM && sk->sk_protocol == IPPROTO_TCP;
28002800
}
28012801

2802+
static inline bool sk_is_stream_unix(const struct sock *sk)
2803+
{
2804+
return sk->sk_family == AF_UNIX && sk->sk_type == SOCK_STREAM;
2805+
}
2806+
28022807
/**
28032808
* sk_eat_skb - Release a skb if it is no longer needed
28042809
* @sk: socket to eat this skb from

net/core/sock_map.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,8 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
536536
{
537537
if (sk_is_tcp(sk))
538538
return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);
539+
if (sk_is_stream_unix(sk))
540+
return (1 << sk->sk_state) & TCPF_ESTABLISHED;
539541
return true;
540542
}
541543

0 commit comments

Comments
 (0)