[PATCH net] tipc: fix several race conditions caused by tipc_node_write_unlock()
flat view
COOLING9d
From: Tung Nguyen <tung.quang.nguyen@est.tech>
Date: 2026-10-05 04:11:04
Subsystem:
networking [general], the rest, tipc network layer · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Jon Maloy, Tung Quang Nguyen
tipc_node_write_unlock() releases the node lock after resetting n->action_flags.
This creates a window for several race conditions:
- A race between link-up and link-down events can occur when the
link-down thread is interrupted before removing a publication from
nt->cluster_scope, and the link-up event inserts a publication into
nt->cluster_scope.
- A race can occur between a tipc_rcv() thread that adds or removes
publications to or from node->publ_list and a node-down thread that
removes publications from node->publ_list. The latter thread traverses
node->publ_list without proper lock protection.
These race conditions can result in various use-after-free issues.
Fix these race conditions by:
1. Holding the node lock during link-up/link-down and node-up/node-down
events.
2. Removing the node lookup and node lock from
tipc_node_subscribe() and tipc_node_unsubscribe().
3. Holding the node lock before calling tipc_named_rcv() from
tipc_node_bc_rcv() and tipc_rcv().
4. Moving tipc_node_broadcast() from tipc_nametbl_publish() and
tipc_nametbl_withdraw() to tipc_node_write_unlock().
5. Moving tipc_node_xmit() from tipc_named_node_up() to
tipc_node_write_unlock().
Fixes: 5405ff6e15f4 ("tipc: convert node lock to rwlock")
Reported-by: Chengfeng Ye <redacted>
Closes: https://lore.kernel.org/netdev/20261001182924.3928331-2-nicoyip.dev@gmail.com/ (local)
Closes: https://lore.kernel.org/netdev/20261001182924.3928331-3-nicoyip.dev@gmail.com/ (local)
Signed-off-by: Tung Nguyen <tung.quang.nguyen@est.tech>
---
net/tipc/name_distr.c | 28 ++++++++-------
net/tipc/name_distr.h | 7 ++--
net/tipc/name_table.c | 25 ++++++-------
net/tipc/name_table.h | 6 ++--
net/tipc/net.c | 8 +++--
net/tipc/node.c | 84 +++++++++++++++++++------------------------
net/tipc/node.h | 4 +--
net/tipc/socket.c | 24 +++++++++----
8 files changed, 95 insertions(+), 91 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index ba4f4906e13b..cf5125212521 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c@@ -202,8 +202,10 @@ static void named_distribute(struct net *net, struct sk_buff_head *list, * @net: the associated network namespace * @dnode: destination node * @capabilities: peer node's capabilities + * @xmitq: list of skbs need to be sent */ -void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) +void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities, + struct sk_buff_head *xmitq) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net);
@@ -218,8 +220,7 @@ 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_node_xmit(net, &head, dnode, 0); + named_distribute(net, xmitq, dnode, &nt->cluster_scope, seqno); read_unlock_bh(&nt->cluster_scope_lock); }
@@ -227,12 +228,11 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities) * tipc_publ_purge - remove publication associated with a failed node * @net: the associated network namespace * @p: the publication to remove - * @addr: failed node's address * * Invoked for each publication issued by a newly failed node. * Removes publication structure from name table & deletes it. */ -static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr) +static void tipc_publ_purge(struct net *net, struct publication *p) { struct tipc_net *tn = tipc_net(net); struct publication *_p;
@@ -243,14 +243,14 @@ static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr) spin_lock_bh(&tn->nametbl_lock); _p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key); if (_p) - tipc_node_unsubscribe(net, &_p->binding_node, addr); + tipc_node_unsubscribe(&_p->binding_node); spin_unlock_bh(&tn->nametbl_lock); if (_p) kfree_rcu(_p, rcu); } void tipc_publ_notify(struct net *net, struct list_head *nsub_list, - u32 addr, u16 capabilities) + u16 capabilities) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net);
@@ -258,7 +258,7 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list, struct publication *publ, *tmp; list_for_each_entry_safe(publ, tmp, nsub_list, binding_node) - tipc_publ_purge(net, publ, addr); + tipc_publ_purge(net, publ); spin_lock_bh(&tn->nametbl_lock); if (!(capabilities & TIPC_NAMED_BCAST)) nt->rc_dests--;
@@ -277,7 +277,8 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list, * Return: the publication item if successful, otherwise NULL. */ static bool tipc_update_nametbl(struct net *net, struct distr_item *i, - u32 node, u32 dtype) + u32 node, u32 dtype, + struct list_head *publ_list) { struct publication *p = NULL; u32 lower = ntohl(i->lower);
@@ -301,13 +302,13 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i, if (dtype == PUBLICATION) { p = tipc_nametbl_insert_publ(net, &ua, &sk, key); if (p) { - tipc_node_subscribe(net, &p->binding_node, node); + tipc_node_subscribe(&p->binding_node, publ_list); return true; } } else if (dtype == WITHDRAWAL) { p = tipc_nametbl_remove_publ(net, &ua, &sk, key); if (p) { - tipc_node_unsubscribe(net, &p->binding_node, node); + tipc_node_unsubscribe(&p->binding_node); kfree_rcu(p, rcu); return true; }
@@ -371,7 +372,7 @@ static struct sk_buff *tipc_named_dequeue(struct sk_buff_head *namedq, * @open: last bulk msg was received (FIXME) */ void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, - u16 *rcv_nxt, bool *open) + struct list_head *publ_list, u16 *rcv_nxt, bool *open) { struct tipc_net *tn = tipc_net(net); struct distr_item *item;
@@ -386,7 +387,8 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, item = (struct distr_item *)msg_data(hdr); count = msg_data_sz(hdr) / ITEM_SIZE; while (count--) { - tipc_update_nametbl(net, item, node, msg_type(hdr)); + tipc_update_nametbl(net, item, node, + msg_type(hdr), publ_list); item++; } kfree_skb(skb);
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index c677f6f082df..14c008ab8644 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h@@ -69,11 +69,12 @@ 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); +void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities, + struct sk_buff_head *xmitq); void tipc_named_rcv(struct net *net, struct sk_buff_head *namedq, - u16 *rcv_nxt, bool *open); + struct list_head *publ_list, u16 *rcv_nxt, bool *open); void tipc_named_reinit(struct net *net); void tipc_publ_notify(struct net *net, struct list_head *nsub_list, - u32 addr, u16 capabilities); + u16 capabilities); #endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 6fda36ab1766..da306d178d56 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c@@ -760,15 +760,14 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp, /* tipc_nametbl_publish - add service binding to name table */ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, - struct tipc_socket_addr *sk, u32 key) + struct tipc_socket_addr *sk, u32 key, + struct sk_buff **skb, u32 *rc_dests) { 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)
@@ -797,14 +796,12 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, */ if (!protocol_type) nt->local_publ_count++; - skb = tipc_named_publish(net, p); + *skb = tipc_named_publish(net, p); } - rc_dests = nt->rc_dests; + *rc_dests = nt->rc_dests; exit: spin_unlock_bh(&tn->nametbl_lock); - if (skb) - tipc_node_broadcast(net, skb, rc_dests); return p; }
@@ -815,15 +812,16 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, * @ua: service address/range being unbound * @sk: address of the socket being unbound from * @key: target publication key + * skb: name distribution message needs to be sent + * rc_dests: the number of replicast destinations */ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, - struct tipc_socket_addr *sk, u32 key) + struct tipc_socket_addr *sk, u32 key, + struct sk_buff **skb, u32 *rc_dests) { struct name_table *nt = tipc_name_table(net); struct tipc_net *tn = tipc_net(net); - struct sk_buff *skb = NULL; struct publication *p; - u32 rc_dests; spin_lock_bh(&tn->nametbl_lock);
@@ -833,15 +831,12 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, p->sr.type != TIPC_LINK_STATE && p->sr.type != TIPC_TOP_SRV) nt->local_publ_count--; - skb = tipc_named_withdraw(net, p); + *skb = tipc_named_withdraw(net, p); list_del_init(&p->binding_sock); kfree_rcu(p, rcu); } - rc_dests = nt->rc_dests; + *rc_dests = nt->rc_dests; spin_unlock_bh(&tn->nametbl_lock); - - if (skb) - tipc_node_broadcast(net, skb, rc_dests); } /**
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 7ff6eeebaae6..6cbc9da17464 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h@@ -126,9 +126,11 @@ bool tipc_nametbl_lookup_group(struct net *net, struct tipc_uaddr *ua, void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp, struct tipc_uaddr *ua); struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua, - struct tipc_socket_addr *sk, u32 key); + struct tipc_socket_addr *sk, u32 key, + struct sk_buff **skb, u32 *rc_dests); void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua, - struct tipc_socket_addr *sk, u32 key); + struct tipc_socket_addr *sk, u32 key, + struct sk_buff **skb, u32 *rc_dests); struct publication *tipc_nametbl_insert_publ(struct net *net, struct tipc_uaddr *ua, struct tipc_socket_addr *sk,
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 7e65d0b0c4a8..1e445c5abc0f 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c@@ -125,9 +125,11 @@ int tipc_net_init(struct net *net, u8 *node_id, u32 addr) static void tipc_net_finalize(struct net *net, u32 addr) { - struct tipc_net *tn = tipc_net(net); struct tipc_socket_addr sk = {0, addr}; + struct tipc_net *tn = tipc_net(net); + struct sk_buff *skb = NULL; struct tipc_uaddr ua; + u32 rc_dests; tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE, TIPC_NODE_STATE, addr, addr);
@@ -138,7 +140,9 @@ 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); + tipc_nametbl_publish(net, &ua, &sk, addr, &skb, &rc_dests); + if (skb) + tipc_node_broadcast(net, skb, rc_dests); } void tipc_net_finalize_work(struct work_struct *work)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index d7cbfa786c13..182c6dbfa49d 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c@@ -397,44 +397,50 @@ static void tipc_node_write_unlock(struct tipc_node *n) __releases(n->lock) { struct tipc_socket_addr sk; + struct sk_buff *skb = NULL; + struct sk_buff_head xmitq; struct net *net = n->net; - u32 flags = n->action_flags; - struct list_head *publ_list; struct tipc_uaddr ua; u32 bearer_id, node; + u32 rc_dests; - if (likely(!flags)) { + if (likely(!n->action_flags)) { write_unlock_bh(&n->lock); return; } + __skb_queue_head_init(&xmitq); tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, TIPC_LINK_STATE, n->addr, n->addr); sk.ref = n->link_id; sk.node = tipc_own_addr(net); node = n->addr; bearer_id = n->link_id & 0xffff; - publ_list = &n->publ_list; - - n->action_flags &= ~(TIPC_NOTIFY_NODE_DOWN | TIPC_NOTIFY_NODE_UP | - TIPC_NOTIFY_LINK_DOWN | TIPC_NOTIFY_LINK_UP); - write_unlock_bh(&n->lock); + if (n->action_flags & TIPC_NOTIFY_NODE_DOWN) + tipc_publ_notify(net, &n->publ_list, n->capabilities); - if (flags & TIPC_NOTIFY_NODE_DOWN) - tipc_publ_notify(net, publ_list, node, n->capabilities); + if (n->action_flags & TIPC_NOTIFY_NODE_UP) + tipc_named_node_up(net, node, n->capabilities, &xmitq); - if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, node, n->capabilities); - - if (flags & TIPC_NOTIFY_LINK_UP) { + if (n->action_flags & TIPC_NOTIFY_LINK_UP) { tipc_mon_peer_up(net, node, bearer_id); - tipc_nametbl_publish(net, &ua, &sk, sk.ref); + tipc_nametbl_publish(net, &ua, &sk, sk.ref, &skb, &rc_dests); } - if (flags & TIPC_NOTIFY_LINK_DOWN) { + if (n->action_flags & TIPC_NOTIFY_LINK_DOWN) { tipc_mon_peer_down(net, node, bearer_id); - tipc_nametbl_withdraw(net, &ua, &sk, sk.ref); + tipc_nametbl_withdraw(net, &ua, &sk, sk.ref, &skb, &rc_dests); } + + n->action_flags &= ~(TIPC_NOTIFY_NODE_DOWN | TIPC_NOTIFY_NODE_UP | + TIPC_NOTIFY_LINK_DOWN | TIPC_NOTIFY_LINK_UP); + write_unlock_bh(&n->lock); + + if (!skb_queue_empty(&xmitq)) + tipc_node_xmit(net, &xmitq, node, 0); + + if (skb) + tipc_node_broadcast(net, skb, rc_dests); } static void tipc_node_assign_peer_net(struct tipc_node *n, u32 hash_mixes)
@@ -653,40 +659,14 @@ void tipc_node_stop(struct net *net) spin_unlock_bh(&tn->node_list_lock); } -void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr) +void tipc_node_subscribe(struct list_head *subscr, struct list_head *publ_list) { - struct tipc_node *n; - - if (in_own_node(net, addr)) - return; - - n = tipc_node_find(net, addr); - if (!n) { - pr_warn("Node subscribe rejected, unknown node 0x%x\n", addr); - return; - } - tipc_node_write_lock(n); - list_add_tail(subscr, &n->publ_list); - tipc_node_write_unlock_fast(n); - tipc_node_put(n); + list_add_tail(subscr, publ_list); } -void tipc_node_unsubscribe(struct net *net, struct list_head *subscr, u32 addr) +void tipc_node_unsubscribe(struct list_head *subscr) { - struct tipc_node *n; - - if (in_own_node(net, addr)) - return; - - n = tipc_node_find(net, addr); - if (!n) { - pr_warn("Node unsubscribe rejected, unknown node 0x%x\n", addr); - return; - } - tipc_node_write_lock(n); list_del_init(subscr); - tipc_node_write_unlock_fast(n); - tipc_node_put(n); } int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port)
@@ -1917,10 +1897,14 @@ static void tipc_node_bc_rcv(struct net *net, struct sk_buff *skb, int bearer_id tipc_node_mcast_rcv(n); /* Handle NAME_DISTRIBUTOR messages sent from 1.7 nodes */ - if (!skb_queue_empty(&n->bc_entry.namedq)) + if (!skb_queue_empty(&n->bc_entry.namedq)) { + tipc_node_write_lock(n); tipc_named_rcv(net, &n->bc_entry.namedq, + &n->publ_list, &n->bc_entry.named_rcv_nxt, &n->bc_entry.named_open); + tipc_node_write_unlock_fast(n); + } /* If reassembly or retransmission failure => reset all links to peer */ if (rc & TIPC_LINK_DOWN_EVT)
@@ -2198,10 +2182,14 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b) if (unlikely(rc & TIPC_LINK_DOWN_EVT)) tipc_node_link_down(n, bearer_id, false); - if (unlikely(!skb_queue_empty(&n->bc_entry.namedq))) + if (unlikely(!skb_queue_empty(&n->bc_entry.namedq))) { + tipc_node_write_lock(n); tipc_named_rcv(net, &n->bc_entry.namedq, + &n->publ_list, &n->bc_entry.named_rcv_nxt, &n->bc_entry.named_open); + tipc_node_write_unlock_fast(n); + } if (unlikely(!skb_queue_empty(&n->bc_entry.inputq1))) tipc_node_mcast_rcv(n);
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 154a5bbb0d29..3599e48457bb 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h@@ -103,8 +103,8 @@ int tipc_node_xmit(struct net *net, struct sk_buff_head *list, u32 dnode, int tipc_node_distr_xmit(struct net *net, struct sk_buff_head *list); int tipc_node_xmit_skb(struct net *net, struct sk_buff *skb, u32 dest, u32 selector); -void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr); -void tipc_node_unsubscribe(struct net *net, struct list_head *subscr, u32 addr); +void tipc_node_subscribe(struct list_head *subscr, struct list_head *publ_list); +void tipc_node_unsubscribe(struct list_head *subscr); void tipc_node_broadcast(struct net *net, struct sk_buff *skb, int rc_dests); int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port); void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index d5d70eb230b5..dda247db0e60 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c@@ -2906,23 +2906,27 @@ static void tipc_sk_timeout(struct timer_list *t) static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua) { - struct sock *sk = &tsk->sk; - struct net *net = sock_net(sk); + struct net *net = sock_net(&tsk->sk); struct tipc_socket_addr skaddr; + struct sk_buff *skb = NULL; struct publication *p; + u32 rc_dests; u32 key; - if (tipc_sk_connected(sk)) + if (tipc_sk_connected(&tsk->sk)) return -EINVAL; key = tsk->portid + tsk->pub_count + 1; if (key == tsk->portid) return -EADDRINUSE; skaddr.ref = tsk->portid; skaddr.node = tipc_own_addr(net); - p = tipc_nametbl_publish(net, ua, &skaddr, key); + p = tipc_nametbl_publish(net, ua, &skaddr, key, &skb, &rc_dests); if (unlikely(!p)) return -EINVAL; + if (skb) + tipc_node_broadcast(net, skb, rc_dests); + list_add(&p->binding_sock, &tsk->publications); tsk->pub_count++; tsk->published = true;
@@ -2934,13 +2938,19 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua) struct net *net = sock_net(&tsk->sk); struct publication *safe, *p; struct tipc_uaddr _ua; + struct sk_buff *skb; int rc = -EINVAL; + u32 rc_dests; list_for_each_entry_safe(p, safe, &tsk->publications, binding_sock) { + skb = NULL; if (!ua) { tipc_uaddr(&_ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type, p->sr.lower, p->sr.upper); - tipc_nametbl_withdraw(net, &_ua, &p->sk, p->key); + tipc_nametbl_withdraw(net, &_ua, &p->sk, p->key, + &skb, &rc_dests); + if (skb) + tipc_node_broadcast(net, skb, rc_dests); continue; } /* Unbind specific publication */
@@ -2952,7 +2962,9 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua) continue; if (p->sr.upper != ua->sr.upper) break; - tipc_nametbl_withdraw(net, ua, &p->sk, p->key); + tipc_nametbl_withdraw(net, ua, &p->sk, p->key, &skb, &rc_dests); + if (skb) + tipc_node_broadcast(net, skb, rc_dests); rc = 0; break; }
--
2.43.0