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