Thread (2 messages) flat view 2 messages, 2 authors, 4d ago

RE: [PATCH net] tipc: read le->link under the node lock in tipc_node_link_down()

From: Tung Quang Nguyen <hidden>
Date: 2026-08-03 02:03:33
Also in: lkml

Subject: [PATCH net] tipc: read le->link under the node lock in
tipc_node_link_down()

From: Jun Yang <redacted>

tipc_node_link_down() caches the link pointer before taking n->lock:

struct tipc_link *l = le->link;		/* unlocked */

if (!l)
	return;
tipc_node_write_lock(n);
if (!tipc_link_is_establishing(l)) {	/* deref l */
...
	tipc_link_reset(l);		/* write into l */
if (delete) {
	kfree(l);
	le->link = NULL;

The delete=true caller frees that very object under n->lock, so the lock does
not protect the cached pointer against it:

- CPU A, delete=false: tipc_rcv() on TIPC_LINK_DOWN_EVT, or the link
  supervision timer via tipc_node_timeout(), reads l unlocked and then
  dereferences it under n->lock;
- CPU B, delete=true: netlink TIPC_NL_BEARER_DISABLE -> bearer_disable()
  -> tipc_node_delete_links() -> tipc_node_link_down(n, bearer_id, true)
  -> kfree(l).

The link is freed with plain kfree(), not kfree_rcu(), and for UDP bearers
disable_media() only schedules the asynchronous cleanup_bearer() work, so
its synchronize_net() runs after the links are already gone.  An in-flight CPU A
that has read l therefore dereferences freed memory once B frees it:
a use-after-free read in tipc_link_is_establishing(), and a use-after-free write
via tipc_link_reset() on the establishing branch.

 BUG: KASAN: slab-use-after-free in tipc_link_is_establishing+0x3f/0x50
 Read of size 4 at addr ffff888111f90868 by task swapper/3/0
  tipc_link_is_establishing+0x3f/0x50
  tipc_node_link_down+0x1a0/0x450
  tipc_node_timeout+0x63d/0xca0
 Allocated by task 7903:
  tipc_link_create+0x17e/0xf20
  tipc_node_check_dest+0x98b/0x1150
  tipc_disc_rcv+0x9f3/0x10c0
  tipc_udp_recv+0x42a/0x7e0
 Freed by task 7903:
  tipc_node_link_down+0x301/0x450
  tipc_node_delete_links+0xf5/0x240
  bearer_disable+0xc0/0x240
  __tipc_nl_bearer_disable+0x21f/0x380
Please update the commit message with decoded stack trace (using linux/scripts/decode_stacktrace.sh).
quoted hunk ↗ jump to hunk
Move the le->link read inside tipc_node_write_lock(), so it is serialised against
the kfree() in the delete path.  A racing teardown now either has not run yet,
and we see a valid link, or has already run, and we see NULL.

Fixes: 73f646cec354 ("tipc: delay ESTABLISH state event when link is
established")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <redacted>
Signed-off-by: Jun Yang <redacted>
---
net/tipc/node.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/tipc/node.c b/net/tipc/node.c index
8e4ef2630ae4..918e59362d07 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1063,16 +1063,23 @@ static void tipc_node_link_down(struct tipc_node
*n, int bearer_id, bool delete)  {
	struct tipc_link_entry *le = &n->links[bearer_id];
Above node access also needs to be moved to below ' tipc_node_write_lock()'.
	struct tipc_media_addr *maddr = NULL;
-	struct tipc_link *l = le->link;
	int old_bearer_id = bearer_id;
	struct sk_buff_head xmitq;
-
-	if (!l)
-		return;
+	struct tipc_link *l;

	__skb_queue_head_init(&xmitq);

+	/* Read le->link under n->lock: the delete path below kfree()s it while
+	 * holding the same lock, so an unlocked read here could be raced by a
+	 * concurrent bearer teardown and leave us with a freed pointer.
+	 */
	tipc_node_write_lock(n);
+	l = le->link;
+	if (!l) {
+		tipc_node_write_unlock(n);
Please use 'tipc_node_write_unlock_fast()' instead.
+		return;
+	}
+
	if (!tipc_link_is_establishing(l)) {
		__tipc_node_link_down(n, &bearer_id, &xmitq, &maddr);
	} else {
--
2.55.0
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help