[PATCH mptcp-net v4] mptcp: bpf: don't expose bpf_skc_to_mptcp_sock() to tracing progs

bpf_mptcp_sock_from_subflow() can be reached from tracing programs
via bpf_skc_to_mptcp_sock() on any socket, with no subflow lock held.
It assumes sk_is_mptcp(sk) means ->conn is a valid pointer to the
parent mptcp_sock, but that's not true during fallback, init, the
passive MPC path in subflow_syn_recv_sock(), and subflow teardown -
a lockless reader can hit a NULL ctx, a NULL ->conn, or a freed one.

sock_ops and cg_sockopt, the other two places this helper is
reachable from, always hold the subflow lock, so they don't have
this problem.

Drop it from tracing_prog_func_proto(), as suggested by Paolo.

Suggested-by: Paolo Abeni <[email protected]>
Fixes: 3bc253c2e652 ("bpf: Add bpf_skc_to_mptcp_sock_proto")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/622
Signed-off-by: Kalpan Jani <[email protected]>
---

Changes in v4:
- Instead of hardening the lockless read (rcu_dereference/acquire-
release/SOCK_RCU_FREE from v3), just drop tracing's access to the
helper, per Paolo. sock_ops and cg_sockopt already hold the subflow
lock so they're not affected, and nothing in-tree uses this from a
tracing program anyway. Patch is now a 2-line removal in
kernel/trace/bpf_trace.c, everything else from v3 is dropped.

Changes in v3:
- Fix the publish race in subflow_syn_recv_sock() that v2 missed
(Sashiko): the passive MPC path stored the freshly cloned parent into
->conn with a plain assignment on an already-hashed child. Publish with
smp_store_release() and pair the helper read with smp_load_acquire(), so
a lockless reader observing a non-NULL ->conn also sees the initialised
parent.
- Clear ->conn before sock_put() in mptcp_subflow_drop_ctx() too, not just
subflow_ulp_release(): it had the same stale-pointer teardown pattern.
- Audited the SOCK_RCU_FREE change: __mptcp_destroy_sock() does all msk
teardown before the final sock_put() and shares mptcp_destroy_common()
with the already-RCU-freed listener, so deferring the free is safe.

Changes in v2:
- Lockless access fixes (Li Xiasong): load the subflow context with
rcu_dereference_check() instead of a plain dereference, and read ->conn
once.
- Fix the teardown use-after-free that v1 did not address: clear ->conn
before dropping the parent reference in subflow_ulp_release(), and give
the parent msk RCU-grace lifetime via SOCK_RCU_FREE.
- Stop exposing the helper to sleepable BPF programs, where classic RCU
gives no lifetime guarantee.

v1: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/

 kernel/trace/bpf_trace.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 82f8feea6931..43a5517fde47 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:
-- 
2.43.0