[PATCH net 2/2] mptcp: diag: fix unbounded listener bucket lock hold
Zihan Xi <[email protected]> Wed, 29 Jul 2026 11:28:40 +0000
| Newsgroups | dev.linux.lists.mptcp,org.kernel.vger.netdev |
|---|---|
| Message-ID | <4632ca2134dbd3947529180ba8b8ba4b467c2098.1785307984.git.zihanx@nebusec.ai> |
MPTCP listener diag dumping reuses sk_diag_dump(), which executes
inet_diag_bc_sk() before filling the netlink reply. The listener walk in
mptcp_diag_dump_listeners() currently performs MPTCP listener selection
and bytecode-triggering dump work while holding the listener bucket lock.
A dump request can therefore force unbounded per-bucket lock hold by
arranging for many listeners in the same bucket to miss the MPTCP or
socket-attribute prefilters, so the old 16-entry batching limit no longer
bounds the locked scan itself.
Fix this by collecting only referenced listener sockets under the bucket
lock. Re-check the MPTCP listener properties, obtain the parent MPTCP
socket reference, and run sk_diag_dump() only after dropping the lock.
Keep a referenced dump cursor so the next batch resumes after the previous
listener instead of rescanning the bucket head under the listener lock.
Fixes: 4fa39b701ce9 ("mptcp: listen diag dump support")
Cc: [email protected]
Reported-by: Vega <[email protected]>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <[email protected]>
---
net/mptcp/mptcp_diag.c | 124 +++++++++++++++++++++++++++++------------
1 file changed, 87 insertions(+), 37 deletions(-)
diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0ee..981e64343039 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -6,12 +6,25 @@
* Author: Paolo Abeni <[email protected]>
*/
+/* Process a bounded number of listeners per bucket lock hold. */
+#define MPTCP_DIAG_BULK_SZ 16
+
#include <linux/kernel.h>
#include <linux/net.h>
#include <linux/inet_diag.h>
#include <net/netlink.h>
#include "protocol.h"
+static void mptcp_diag_save_cursor(struct inet_diag_dump_data *cb_data,
+ unsigned int slot, struct sock *sk)
+{
+ sock_hold(sk);
+ inet_diag_dump_clear_cursor(cb_data);
+ cb_data->dump_cursor = sk;
+ cb_data->dump_cursor_slot = slot;
+ cb_data->dump_cursor_type = INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN;
+}
+
static int sk_diag_dump(struct sock *sk, struct sk_buff *skb,
struct netlink_callback *cb,
const struct inet_diag_req_v2 *req,
@@ -77,6 +90,7 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
bool net_admin)
{
struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
+ struct inet_diag_dump_data *cb_data = cb->data;
struct net *net = sock_net(skb->sk);
struct inet_hashinfo *hinfo;
int i;
@@ -86,62 +100,98 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
for (i = diag_ctx->l_slot; i <= hinfo->lhash2_mask; i++) {
struct inet_listen_hashbucket *ilb;
struct hlist_nulls_node *node;
- struct sock *sk;
- int num = 0;
-
+ struct sock *tmp, *sk, *sk_arr[MPTCP_DIAG_BULK_SZ];
+ struct sock *cursor;
+ int accum, idx, num, ret;
+ int num_arr[MPTCP_DIAG_BULK_SZ];
+ bool use_cursor;
+
+resume_listen_walk:
+ num = 0;
+ accum = 0;
ilb = &hinfo->lhash2[i];
+ ret = 0;
rcu_read_lock();
spin_lock(&ilb->lock);
- sk_nulls_for_each(sk, node, &ilb->nulls_head) {
- const struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(sk);
- struct inet_sock *inet = inet_sk(sk);
- int ret;
-
- if (num < diag_ctx->l_num)
- goto next_listen;
-
- if (!ctx || strcmp(inet_csk(sk)->icsk_ulp_ops->name, "mptcp"))
- goto next_listen;
-
- sk = ctx->conn;
- if (!sk || !net_eq(sock_net(sk), net))
- goto next_listen;
-
- if (r->sdiag_family != AF_UNSPEC &&
- sk->sk_family != r->sdiag_family)
- goto next_listen;
-
- if (r->id.idiag_sport != inet->inet_sport &&
- r->id.idiag_sport)
+ cursor = cb_data->dump_cursor;
+ use_cursor = cursor &&
+ cb_data->dump_cursor_type ==
+ INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN &&
+ cb_data->dump_cursor_slot == i &&
+ !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
+ cursor->sk_nulls_node.pprev != LIST_POISON2;
+ node = use_cursor ? cursor->sk_nulls_node.next :
+ ilb->nulls_head.first;
+ hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {
+ if (!use_cursor && num < diag_ctx->l_num)
goto next_listen;
if (!refcount_inc_not_zero(&sk->sk_refcnt))
goto next_listen;
- ret = sk_diag_dump(sk, skb, cb, r, net_admin);
-
- sock_put(sk);
-
- if (ret < 0) {
- spin_unlock(&ilb->lock);
- rcu_read_unlock();
- diag_ctx->l_slot = i;
- diag_ctx->l_num = num;
- return;
- }
- diag_ctx->l_num = num + 1;
- num = 0;
+ num_arr[accum] = num;
+ sk_arr[accum] = sk;
+ if (++accum == MPTCP_DIAG_BULK_SZ)
+ break;
next_listen:
++num;
}
spin_unlock(&ilb->lock);
rcu_read_unlock();
+ for (idx = 0; idx < accum; idx++) {
+ const struct tcp_ulp_ops *ulp_ops;
+ const struct mptcp_subflow_context *ctx;
+ struct inet_sock *inet;
+
+ sk = sk_arr[idx];
+ rcu_read_lock();
+ ctx = mptcp_subflow_ctx(sk);
+ ulp_ops = READ_ONCE(inet_csk(sk)->icsk_ulp_ops);
+ inet = inet_sk(sk);
+ tmp = ctx ? ctx->conn : NULL;
+ if (!ctx || !ulp_ops || strcmp(ulp_ops->name, "mptcp") ||
+ !tmp || !net_eq(sock_net(tmp), net) ||
+ (r->sdiag_family != AF_UNSPEC &&
+ tmp->sk_family != r->sdiag_family) ||
+ (r->id.idiag_sport != inet->inet_sport &&
+ r->id.idiag_sport) ||
+ !refcount_inc_not_zero(&tmp->sk_refcnt)) {
+ rcu_read_unlock();
+ goto processed_listener_sk;
+ }
+ rcu_read_unlock();
+ if (ret >= 0) {
+ ret = sk_diag_dump(tmp, skb, cb, r, net_admin);
+ if (ret < 0)
+ num = num_arr[idx];
+ }
+ sock_put(tmp);
+processed_listener_sk:
+ if (ret >= 0)
+ mptcp_diag_save_cursor(cb_data, i, sk);
+ sock_put(sk);
+ }
+
+ if (ret < 0) {
+ diag_ctx->l_slot = i;
+ diag_ctx->l_num = num;
+ return;
+ }
+
cond_resched();
+
+ if (accum == MPTCP_DIAG_BULK_SZ) {
+ diag_ctx->l_num = 0;
+ goto resume_listen_walk;
+ }
+
+ inet_diag_dump_clear_cursor(cb_data);
diag_ctx->l_num = 0;
}
+ inet_diag_dump_clear_cursor(cb_data);
diag_ctx->l_num = 0;
diag_ctx->l_slot = i;
}
--
2.43.0