[PATCH net] mptcp: fix use-after-free of the subflow context on the delegated list
From: Hyunwoo Kim <hidden>
Date: 2026-08-19 10:13:08
Also in:
mptcp
Subsystem:
networking [general], networking [mptcp], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthieu Baerts, Mat Martineau, Linus Torvalds
mptcp_subflow_delegate() links the subflow context into a per-CPU napi
list and takes a reference on the subflow socket. The context is a
separate allocation, reached through icsk_ulp_data, and the list holds
no reference to it.
inet_csk_destroy_sock() runs sk_prot->destroy() regardless of the
outstanding references. That path ends in tcp_cleanup_ulp(), which calls
subflow_ulp_release(). subflow_ulp_release() frees the context with
kfree_rcu() but does not remove it from the list. Once the grace period
elapses and the context is actually freed, the next mptcp_napi_poll()
dequeues that entry and mptcp_subflow_delegated_next() writes to the
freed context with list_del_init(). That is the use-after-free.
The freeing path cannot dequeue the entry itself: the list is per-CPU
and only the CPU that queued the subflow may touch it, as
tcp_release_cb_override() already notes.
Delay the free instead. Add MPTCP_DELEGATE_DEAD.
mptcp_subflow_free_ctx() sets it and frees the context only if the
subflow is not scheduled, while mptcp_napi_poll() frees it if the dead
bit is set when it clears the scheduled bit. Either
mptcp_subflow_free_ctx() gets there first, sees the scheduled bit and
leaves the free to mptcp_napi_poll(), or mptcp_napi_poll() gets there
first, does not see the dead bit and mptcp_subflow_free_ctx() does the
free.
Also skip mptcp_subflow_process_delegated() once the dead bit is set.
subflow_ulp_release() has already dropped the msk reference by then, so
the pending actions must not run.
Finally, move delegated_status out of struct_group(reset).
mptcp_subflow_ctx_reset() clears that group on disconnect, and clearing
the scheduled bit while the entry is still queued would let
mptcp_subflow_free_ctx() free a context the napi list still points to.
delegated_node is already outside the group.
Fixes: b19bc2945b40 ("mptcp: implement delegated actions")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <redacted>
---
net/mptcp/protocol.c | 13 ++++++++++---
net/mptcp/protocol.h | 16 ++++++++++++++--
net/mptcp/subflow.c | 4 ++--
3 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7c8180d8d5eff6..8aa276dfe88639 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c@@ -2641,7 +2641,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk, */ if (!inet_csk(ssk)->icsk_ulp_ops) { WARN_ON_ONCE(!sock_flag(ssk, SOCK_DEAD)); - kfree_rcu(subflow, rcu); + mptcp_subflow_free_ctx(subflow); } else { /* otherwise tcp will dispose of the ssk and subflow ctx */ __tcp_close(ssk, 0);
@@ -4617,10 +4617,13 @@ static int mptcp_napi_poll(struct napi_struct *napi, int budget) delegated = container_of(napi, struct mptcp_delegated_action, napi); while ((subflow = mptcp_subflow_delegated_next(delegated)) != NULL) { struct sock *ssk = mptcp_subflow_tcp_sock(subflow); + long status; bh_lock_sock_nested(ssk); if (!sock_owned_by_user(ssk)) { - mptcp_subflow_process_delegated(ssk, xchg(&subflow->delegated_status, 0)); + status = xchg(&subflow->delegated_status, 0); + if (!(status & BIT(MPTCP_DELEGATE_DEAD))) + mptcp_subflow_process_delegated(ssk, status); } else { /* tcp_release_cb_override already processed * the action or will do at next release_sock().
@@ -4628,11 +4631,15 @@ static int mptcp_napi_poll(struct napi_struct *napi, int budget) * CPU that scheduled it. */ smp_wmb(); - clear_bit(MPTCP_DELEGATE_SCHEDULED, &subflow->delegated_status); + status = set_mask_bits(&subflow->delegated_status, + BIT(MPTCP_DELEGATE_SCHEDULED), 0); } bh_unlock_sock(ssk); sock_put(ssk); + if (status & BIT(MPTCP_DELEGATE_DEAD)) + kfree_rcu(subflow, rcu); + if (++work_done == budget) return budget; }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 1b80f2d6ec5a23..df9d0bf127febd 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h@@ -538,8 +538,10 @@ DECLARE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions); #define MPTCP_DELEGATE_SEND 1 #define MPTCP_DELEGATE_ACK 2 #define MPTCP_DELEGATE_SNDBUF 3 +#define MPTCP_DELEGATE_DEAD 4 -#define MPTCP_DELEGATE_ACTIONS_MASK (~BIT(MPTCP_DELEGATE_SCHEDULED)) +#define MPTCP_DELEGATE_ACTIONS_MASK (~(BIT(MPTCP_DELEGATE_SCHEDULED) | \ + BIT(MPTCP_DELEGATE_DEAD))) /* MPTCP subflow context */ struct mptcp_subflow_context { struct list_head node;/* conn_list of subflows */
@@ -608,11 +610,11 @@ struct mptcp_subflow_context { u32 subflow_id; - long delegated_status; unsigned long fail_tout; ); + long delegated_status; struct list_head delegated_node; /* link into delegated_action, protected by local BH */ u32 setsockopt_seq;
@@ -788,6 +790,16 @@ mptcp_subflow_delegated_next(struct mptcp_delegated_action *delegated) return ret; } +static inline void mptcp_subflow_free_ctx(struct mptcp_subflow_context *subflow) +{ + long old = set_mask_bits(&subflow->delegated_status, 0, + BIT(MPTCP_DELEGATE_DEAD)); + + /* a scheduled subflow is owned by mptcp_napi_poll(), which will free it */ + if (!(old & BIT(MPTCP_DELEGATE_SCHEDULED))) + kfree_rcu(subflow, rcu); +} + void __mptcp_inherit_memcg(struct sock *sk, struct sock *ssk, gfp_t gfp); void __mptcp_inherit_cgrp_data(struct sock *sk, struct sock *ssk);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index e1f20ff8fdb424..b118594ab1b9a8 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c@@ -786,7 +786,7 @@ void mptcp_subflow_drop_ctx(struct sock *ssk) sock_put(ctx->conn); } - kfree_rcu(ctx, rcu); + mptcp_subflow_free_ctx(ctx); } void __mptcp_subflow_fully_established(struct mptcp_sock *msk,
@@ -2024,7 +2024,7 @@ static void subflow_ulp_release(struct sock *ssk) mptcp_subflow_ops_undo_override(ssk); if (release) - kfree_rcu(ctx, rcu); + mptcp_subflow_free_ctx(ctx); } static void subflow_ulp_clone(const struct request_sock *req,
--
2.43.0