Series comparison

bpf_mptcp_sock_from_subflow() is reachable from 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 three windows:

- Fallback: subflow_ulp_fallback() clears icsk_ulp_data via
  rcu_assign_pointer(..., NULL) before clearing tcp_sk(sk)->is_mptcp, so a
  concurrent reader can observe is_mptcp == 1 with a NULL context and
  dereference mptcp_subflow_ctx(sk)->conn through a NULL pointer.

- Init: subflow_ulp_init() sets is_mptcp = 1 while ->conn is still NULL;
  ->conn is only assigned later in subflow_syn_recv_sock() or
  mptcp_subflow_create_socket(). On CONFIG_DEBUG_NET, mptcp_sk()
  dereferences its argument in a WARN_ON(), so mptcp_sk(NULL) faults.

- Teardown: subflow_ulp_release() drops the subflow-owned parent
  reference with sock_put(ctx->conn) without clearing ctx->conn. The
  established parent msk is not SOCK_RCU_FREE (mptcp_sk_clone_init()
  resets the flag), so it is freed synchronously and a lockless reader can
  dereference a dangling ->conn: a use-after-free.

icsk_ulp_data is an __rcu pointer and this helper can run locklessly, so
load the context with rcu_dereference_check() (lockdep_sock_is_held()
covers the locked struct_ops callers), load ->conn once with READ_ONCE(),
pair the ->conn stores with WRITE_ONCE(), reject a NULL context or NULL
->conn, and clear ->conn before dropping the parent reference.

A NULL check alone cannot fix the teardown case: a reader may load a
non-NULL ->conn just before it is cleared and dereference the parent
after sock_put() has freed it. Give the parent msk RCU-grace lifetime by
setting SOCK_RCU_FREE in the shared __mptcp_init_sock(), covering both the
active and passive paths, and dropping the reset in mptcp_sk_clone_init().
Non-sleepable BPF runs in an RCU read-side critical section, so a reader
that obtains the msk is then guaranteed it stays allocated for the
program's duration.

That lifetime guarantee relies on classic RCU, which does not hold off RCU
Tasks Trace, so it does not extend to sleepable programs. Unlike the other
bpf_skc_to_*() casts, which return the same object as the trusted input
sk, this helper returns a different object, so stop exposing it to
sleepable programs in tracing_prog_func_proto().

Fixes: 3bc253c2e652 ("bpf: Add bpf_skc_to_mptcp_sock_proto")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/622
Assisted-by: Claude:claude-opus-4.8.
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
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 with READ_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 so a lockless reader
  cannot dereference a freed parent.
- 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/

kernel/trace/bpf_trace.c |  2 +-
 net/mptcp/bpf.c          | 14 ++++++++++++--
 net/mptcp/protocol.c     |  6 +++++-
 net/mptcp/subflow.c      |  9 +++++++--
 4 files changed, 25 insertions(+), 6 deletions(-)

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.

Load the context with rcu_dereference() and read ->conn with
smp_load_acquire(), paired with an smp_store_release() on the publish so
a reader observing a non-NULL ->conn also observes the initialised
parent. Reject a NULL context or NULL ->conn before casting. Clear ->conn
before dropping the parent reference in both release paths, and give the
parent msk RCU-grace lifetime by setting SOCK_RCU_FREE in
__mptcp_init_sock().

Commit 5e20087d1b67 ("mptcp: handle mptcp listener destruction via rcu")
already makes the listener msk SOCK_RCU_FREE for its lockless ->conn
reader and resets the flag on the accepted child, which had no such
reader. This helper adds one to the accepted msk; __mptcp_destroy_sock()
performs all msk teardown before the final sock_put() and shares
mptcp_destroy_common() with the already-RCU-freed listener, so extending
the same lifetime is safe. Drop the reset.

Non-sleepable BPF runs in an RCU read-side critical section, so a reader
that obtains the msk is guaranteed it stays allocated for the program's
duration. That relies on classic RCU and does not hold off RCU Tasks
Trace, so it does not extend to sleepable programs. Unlike the other
bpf_skc_to_*() casts, which return the same object as the input sk, this
helper returns a different object, so stop exposing it to sleepable
programs in tracing_prog_func_proto().

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 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/

kernel/trace/bpf_trace.c |  2 +-
 net/mptcp/bpf.c          | 14 ++++++++++++--
 net/mptcp/protocol.c     |  6 +++++-
 net/mptcp/subflow.c      | 25 ++++++++++++++++++-------
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index XXXXXXX..XXXXXXX 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -XXX,XX +XXX,XX @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	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;
+		return prog->sleepable ? NULL : &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:
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -XXX,XX +XXX,XX @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = {
 
 struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
 {
-	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
-		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
+	struct mptcp_subflow_context *ctx;
+	struct sock *conn;
+
+	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk)) {
+		ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
+		if (ctx) {
+			/* pairs with smp_store_release() when ->conn is
+			 * published in subflow_syn_recv_sock()
+			 */
+			conn = smp_load_acquire(&ctx->conn);
+			if (conn)
+				return mptcp_sk(conn);
+		}
+	}
 
 	return NULL;
 }
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -XXX,XX +XXX,XX @@ static void __mptcp_init_sock(struct sock *sk)
 	mptcp_pm_data_init(msk);
 	spin_lock_init(&msk->fallback_lock);
 
+	/* the returned parent msk may be handed to lockless readers
+	 * (bpf_skc_to_mptcp_sock()); free it after an RCU grace period
+	 */
+	sock_set_flag(sk, SOCK_RCU_FREE);
+
 	/* re-use the csk retrans timer for MPTCP-level retrans */
 	timer_setup(&sk->mptcp_retransmit_timer, mptcp_retransmit_timer, 0);
 	timer_setup(&msk->sk.mptcp_tout_timer, mptcp_tout_timer, 0);
@@ -XXX,XX +XXX,XX @@ struct sock *mptcp_sk_clone_init(const struct sock *sk,
 	/* passive msk is created after the first/MPC subflow */
 	msk->subflow_id = 2;
 
-	sock_reset_flag(nsk, SOCK_RCU_FREE);
 	security_inet_csk_clone(nsk, req);
 
 	/* this can't race with mptcp_close(), as the msk is
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -XXX,XX +XXX,XX @@ void mptcp_subflow_drop_ctx(struct sock *ssk)
 	list_del(&mptcp_subflow_ctx(ssk)->node);
 	if (inet_csk(ssk)->icsk_ulp_ops) {
 		subflow_ulp_fallback(ssk, ctx);
-		if (ctx->conn)
-			sock_put(ctx->conn);
+		if (ctx->conn) {
+			struct sock *conn = ctx->conn;
+
+			WRITE_ONCE(ctx->conn, NULL);
+			sock_put(conn);
+		}
 	}
 
 	kfree_rcu(ctx, rcu);
@@ -XXX,XX +XXX,XX @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 	bool fallback, fallback_is_fatal;
 	enum sk_rst_reason reason;
 	struct mptcp_sock *owner;
+	struct sock *conn;
 	struct sock *child;
 
 	pr_debug("listener=%p, req=%p, conn=%p\n", listener, req, listener->conn);
@@ -XXX,XX +XXX,XX @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 		ctx->setsockopt_seq = listener->setsockopt_seq;
 
 		if (ctx->mp_capable) {
-			ctx->conn = mptcp_sk_clone_init(listener->conn, &mp_opt, child, req);
-			if (!ctx->conn)
+			conn = mptcp_sk_clone_init(listener->conn, &mp_opt, child, req);
+			if (!conn)
 				goto fallback;
 
 			ctx->subflow_id = 1;
-			owner = mptcp_sk(ctx->conn);
+			owner = mptcp_sk(conn);
+			/* publish the fully initialised parent; pairs with
+			 * smp_load_acquire() in bpf_mptcp_sock_from_subflow()
+			 */
+			smp_store_release(&ctx->conn, conn);
 
 			if (mp_opt.deny_join_id0)
 				WRITE_ONCE(owner->pm.remote_deny_join_id0, true);
@@ -XXX,XX +XXX,XX @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 
 			/* move the msk reference ownership to the subflow */
 			subflow_req->msk = NULL;
-			ctx->conn = (struct sock *)owner;
+			WRITE_ONCE(ctx->conn, (struct sock *)owner);
 
 			if (subflow_use_different_sport(owner, sk)) {
 				pr_debug("ack inet_sport=%d %d\n",
@@ -XXX,XX +XXX,XX @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
 
 	*new_sock = sf;
 	sock_hold(sk);
-	subflow->conn = sk;
+	WRITE_ONCE(subflow->conn, sk);
 	mptcp_subflow_ops_override(sf->sk);
 
 	return 0;
@@ -XXX,XX +XXX,XX @@ static void subflow_ulp_release(struct sock *ssk)
 		if (!release && !test_and_set_bit(MPTCP_WORK_CLOSE_SUBFLOW,
 						  &mptcp_sk(sk)->flags))
 			mptcp_schedule_work(sk);
+
+		/* hide the parent from lockless readers (e.g. BPF) before
+		 * dropping the subflow-owned reference
+		 */
+		WRITE_ONCE(ctx->conn, NULL);
 		sock_put(sk);
 	}
 
-- 
2.43.0