Re: [PATCH mptcp-net v3] mptcp: bpf: fix NULL deref and UAF in bpf_mptcp_sock_from_subflow()

[Paolo Abeni <pabeni@redhat.com>]

On 6/29/26 12:50 PM, Kalpan Jani wrote:
> bpf_mptcp_sock_from_subflow() is reachable from tracing BPF programs via
> bpf_skc_to_mptcp_sock() on an arbitrary socket, without the subflow
> socket lock held. It assumes sk_is_mptcp(sk) implies a valid subflow
> context whose ->conn points to a live parent mptcp_sock. That invariant
> does not hold in several windows:
> 
> - Fallback: subflow_ulp_fallback() clears icsk_ulp_data before clearing
>   tcp_sk(sk)->is_mptcp, so a reader can observe is_mptcp == 1 with a NULL
>   context and dereference mptcp_subflow_ctx(sk)->conn through NULL.
> 
> - Init: subflow_ulp_init() sets is_mptcp = 1 while ->conn is still NULL;
>   on CONFIG_DEBUG_NET, mptcp_sk() dereferences its argument in a
>   WARN_ON(), so mptcp_sk(NULL) faults.
> 
> - Publish: the passive MPC path in subflow_syn_recv_sock() stored the
>   freshly cloned parent into ->conn with a plain assignment, on a child
>   that is already hashed and globally visible. A lockless reader could
>   observe a non-NULL ->conn before the stores initialising the new
>   mptcp_sock were visible.
> 
> - Teardown: subflow_ulp_release() and mptcp_subflow_drop_ctx() dropped
>   the subflow-owned parent reference with sock_put() without clearing
>   ->conn, and the established parent msk was not SOCK_RCU_FREE, so a
>   lockless reader could dereference a freed parent: a use-after-free.

I think we should expose to tracing program arbitrary socket state
locklessly.

@Geliang: what about just removing such possibily? Something alike the 
following (completely untested).

If a tracing bpf program needs to access the msk, it should do just:

	mptcp_sk(sk)

on the main socket, via a new, to be defined/implemented helper, and
_non_ starting form a subflow.

AFAICS all the other accessors using from tracing are simple casts,
like the above, and do not traverse the rather risky ulp layer.

/P
---
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 75495a5c3507..ad4b8d04b190 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1745,8 +1745,6 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_skc_to_udp6_sock_proto;
 	case BPF_FUNC_skc_to_unix_sock:
 		return &bpf_skc_to_unix_sock_proto;
-	case BPF_FUNC_skc_to_mptcp_sock:
-		return &bpf_skc_to_mptcp_sock_proto;
 	case BPF_FUNC_sk_storage_get:
 		return &bpf_sk_storage_get_tracing_proto;
 	case BPF_FUNC_sk_storage_delete: