Re: [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NULL pointer dereferences in bpf_mptcp_sock_from_subflow due to lockless concurrent accesses
- [High] Use-After-Free in BPF programs returning freed mptcp socket during teardown
--

commit 52dbcdaad7d49ff5db2faacf11c47b1c581024a8
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()

This commit fixes a type confusion issue in bpf_mptcp_sock_from_subflow() by
checking if the socket is actually a TCP socket before calling sk_is_mptcp().
Previously, raw sockets could bypass the IPPROTO_TCP check and cause incorrect
assumptions about the socket type.

> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 8a16672b94e2..4cc16cbeb328 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -14,7 +14,7 @@
>  
>  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))
> +	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
>  		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);

Could this cause a regression by dereferencing a NULL pointer when accessed
concurrently?

In subflow_ulp_fallback(), icsk->icsk_ulp_data is set to NULL prior to
clearing tcp_sk(sk)->is_mptcp. If a BPF program triggers in this window,
sk_is_mptcp() would return true but mptcp_subflow_ctx(sk) would return NULL,
leading to a NULL pointer dereference.

Additionally, could ctx->conn be NULL when read here?

During subflow_ulp_init(), the context is allocated and is_mptcp is set to 1,
but ctx->conn is initially NULL. Passing NULL to mptcp_sk(ctx->conn) triggers
a kernel panic on CONFIG_DEBUG_NET builds. On non-debug builds, it returns a
non-NULL offset pointer, which bypasses BPF verifier NULL checks and causes
invalid memory accesses.

Could this introduce a regression with a use-after-free of the mptcp_sock
during teardown?

During subflow_ulp_release(), sock_put(ctx->conn) drops the reference to the
parent mptcp_sock and may free it. However, ctx->conn is not cleared, and the
subflow context remains valid until the RCU grace period ends.

If a BPF tracing program triggers after sock_put(), could this retrieve
and return the dangling ctx->conn pointer, enabling the BPF program to read
from freed kernel slab memory?

>  
>  	return NULL;
>  }

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