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

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index bc96c18df4e0..cd88633f8dc1 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;
>
>  	return (unsigned long)NULL;

While this patch fixes bpf_tcp_sock(), does sol_tcp_sockopt() remain
vulnerable? 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, can a user create a RAW socket with
IPPROTO_TCP and directly pass ctx->sk to bpf_setsockopt(), bypassing
bpf_tcp_sock() entirely?

Looking at sol_tcp_sockopt():

static int sol_tcp_sockopt(struct sock *sk, int optname,
			   char *optval, int *optlen,
			   bool getopt)
{
	if (sk->sk_protocol != IPPROTO_TCP)
		return -EINVAL;

This appears to only check sk->sk_protocol == IPPROTO_TCP without verifying
sk->sk_type == SOCK_STREAM, which would allow the same slab out-of-bounds
access through a different code path.

Additionally, do bpf_skc_to_tcp_sock() and bpf_skc_to_tcp6_sock() suffer
from the same type confusion? They appear to check 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?

Looking at bpf_skc_to_tcp_sock():

BPF_CALL_1(bpf_skc_to_tcp_sock, struct sock *, sk)
{
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
		return (unsigned long)sk;

	return (unsigned long)NULL;
}

And bpf_skc_to_tcp6_sock():

BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
{
	/* tcp6_sock type is not generated in dwarf and hence btf,
	 * trigger an explicit type generation here.
	 */
	BTF_TYPE_EMIT(struct tcp6_sock);
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
	    sk->sk_family == AF_INET6)
		return (unsigned long)sk;

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:

struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
{
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);

	return NULL;
}

These issues were raised by sashiko-bot@kernel.org in v1:
https://lore.kernel.org/bpf/20260501184452.7F46FC2BCB4@smtp.kernel.org/


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25344293379