Re: [PATCH v1 bpf 1/2] bpf: tcp: Fix type confusion in bpf_tcp_sock().

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incomplete fix: `sol_tcp_sockopt()` remains vulnerable to slab out-of-bounds access.
- [High] Missed vulnerability: `bpf_skc_to_tcp_sock()` and `bpf_skc_to_tcp6_sock()` suffer from the exact same type confusion.
- [High] Missed vulnerability: `bpf_mptcp_sock_from_subflow()` suffers from the same type confusion and causes immediate OOB reads.
--

commit 72139ec7da09274112730adc692a99e89ef3eb2e
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Fix type confusion in bpf_tcp_sock().

This commit updates bpf_tcp_sock() to use sk_is_tcp(sk) instead of just
checking if the protocol is IPPROTO_TCP. This prevents RAW sockets from
causing out-of-bounds access to slab objects.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index bc96c18df4e03..cd88633f8dc14 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
>  
>  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
>  {
> -	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> +	if (sk_fullsock(sk) && sk_is_tcp(sk))
>  		return (unsigned long)sk;

Since ctx->sk evaluates to PTR_TO_SOCKET in a CGROUP_SOCKOPT program,
and is directly accepted by bpf_setsockopt() and bpf_getsockopt() via
ARG_PTR_TO_BTF_ID_SOCK_COMMON, does sol_tcp_sockopt() still remain
vulnerable?

Looking at sol_tcp_sockopt(), it appears to only check
sk->sk_protocol == IPPROTO_TCP. Can a user create a RAW socket with
IPPROTO_TCP and directly pass ctx->sk to bpf_setsockopt(), bypassing
bpf_tcp_sock() entirely and triggering the same slab out-of-bounds
access?

Additionally, do bpf_skc_to_tcp_sock() and bpf_skc_to_tcp6_sock() suffer
from the same type confusion?

They appear to continue checking sk->sk_protocol == IPPROTO_TCP without
verifying sk->sk_type == SOCK_STREAM. Could a BPF program pass a RAW
socket with IPPROTO_TCP to these helpers and receive a valid BTF
pointer to struct tcp_sock, allowing out-of-bounds slab reads?

Finally, does bpf_mptcp_sock_from_subflow() in net/mptcp/bpf.c also need
a similar update?

It checks sk->sk_protocol == IPPROTO_TCP and immediately evaluates
sk_is_mptcp(sk). Since sk_is_mptcp() indiscriminately casts the socket
to struct tcp_sock and accesses tcp_sk(sk)->is_mptcp, passing a RAW
socket could trigger an immediate out-of-bounds slab read.

If this memory happens to be non-zero, it might proceed to call
mptcp_subflow_ctx(sk)->conn, which would read a garbage pointer from
out-of-bounds memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430184405.1227386-1-kuniyu@google.com?part=1