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

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

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

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.

The following trace was captured on 7.2.0-rc5-00284-gaf39eb111ce6:

  BUG: KASAN: slab-use-after-free in tipc_link_is_establishing (net/tipc/link.c:285)
  Read of size 4 at addr ffff88802e2aa068 by task swapper/2/0
   tipc_link_is_establishing (net/tipc/link.c:285)
   tipc_node_link_down (net/tipc/node.c:1076)
   tipc_node_timeout (net/tipc/node.c:843)
  Allocated by task 9549:
   tipc_link_create (net/tipc/link.c:490)
   tipc_node_check_dest (net/tipc/node.c:1279)
   tipc_disc_rcv (net/tipc/discover.c:252)
   tipc_udp_recv (net/tipc/udp_media.c:389)
  Freed by task 9549:
   tipc_node_link_down (net/tipc/node.c:1084)
   tipc_node_delete_links (net/tipc/node.c:1320)
   bearer_disable (net/tipc/bearer.c:414)
   __tipc_nl_bearer_disable (net/tipc/bearer.c:992)

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 <corvus@tencent.com>
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
A KASAN reproducer for this issue is available if requested.

v2:
 - Take &n->links[bearer_id] under tipc_node_write_lock() as well.
 - Use tipc_node_write_unlock_fast() on the NULL link path.
 - Decode the KASAN report with scripts/decode_stacktrace.sh, re-taken on
   7.2.0-rc5-00284-gaf39eb111ce6.

v1: https://lore.kernel.org/netdev/20260731102145.33622-1-juny24602@gmail.com/

 net/tipc/node.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 8e4ef2630ae4..683a136e53ef 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1061,18 +1061,23 @@ static void __tipc_node_link_down(struct tipc_node *n, int *bearer_id,
 
 static void tipc_node_link_down(struct tipc_node *n, int bearer_id, bool delete)
 {
-	struct tipc_link_entry *le = &n->links[bearer_id];
 	struct tipc_media_addr *maddr = NULL;
-	struct tipc_link *l = le->link;
 	int old_bearer_id = bearer_id;
+	struct tipc_link_entry *le;
 	struct sk_buff_head xmitq;
-
-	if (!l)
-		return;
+	struct tipc_link *l;
 
 	__skb_queue_head_init(&xmitq);
 
+	/* Synchronize the link lookup with bearer teardown. */
 	tipc_node_write_lock(n);
+	le = &n->links[bearer_id];
+	l = le->link;
+	if (!l) {
+		tipc_node_write_unlock_fast(n);
+		return;
+	}
+
 	if (!tipc_link_is_establishing(l)) {
 		__tipc_node_link_down(n, &bearer_id, &xmitq, &maddr);
 	} else {
-- 
2.55.0

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

[Tung Quang Nguyen]

>A KASAN reproducer for this issue is available if requested.

Yes, please send me the reproducer.
It would be good if I can verify the fix at my side.

>
>v2:
> - Take &n->links[bearer_id] under tipc_node_write_lock() as well.
> - Use tipc_node_write_unlock_fast() on the NULL link path.
> - Decode the KASAN report with scripts/decode_stacktrace.sh, re-taken on
>   7.2.0-rc5-00284-gaf39eb111ce6.
>
>v1: https://lore.kernel.org/netdev/20260731102145.33622-1-
>juny24602@gmail.com/
>
> net/tipc/node.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>diff --git a/net/tipc/node.c b/net/tipc/node.c index
>8e4ef2630ae4..683a136e53ef 100644
>--- a/net/tipc/node.c
>+++ b/net/tipc/node.c
>@@ -1061,18 +1061,23 @@ static void __tipc_node_link_down(struct
>tipc_node *n, int *bearer_id,
>
> static void tipc_node_link_down(struct tipc_node *n, int bearer_id, bool
>delete)  {
>-	struct tipc_link_entry *le = &n->links[bearer_id];
> 	struct tipc_media_addr *maddr = NULL;
>-	struct tipc_link *l = le->link;
> 	int old_bearer_id = bearer_id;
>+	struct tipc_link_entry *le;
> 	struct sk_buff_head xmitq;
>-
>-	if (!l)
>-		return;
>+	struct tipc_link *l;
>
> 	__skb_queue_head_init(&xmitq);
>
>+	/* Synchronize the link lookup with bearer teardown. */
> 	tipc_node_write_lock(n);
>+	le = &n->links[bearer_id];
>+	l = le->link;
>+	if (!l) {
>+		tipc_node_write_unlock_fast(n);
>+		return;
>+	}
>+
> 	if (!tipc_link_is_establishing(l)) {
> 		__tipc_node_link_down(n, &bearer_id, &xmitq, &maddr);
> 	} else {
>--
>2.55.0
>

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

[Tung Quang Nguyen]

>Subject: [PATCH net v2] tipc: read le->link under the node lock in
>tipc_node_link_down()
>
>From: Jun Yang <junvyyang@tencent.com>
>
>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.
>
>The following trace was captured on 7.2.0-rc5-00284-gaf39eb111ce6:
>
>  BUG: KASAN: slab-use-after-free in tipc_link_is_establishing
>(net/tipc/link.c:285)
>  Read of size 4 at addr ffff88802e2aa068 by task swapper/2/0
>   tipc_link_is_establishing (net/tipc/link.c:285)
>   tipc_node_link_down (net/tipc/node.c:1076)
>   tipc_node_timeout (net/tipc/node.c:843)
>  Allocated by task 9549:
>   tipc_link_create (net/tipc/link.c:490)
>   tipc_node_check_dest (net/tipc/node.c:1279)
>   tipc_disc_rcv (net/tipc/discover.c:252)
>   tipc_udp_recv (net/tipc/udp_media.c:389)
>  Freed by task 9549:
>   tipc_node_link_down (net/tipc/node.c:1084)
>   tipc_node_delete_links (net/tipc/node.c:1320)
>   bearer_disable (net/tipc/bearer.c:414)
>   __tipc_nl_bearer_disable (net/tipc/bearer.c:992)
>
>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.
>

Reviewed-by: Tung Nguyen <tung.quang.nguyen@est.tech>

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 10 Aug 2026 18:21:38 +0800 you wrote:
> From: Jun Yang <junvyyang@tencent.com>
> 
> 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;
> 
> [...]

Here is the summary with links:
  - [net,v2] tipc: read le->link under the node lock in tipc_node_link_down()
    https://git.kernel.org/netdev/net/c/cba9ccb47e9f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html