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

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

[Kalpan Jani]

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 <pabeni@redhat.com>
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 <kalpan.jani@mpiricsoftware.com>
---

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/20260612072643.2313900-1-kalpan.jani@mpiricsoftware.com/
v2: https://lore.kernel.org/all/20260626125058.868855-1-kalpan.jani@mpiricsoftware.com/
v3: https://lore.kernel.org/all/20260629105020.1670781-1-kalpan.jani@mpiricsoftware.com/

 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch attempts to fix a Use-After-Free / NULL pointer dereference in `bpf_skc_to_mptcp_sock()` by restricting its use to BPF hooks that hold the subflow lock (e.g., `sock_ops`, `cg_sockopt`), thereby removing it from `tracing_prog_func_proto()`. However, the fix is fundamentally incomplete for two reasons. First, the helper remains exported via `bpf_sk_base_func_proto()`, which serves as the default for numerous datapath hooks including XDP (`xdp_func_proto()`) and TC (`tc_cls_act_func_proto()`). Second, even in locked hooks like `sock_ops`, a BPF program can perform a lockless socket lookup (e.g., via `bpf_sk_lookup_tcp()`) and pass the resulting unlocked socket to the helper. Since the helper accepts any `ARG_PTR_TO_SOCK_COMMON` regardless of whether it is the context socket, and the lockless read hardening from v3 was dropped in this patch, this allows the exact same lockless Use-After-Free or NULL pointer dereference on `mptcp_subflow_ctx(sk)->conn` during socket fallback, initialization, or teardown.
--

--- Patch [1]: [PATCH mptcp-net v4] mptcp: bpf: don't expose bpf_skc_to_mptcp_sock() to tracing progs ---
commit 5acf7057a22cd1b7a4c59432f2d0744627b03558
Author: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

mptcp: bpf: don't expose bpf_skc_to_mptcp_sock() to tracing progs

The patch drops bpf_skc_to_mptcp_sock() from tracing_prog_func_proto() to
prevent lockless access to mptcp_subflow_ctx(sk)->conn during subflow
fallback, init, and teardown.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/622

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 75495a5c35079..ad4b8d04b190a 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;

[Severity: Critical]
This is a pre-existing issue, but does removing this helper from tracing
programs completely prevent the lockless dereference regression?

The helper remains exported via bpf_sk_base_func_proto() in net/core/filter.c,
which makes it available to datapath hooks like XDP and TC that do not hold
the subflow lock.

Additionally, even in hooks that do hold the subflow lock for the context
socket (like sock_ops), a BPF program can perform a lockless socket lookup
(e.g., using bpf_sk_lookup_tcp()) and pass that unlocked socket to the helper.

Since the lockless read hardening was dropped from v3 of this patch, the
underlying helper in net/mptcp/bpf.c:bpf_mptcp_sock_from_subflow() still
performs:

    if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
        return mptcp_sk(mptcp_subflow_ctx(sk)->conn);

Could this still allow a use-after-free or NULL pointer dereference on
mptcp_subflow_ctx(sk)->conn when an unlocked socket is passed during socket
fallback, initialization, or teardown?

>  	case BPF_FUNC_sk_storage_get:
>  		return &bpf_sk_storage_get_tracing_proto;
>  	case BPF_FUNC_sk_storage_delete:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817113202.1832692-1-kalpan.jani@mpiricsoftware.com?part=1

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

[MPTCP CI]

Hi Kalpan,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/32026702788

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/51b239585c5b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1147170


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)