[PATCH net] tipc: serialize publication purging with name table updates

[PATCH net] tipc: serialize publication purging with name table updates

[Chengfeng Ye]

tipc_publ_notify() walks a failed node's publication list after the node
lock has been released. Its list iterator is not protected by the name
table lock, which is only acquired inside tipc_publ_purge().

CPU A can save the next publication before entering tipc_publ_purge().
CPU B then takes nametbl_lock in tipc_named_rcv(), processes a WITHDRAWAL
for that publication, unlinks it with list_del_init(), and queues it for
freeing with kfree_rcu(). CPU A advances to the removed publication and
loops on its self-linked binding_node, stalling the CPU.

The kernel reported:

  rcu: INFO: rcu_sched self-detected stall on CPU
  Call Trace:
   tipc_publ_notify+0x3b5/0x650
   tipc_node_write_unlock+0x49d/0x5d0
   tipc_node_link_down+0x15c/0x4a0
   tipc_node_delete_links+0xfc/0x190
   bearer_disable+0x111/0x270
   __tipc_nl_bearer_disable+0x1db/0x2f0
   tipc_nl_bearer_disable+0x1c/0x30

Locking the entire traversal would also prevent the race, but would hold
nametbl_lock with bottom halves disabled while purging every publication.
A node with many publications could therefore cause excessive lock hold
times and delay other name-table operations.

Move the failed node's publications to a private list under nametbl_lock,
then select and unlink its first entry under the same lock before purging
it. Concurrent withdrawals can still unlink entries from this private
list, while publications arriving after node recovery stay on the node's
live list. No publication pointer is carried across an unlocked interval.

Unlink before the table lookup to make progress even if the lookup fails,
and acquire and release the lock for each publication. Check the private
list under the lock on every iteration, since concurrent withdrawals can
still remove entries. Keep withdrawal notifications and the final
rc_dests update in their existing order.

Remove the failed-node address argument from tipc_publ_notify() and its
purge helper since unlinking no longer needs a node lookup.

Fixes: 9db9fdd1983e ("tipc: avoid to asynchronously notify subscriptions")
Cc: stable@vger.kernel.org
Assisted-by: GPT-6-Astra
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/tipc/name_distr.c | 34 +++++++++++++++++++++++-----------
 net/tipc/name_distr.h |  2 +-
 net/tipc/node.c       |  2 +-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index ba4f4906e13b..34867b69044c 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -227,38 +227,50 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
  * tipc_publ_purge - remove publication associated with a failed node
  * @net: the associated network namespace
  * @p: the publication to remove
- * @addr: failed node's address
  *
  * Invoked for each publication issued by a newly failed node.
  * Removes publication structure from name table & deletes it.
+ * The caller must hold nametbl_lock and unlink the node subscription.
  */
-static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
+static void tipc_publ_purge(struct net *net, struct publication *p)
 {
-	struct tipc_net *tn = tipc_net(net);
 	struct publication *_p;
 	struct tipc_uaddr ua;
 
 	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type,
 		   p->sr.lower, p->sr.upper);
-	spin_lock_bh(&tn->nametbl_lock);
 	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
-	if (_p)
-		tipc_node_unsubscribe(net, &_p->binding_node, addr);
-	spin_unlock_bh(&tn->nametbl_lock);
 	if (_p)
 		kfree_rcu(_p, rcu);
 }
 
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
-		      u32 addr, u16 capabilities)
+		      u16 capabilities)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
 
-	struct publication *publ, *tmp;
+	struct publication *publ;
+	LIST_HEAD(purge_list);
+
+	spin_lock_bh(&tn->nametbl_lock);
+	/* Leave new publications on the node's list during the purge. */
+	list_splice_init(nsub_list, &purge_list);
+	spin_unlock_bh(&tn->nametbl_lock);
+
+	for (;;) {
+		spin_lock_bh(&tn->nametbl_lock);
+		if (list_empty(&purge_list)) {
+			spin_unlock_bh(&tn->nametbl_lock);
+			break;
+		}
+		publ = list_first_entry(&purge_list, struct publication,
+					binding_node);
+		list_del_init(&publ->binding_node);
+		tipc_publ_purge(net, publ);
+		spin_unlock_bh(&tn->nametbl_lock);
+	}
 
-	list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
-		tipc_publ_purge(net, publ, addr);
 	spin_lock_bh(&tn->nametbl_lock);
 	if (!(capabilities & TIPC_NAMED_BCAST))
 		nt->rc_dests--;
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index c677f6f082df..8debe23469b2 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -74,6 +74,6 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq,
 		    u16 *rcv_nxt, bool *open);
 void tipc_named_reinit(struct net *net);
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
-		      u32 addr, u16 capabilities);
+		      u16 capabilities);
 
 #endif
diff --git a/net/tipc/node.c b/net/tipc/node.c
index bd91378b7540..9a218d137c45 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -422,7 +422,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
 	write_unlock_bh(&n->lock);
 
 	if (flags & TIPC_NOTIFY_NODE_DOWN)
-		tipc_publ_notify(net, publ_list, node, n->capabilities);
+		tipc_publ_notify(net, publ_list, n->capabilities);
 
 	if (flags & TIPC_NOTIFY_NODE_UP)
 		tipc_named_node_up(net, node, n->capabilities);
-- 
2.43.0

RE: [PATCH net] tipc: serialize publication purging with name table updates

[Tung Quang Nguyen]

>Subject: [PATCH net] tipc: serialize publication purging with name table updates
>
>tipc_publ_notify() walks a failed node's publication list after the node lock has
>been released. Its list iterator is not protected by the name table lock, which is
>only acquired inside tipc_publ_purge().
>
>CPU A can save the next publication before entering tipc_publ_purge().
>CPU B then takes nametbl_lock in tipc_named_rcv(), processes a
>WITHDRAWAL for that publication, unlinks it with list_del_init(), and queues it
>for freeing with kfree_rcu(). CPU A advances to the removed publication and
>loops on its self-linked binding_node, stalling the CPU.
>
>The kernel reported:
>
>  rcu: INFO: rcu_sched self-detected stall on CPU
>  Call Trace:
>   tipc_publ_notify+0x3b5/0x650
>   tipc_node_write_unlock+0x49d/0x5d0
>   tipc_node_link_down+0x15c/0x4a0
>   tipc_node_delete_links+0xfc/0x190
>   bearer_disable+0x111/0x270
>   __tipc_nl_bearer_disable+0x1db/0x2f0
>   tipc_nl_bearer_disable+0x1c/0x30
>

Please update your changelog with decoded stack trace.

>Locking the entire traversal would also prevent the race, but would hold
>nametbl_lock with bottom halves disabled while purging every publication.
>A node with many publications could therefore cause excessive lock hold times
>and delay other name-table operations.
>
>Move the failed node's publications to a private list under nametbl_lock, then
>select and unlink its first entry under the same lock before purging it.
>Concurrent withdrawals can still unlink entries from this private list, while
>publications arriving after node recovery stay on the node's live list. No
>publication pointer is carried across an unlocked interval.
>
>Unlink before the table lookup to make progress even if the lookup fails, and
>acquire and release the lock for each publication. Check the private list under
>the lock on every iteration, since concurrent withdrawals can still remove
>entries. Keep withdrawal notifications and the final rc_dests update in their
>existing order.
>
>Remove the failed-node address argument from tipc_publ_notify() and its
>purge helper since unlinking no longer needs a node lookup.
>
>Fixes: 9db9fdd1983e ("tipc: avoid to asynchronously notify subscriptions")
>Cc: stable@vger.kernel.org
>Assisted-by: GPT-6-Astra
>Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
>---
> net/tipc/name_distr.c | 34 +++++++++++++++++++++++-----------
> net/tipc/name_distr.h |  2 +-
> net/tipc/node.c       |  2 +-
> 3 files changed, 25 insertions(+), 13 deletions(-)
>
>diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index
>ba4f4906e13b..34867b69044c 100644
>--- a/net/tipc/name_distr.c
>+++ b/net/tipc/name_distr.c
>@@ -227,38 +227,50 @@ void tipc_named_node_up(struct net *net, u32
>dnode, u16 capabilities)
>  * tipc_publ_purge - remove publication associated with a failed node
>  * @net: the associated network namespace
>  * @p: the publication to remove
>- * @addr: failed node's address
>  *
>  * Invoked for each publication issued by a newly failed node.
>  * Removes publication structure from name table & deletes it.
>+ * The caller must hold nametbl_lock and unlink the node subscription.
>  */
>-static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
>+static void tipc_publ_purge(struct net *net, struct publication *p)
> {
>-	struct tipc_net *tn = tipc_net(net);
> 	struct publication *_p;
> 	struct tipc_uaddr ua;
>
> 	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type,
> 		   p->sr.lower, p->sr.upper);
>-	spin_lock_bh(&tn->nametbl_lock);
> 	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
>-	if (_p)
>-		tipc_node_unsubscribe(net, &_p->binding_node, addr);
>-	spin_unlock_bh(&tn->nametbl_lock);
> 	if (_p)
> 		kfree_rcu(_p, rcu);
> }
>
> void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
>-		      u32 addr, u16 capabilities)
>+		      u16 capabilities)
> {
> 	struct name_table *nt = tipc_name_table(net);
> 	struct tipc_net *tn = tipc_net(net);
>
>-	struct publication *publ, *tmp;
>+	struct publication *publ;
>+	LIST_HEAD(purge_list);
>+
>+	spin_lock_bh(&tn->nametbl_lock);
>+	/* Leave new publications on the node's list during the purge. */
>+	list_splice_init(nsub_list, &purge_list);
>+	spin_unlock_bh(&tn->nametbl_lock);

After this lock is released, new publications can be inserted into node->publ_list. Does this defeat the purpose of current publication release ?

>+
>+	for (;;) {
>+		spin_lock_bh(&tn->nametbl_lock);
>+		if (list_empty(&purge_list)) {
>+			spin_unlock_bh(&tn->nametbl_lock);
>+			break;
>+		}
>+		publ = list_first_entry(&purge_list, struct publication,
>+					binding_node);
>+		list_del_init(&publ->binding_node);
>+		tipc_publ_purge(net, publ);
>+		spin_unlock_bh(&tn->nametbl_lock);
>+	}
>
>-	list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
>-		tipc_publ_purge(net, publ, addr);
> 	spin_lock_bh(&tn->nametbl_lock);
> 	if (!(capabilities & TIPC_NAMED_BCAST))
> 		nt->rc_dests--;
>diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index
>c677f6f082df..8debe23469b2 100644
>--- a/net/tipc/name_distr.h
>+++ b/net/tipc/name_distr.h
>@@ -74,6 +74,6 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head
>*namedq,
> 		    u16 *rcv_nxt, bool *open);
> void tipc_named_reinit(struct net *net);  void tipc_publ_notify(struct net *net,
>struct list_head *nsub_list,
>-		      u32 addr, u16 capabilities);
>+		      u16 capabilities);
>
> #endif
>diff --git a/net/tipc/node.c b/net/tipc/node.c index
>bd91378b7540..9a218d137c45 100644
>--- a/net/tipc/node.c
>+++ b/net/tipc/node.c
>@@ -422,7 +422,7 @@ static void tipc_node_write_unlock(struct tipc_node
>*n)
> 	write_unlock_bh(&n->lock);
>
> 	if (flags & TIPC_NOTIFY_NODE_DOWN)
>-		tipc_publ_notify(net, publ_list, node, n->capabilities);
>+		tipc_publ_notify(net, publ_list, n->capabilities);
>
> 	if (flags & TIPC_NOTIFY_NODE_UP)
> 		tipc_named_node_up(net, node, n->capabilities);
>--
>2.43.0

Re: [PATCH net] tipc: serialize publication purging with name table updates

[netdev-bot+sashiko]

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

Critical: 1 · High: 0 · 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).

- [Critical] In tipc_update_nametbl() (net/tipc/name_distr.c), the
  WITHDRAWAL branch frees a publication with kfree_rcu(p, rcu) whether…

--

tipc: serialize publication purging with name table updates

This patch changes tipc_publ_notify() so that it moves a failed node's
publications onto a private on-stack list while holding nametbl_lock. It
then unlinks and purges them one at a time, taking and releasing the lock
for each entry. The addr argument is dropped from tipc_publ_notify() and
tipc_publ_purge() because the unlink no longer needs a node lookup.

> diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
> index ba4f4906e13b7..34867b69044c0 100644
> --- a/net/tipc/name_distr.c
> +++ b/net/tipc/name_distr.c
> @@ -227,38 +227,50 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)

[ ... ]

>  void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
> -		      u32 addr, u16 capabilities)
> +		      u16 capabilities)
>  {
>  	struct name_table *nt = tipc_name_table(net);
>  	struct tipc_net *tn = tipc_net(net);
>  
> -	struct publication *publ, *tmp;
> +	struct publication *publ;
> +	LIST_HEAD(purge_list);
> +
> +	spin_lock_bh(&tn->nametbl_lock);
> +	/* Leave new publications on the node's list during the purge. */
> +	list_splice_init(nsub_list, &purge_list);
> +	spin_unlock_bh(&tn->nametbl_lock);
> +
> +	for (;;) {
> +		spin_lock_bh(&tn->nametbl_lock);
> +		if (list_empty(&purge_list)) {
> +			spin_unlock_bh(&tn->nametbl_lock);
> +			break;
> +		}
> +		publ = list_first_entry(&purge_list, struct publication,
> +					binding_node);
> +		list_del_init(&publ->binding_node);
> +		tipc_publ_purge(net, publ);
> +		spin_unlock_bh(&tn->nametbl_lock);
> +	}

[Severity: Critical]

The commit message says "Concurrent withdrawals can still unlink entries
from this private list". Is that always the case?

The WITHDRAWAL branch in tipc_update_nametbl() frees the publication even
when binding_node was never unlinked:

net/tipc/name_distr.c:tipc_update_nametbl() {
    ...
	} else if (dtype == WITHDRAWAL) {
		p = tipc_nametbl_remove_publ(net, &ua, &sk, key);
		if (p) {
			tipc_node_unsubscribe(net, &p->binding_node, node);
			kfree_rcu(p, rcu);
			return true;
		}
    ...
}

tipc_node_unsubscribe() returns without calling list_del_init() in two
cases:

net/tipc/node.c:tipc_node_unsubscribe() {
	if (in_own_node(net, addr))
		return;

	n = tipc_node_find(net, addr);
	if (!n) {
		pr_warn("Node unsubscribe rejected, unknown node 0x%x\n", addr);
		return;
	}
    ...
}

In both cases the freed publication stays linked on n->publ_list. This
loop then splices it into purge_list, writes to it with list_del_init(),
and reads p->scope, p->sr, p->sk and p->key in tipc_publ_purge().

The lock is dropped and BH is re-enabled between iterations, so a grace
period can end in the middle of the loop. Can a later iteration then touch
a publication that kfree_rcu() has already freed?

One way to make tipc_node_find() fail while the purge is running:

CPU B: tipc_named_rcv() holds nametbl_lock and has already dequeued a
       WITHDRAWAL from node N (node_lost_contact() only purges what is
       still left in bc_entry.namedq)
CPU A: tipc_node_write_unlock()->tipc_publ_notify() waits on
       nametbl_lock
CPU C: tipc_nl_peer_rm()->tipc_node_delete() removes N from the hash
       once N is SELF_DOWN_PEER_LEAVING or SELF_DOWN_PEER_DOWN
CPU B: tipc_update_nametbl()->tipc_node_unsubscribe() fails the lookup,
       then kfree_rcu(p) runs with p still on N->publ_list
CPU A: list_splice_init() moves p onto purge_list, and a later iteration
       accesses p

A second path does not depend on the purge race. tipc_named_rcv() takes
the node from the header the peer sends, and nothing validates it:

		node = msg_orignode(hdr);

tipc_service_remove_publ() treats node == 0 as a wildcard:

		if (p->key != key || p->sk.ref != sk->ref ||
		    (node && node != p->sk.node))
			continue;

A peer with a link could send a WITHDRAWAL with orignode 0 that matches a
publication from node X. That publication is removed and freed. Because
in_own_node(net, 0) is true, tipc_node_unsubscribe() skips the unlink, and
the freed entry stays on X->publ_list.

>From then on, list_add_tail() in tipc_node_subscribe(), list_del_init()
on neighbouring entries, and the splice and walk here when X goes down
would all operate on freed memory. The spoofing itself predates this
patch, but it produces the same state that this loop assumes cannot
happen.

Could the WITHDRAWAL branch always do list_del_init(&p->binding_node)
under nametbl_lock? Every writer of binding_node for remote publications
already holds that lock.

Separately, could tipc_named_rcv() reject an orignode of 0 or our own
address, or require it to match the owner of the removed publication?
Without that check, a spoofed withdrawal can also free local publications
that are still on the socket and cluster_scope lists.

The kernel-doc for struct publication in net/tipc/name_table.h also still
says:

 * - Remote publications: in node->publ_list;

While a purge is running, binding_node is on the on-stack purge_list.
Should that comment be updated too?

>  
> -	list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
> -		tipc_publ_purge(net, publ, addr);
>  	spin_lock_bh(&tn->nametbl_lock);
>  	if (!(capabilities & TIPC_NAMED_BCAST))
>  		nt->rc_dests--;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927180806.1315902-1-nicoyip.dev%40gmail.com

[PATCH net v2 0/2] tipc: fix publication lifetime races

[Chengfeng Ye]

Two races can leave the publication lists referring to objects with an
invalid lifetime.

First, tipc_node_unsubscribe() looks up the publishing node before
unlinking a publication. If the node has already been removed from the
hash, the lookup fails and the caller frees the publication while its
binding_node remains linked.

Second, tipc_publ_notify() retains the next publication from a failed
node's list across an unlocked interval. A concurrent withdrawal can
unlink and schedule that publication for freeing before the purge
iterator advances to it.

Patch 1 unlinks successfully removed remote publications directly under
nametbl_lock. Patch 2 moves the failed node's publications to a private
list and selects each publication under the same lock, so no publication
pointer is retained across an unlocked interval.

Changes in v2:
- Split the original fix into two patches.
- Add patch 1/2 to address the unlink-before-free issue reported by
  Sashiko and remove the now-unused tipc_node_unsubscribe() helper.
- Add the decoded causal call trace requested by Tung Quang Nguyen.
- Explain the ordering of name-table updates around the node-down
  publication snapshot.

v1:
https://lore.kernel.org/netdev/20260927180806.1315902-1-nicoyip.dev@gmail.com/

Chengfeng Ye (2):
  tipc: unlink publications without a node lookup
  tipc: serialize publication purging with name table updates

 net/tipc/name_distr.c | 36 ++++++++++++++++++++++++------------
 net/tipc/name_distr.h |  2 +-
 net/tipc/node.c       | 20 +-------------------
 net/tipc/node.h       |  1 -
 4 files changed, 26 insertions(+), 33 deletions(-)

-- 
2.43.0

[PATCH net v2 1/2] tipc: unlink publications without a node lookup

[Chengfeng Ye]

The two remote-publication removal paths remove a publication from the
name table and call tipc_node_unsubscribe() before scheduling it for
freeing.

tipc_node_unsubscribe() looks up the publishing node by address and returns
without unlinking when the node has already disappeared from the hash. The
publication is then freed while its binding_node remains linked, so a later
publication-list traversal can access freed memory.

Both paths already run under nametbl_lock. Unlink binding_node directly
under that lock instead of performing another node lookup. For a valid
remote publication, binding_node is either on the node publication list or
is initialized as an empty list when subscription failed. Remove the
now-unused helper and address arguments.

Fixes: a8f48af587b0 ("tipc: remove node subscription infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/tipc/name_distr.c | 11 +++++------
 net/tipc/name_distr.h |  2 +-
 net/tipc/node.c       | 20 +-------------------
 net/tipc/node.h       |  1 -
 4 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index ba4f4906e13b..acf96562608b 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -227,12 +227,11 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
  * tipc_publ_purge - remove publication associated with a failed node
  * @net: the associated network namespace
  * @p: the publication to remove
- * @addr: failed node's address
  *
  * Invoked for each publication issued by a newly failed node.
  * Removes publication structure from name table & deletes it.
  */
-static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
+static void tipc_publ_purge(struct net *net, struct publication *p)
 {
 	struct tipc_net *tn = tipc_net(net);
 	struct publication *_p;
@@ -243,14 +242,14 @@ static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
 	spin_lock_bh(&tn->nametbl_lock);
 	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
 	if (_p)
-		tipc_node_unsubscribe(net, &_p->binding_node, addr);
+		list_del_init(&_p->binding_node);
 	spin_unlock_bh(&tn->nametbl_lock);
 	if (_p)
 		kfree_rcu(_p, rcu);
 }
 
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
-		      u32 addr, u16 capabilities)
+		      u16 capabilities)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
@@ -258,7 +257,7 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
 	struct publication *publ, *tmp;
 
 	list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
-		tipc_publ_purge(net, publ, addr);
+		tipc_publ_purge(net, publ);
 	spin_lock_bh(&tn->nametbl_lock);
 	if (!(capabilities & TIPC_NAMED_BCAST))
 		nt->rc_dests--;
@@ -307,7 +306,7 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 	} else if (dtype == WITHDRAWAL) {
 		p = tipc_nametbl_remove_publ(net, &ua, &sk, key);
 		if (p) {
-			tipc_node_unsubscribe(net, &p->binding_node, node);
+			list_del_init(&p->binding_node);
 			kfree_rcu(p, rcu);
 			return true;
 		}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index c677f6f082df..8debe23469b2 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -74,6 +74,6 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq,
 		    u16 *rcv_nxt, bool *open);
 void tipc_named_reinit(struct net *net);
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
-		      u32 addr, u16 capabilities);
+		      u16 capabilities);
 
 #endif
diff --git a/net/tipc/node.c b/net/tipc/node.c
index bd91378b7540..0e333f952c4f 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -422,7 +422,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
 	write_unlock_bh(&n->lock);
 
 	if (flags & TIPC_NOTIFY_NODE_DOWN)
-		tipc_publ_notify(net, publ_list, node, n->capabilities);
+		tipc_publ_notify(net, publ_list, n->capabilities);
 
 	if (flags & TIPC_NOTIFY_NODE_UP)
 		tipc_named_node_up(net, node, n->capabilities);
@@ -671,24 +671,6 @@ void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr)
 	tipc_node_put(n);
 }
 
-void tipc_node_unsubscribe(struct net *net, struct list_head *subscr, u32 addr)
-{
-	struct tipc_node *n;
-
-	if (in_own_node(net, addr))
-		return;
-
-	n = tipc_node_find(net, addr);
-	if (!n) {
-		pr_warn("Node unsubscribe rejected, unknown node 0x%x\n", addr);
-		return;
-	}
-	tipc_node_write_lock(n);
-	list_del_init(subscr);
-	tipc_node_write_unlock_fast(n);
-	tipc_node_put(n);
-}
-
 int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port)
 {
 	struct tipc_node *node;
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 154a5bbb0d29..a5f060b622a7 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -104,7 +104,6 @@ int tipc_node_distr_xmit(struct net *net, struct sk_buff_head *list);
 int tipc_node_xmit_skb(struct net *net, struct sk_buff *skb, u32 dest,
 		       u32 selector);
 void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr);
-void tipc_node_unsubscribe(struct net *net, struct list_head *subscr, u32 addr);
 void tipc_node_broadcast(struct net *net, struct sk_buff *skb, int rc_dests);
 int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port);
 void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port);
-- 
2.43.0

[PATCH net v2 2/2] tipc: serialize publication purging with name table updates

[Chengfeng Ye]

tipc_publ_notify() walks a failed node publication list after the node lock
has been released. Its safe iterator is not protected by nametbl_lock,
which is acquired only inside tipc_publ_purge().

A concurrent withdrawal can unlink and schedule the saved next publication
for freeing. The purge iterator then advances to that removed publication.
It may access freed memory after the RCU grace period, or repeatedly follow
the self-linked binding_node before then.

The decoded causal stack is:

  tipc_nametbl_remove_publ net/tipc/name_table.c:543
  tipc_publ_purge          net/tipc/name_distr.c:244
  tipc_publ_notify         net/tipc/name_distr.c:261
  tipc_node_write_unlock   net/tipc/node.c:425
  tipc_node_link_down      net/tipc/node.c:1094
  tipc_node_delete_links   net/tipc/node.c:1325
  bearer_disable           net/tipc/bearer.c:414
  __tipc_nl_bearer_disable net/tipc/bearer.c:992
  tipc_nl_bearer_disable   net/tipc/bearer.c:1002

Move the failed node publications to a private list under nametbl_lock.
Select, unlink and purge one publication during each lock acquisition, so
no publication pointer is retained across an unlocked interval. Concurrent
withdrawals can remove entries from the private list under the same lock.

Holding the lock for the whole purge would keep bottom halves disabled
while removing every publication. Releasing it after each entry avoids
an excessive lock hold for nodes with many publications.

node_lost_contact() purges queued name-table updates before scheduling the
node-down notification. An update already dequeued by tipc_named_rcv()
holds nametbl_lock until it updates the publication list, so it completes
before the snapshot and is included. A publication accepted after the
snapshot remains on the live node list for a later contact.

Fixes: 9db9fdd1983e ("tipc: avoid to asynchronously notify subscriptions")
Cc: stable@vger.kernel.org

Link: https://lore.kernel.org/netdev/20260927180806.1315902-1-nicoyip.dev@gmail.com/
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/tipc/name_distr.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index acf96562608b..9a400a1fa4d7 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -230,20 +230,16 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
  *
  * Invoked for each publication issued by a newly failed node.
  * Removes publication structure from name table & deletes it.
+ * The caller must hold nametbl_lock and unlink the node subscription.
  */
 static void tipc_publ_purge(struct net *net, struct publication *p)
 {
-	struct tipc_net *tn = tipc_net(net);
 	struct publication *_p;
 	struct tipc_uaddr ua;
 
 	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type,
 		   p->sr.lower, p->sr.upper);
-	spin_lock_bh(&tn->nametbl_lock);
 	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
-	if (_p)
-		list_del_init(&_p->binding_node);
-	spin_unlock_bh(&tn->nametbl_lock);
 	if (_p)
 		kfree_rcu(_p, rcu);
 }
@@ -254,10 +250,27 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
 
-	struct publication *publ, *tmp;
+	struct publication *publ;
+	LIST_HEAD(purge_list);
 
-	list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
+	spin_lock_bh(&tn->nametbl_lock);
+	/* Preserve publications learned after this node-down snapshot. */
+	list_splice_init(nsub_list, &purge_list);
+	spin_unlock_bh(&tn->nametbl_lock);
+
+	for (;;) {
+		spin_lock_bh(&tn->nametbl_lock);
+		if (list_empty(&purge_list)) {
+			spin_unlock_bh(&tn->nametbl_lock);
+			break;
+		}
+		publ = list_first_entry(&purge_list, struct publication,
+					binding_node);
+		list_del_init(&publ->binding_node);
 		tipc_publ_purge(net, publ);
+		spin_unlock_bh(&tn->nametbl_lock);
+	}
+
 	spin_lock_bh(&tn->nametbl_lock);
 	if (!(capabilities & TIPC_NAMED_BCAST))
 		nt->rc_dests--;
-- 
2.43.0

Re: [PATCH net v2 0/2] tipc: fix publication lifetime races

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] tipc: serialize publication purging with name table updates

[Chengfeng Ye]

On Tue, Sep 29, 2026 at 7:14 PM Tung Quang Nguyen
<tung.quang.nguyen@est.tech> wrote:
>
> >Subject: [PATCH net] tipc: serialize publication purging with name table updates
> >
> >tipc_publ_notify() walks a failed node's publication list after the node lock has
> >been released. Its list iterator is not protected by the name table lock, which is
> >only acquired inside tipc_publ_purge().
> >
> >CPU A can save the next publication before entering tipc_publ_purge().
> >CPU B then takes nametbl_lock in tipc_named_rcv(), processes a
> >WITHDRAWAL for that publication, unlinks it with list_del_init(), and queues it
> >for freeing with kfree_rcu(). CPU A advances to the removed publication and
> >loops on its self-linked binding_node, stalling the CPU.
> >
> >The kernel reported:
> >
> >  rcu: INFO: rcu_sched self-detected stall on CPU
> >  Call Trace:
> >   tipc_publ_notify+0x3b5/0x650
> >   tipc_node_write_unlock+0x49d/0x5d0
> >   tipc_node_link_down+0x15c/0x4a0
> >   tipc_node_delete_links+0xfc/0x190
> >   bearer_disable+0x111/0x270
> >   __tipc_nl_bearer_disable+0x1db/0x2f0
> >   tipc_nl_bearer_disable+0x1c/0x30
> >
>
> Please update your changelog with decoded stack trace.

Updated in v2.

> >Locking the entire traversal would also prevent the race, but would hold
> >nametbl_lock with bottom halves disabled while purging every publication.
> >A node with many publications could therefore cause excessive lock hold times
> >and delay other name-table operations.
> >
> >Move the failed node's publications to a private list under nametbl_lock, then
> >select and unlink its first entry under the same lock before purging it.
> >Concurrent withdrawals can still unlink entries from this private list, while
> >publications arriving after node recovery stay on the node's live list. No
> >publication pointer is carried across an unlocked interval.
> >
> >Unlink before the table lookup to make progress even if the lookup fails, and
> >acquire and release the lock for each publication. Check the private list under
> >the lock on every iteration, since concurrent withdrawals can still remove
> >entries. Keep withdrawal notifications and the final rc_dests update in their
> >existing order.
> >
> >Remove the failed-node address argument from tipc_publ_notify() and its
> >purge helper since unlinking no longer needs a node lookup.
> >
> >Fixes: 9db9fdd1983e ("tipc: avoid to asynchronously notify subscriptions")
> >Cc: stable@vger.kernel.org
> >Assisted-by: GPT-6-Astra
> >Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> >---
> > net/tipc/name_distr.c | 34 +++++++++++++++++++++++-----------
> > net/tipc/name_distr.h |  2 +-
> > net/tipc/node.c       |  2 +-
> > 3 files changed, 25 insertions(+), 13 deletions(-)
> >
> >diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index
> >ba4f4906e13b..34867b69044c 100644
> >--- a/net/tipc/name_distr.c
> >+++ b/net/tipc/name_distr.c
> >@@ -227,38 +227,50 @@ void tipc_named_node_up(struct net *net, u32
> >dnode, u16 capabilities)
> >  * tipc_publ_purge - remove publication associated with a failed node
> >  * @net: the associated network namespace
> >  * @p: the publication to remove
> >- * @addr: failed node's address
> >  *
> >  * Invoked for each publication issued by a newly failed node.
> >  * Removes publication structure from name table & deletes it.
> >+ * The caller must hold nametbl_lock and unlink the node subscription.
> >  */
> >-static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
> >+static void tipc_publ_purge(struct net *net, struct publication *p)
> > {
> >-      struct tipc_net *tn = tipc_net(net);
> >       struct publication *_p;
> >       struct tipc_uaddr ua;
> >
> >       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type,
> >                  p->sr.lower, p->sr.upper);
> >-      spin_lock_bh(&tn->nametbl_lock);
> >       _p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
> >-      if (_p)
> >-              tipc_node_unsubscribe(net, &_p->binding_node, addr);
> >-      spin_unlock_bh(&tn->nametbl_lock);
> >       if (_p)
> >               kfree_rcu(_p, rcu);
> > }
> >
> > void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
> >-                    u32 addr, u16 capabilities)
> >+                    u16 capabilities)
> > {
> >       struct name_table *nt = tipc_name_table(net);
> >       struct tipc_net *tn = tipc_net(net);
> >
> >-      struct publication *publ, *tmp;
> >+      struct publication *publ;
> >+      LIST_HEAD(purge_list);
> >+
> >+      spin_lock_bh(&tn->nametbl_lock);
> >+      /* Leave new publications on the node's list during the purge. */
> >+      list_splice_init(nsub_list, &purge_list);
> >+      spin_unlock_bh(&tn->nametbl_lock);
>
> After this lock is released, new publications can be inserted into node->publ_list. Does this defeat the purpose of current publication release ?

I think probably no. I understand that tipc_publ_notify() as to remove
the publications associated with the contact that has just failed. The
original traversal did not provide deterministic behavior for such
concurrent insertions. Since publications are appended with
list_add_tail(), list_for_each_entry_safe() could either reach a newly
inserted publication or miss it, depending on when the iterator saved
its next entry. The private list makes the node-down cleanup boundary
explicit.

> >+
> >+      for (;;) {
> >+              spin_lock_bh(&tn->nametbl_lock);
> >+              if (list_empty(&purge_list)) {
> >+                      spin_unlock_bh(&tn->nametbl_lock);
> >+                      break;
> >+              }
> >+              publ = list_first_entry(&purge_list, struct publication,
> >+                                      binding_node);
> >+              list_del_init(&publ->binding_node);
> >+              tipc_publ_purge(net, publ);
> >+              spin_unlock_bh(&tn->nametbl_lock);
> >+      }
> >
> >-      list_for_each_entry_safe(publ, tmp, nsub_list, binding_node)
> >-              tipc_publ_purge(net, publ, addr);
> >       spin_lock_bh(&tn->nametbl_lock);
> >       if (!(capabilities & TIPC_NAMED_BCAST))
> >               nt->rc_dests--;
> >diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index
> >c677f6f082df..8debe23469b2 100644
> >--- a/net/tipc/name_distr.h
> >+++ b/net/tipc/name_distr.h
> >@@ -74,6 +74,6 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head
> >*namedq,
> >                   u16 *rcv_nxt, bool *open);
> > void tipc_named_reinit(struct net *net);  void tipc_publ_notify(struct net *net,
> >struct list_head *nsub_list,
> >-                    u32 addr, u16 capabilities);
> >+                    u16 capabilities);
> >
> > #endif
> >diff --git a/net/tipc/node.c b/net/tipc/node.c index
> >bd91378b7540..9a218d137c45 100644
> >--- a/net/tipc/node.c
> >+++ b/net/tipc/node.c
> >@@ -422,7 +422,7 @@ static void tipc_node_write_unlock(struct tipc_node
> >*n)
> >       write_unlock_bh(&n->lock);
> >
> >       if (flags & TIPC_NOTIFY_NODE_DOWN)
> >-              tipc_publ_notify(net, publ_list, node, n->capabilities);
> >+              tipc_publ_notify(net, publ_list, n->capabilities);
> >
> >       if (flags & TIPC_NOTIFY_NODE_UP)
> >               tipc_named_node_up(net, node, n->capabilities);
> >--
> >2.43.0
>

The node-lookup failure reported by sashiko is pre-existing and was
not introduced by the patch, but the patch relies on the affected
list-membership invariant, and the original publication release
implementation is also affected.

Since the race addressed in this patch and reported by sashiko are of
different root cause, I address both cases in v2 with a two-patch
series. Patch 1 replaces tipc_node_unsubscribe() with
list_del_init(&p->binding_node) while nametbl_lock is held and before
kfree_rcu(). Consequently, a failed node lookup can no longer cause a
remote publication to be freed while it remains on either
node->publ_list or the private purge list. Patch 2 fixes the problem
addressed by this v1 patch.

https://lore.kernel.org/netdev/20261001182924.3928331-1-nicoyip.dev@gmail.com/

Best regards,
Chengfeng