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