[PATCH net] tipc: protect node reset trace dump with node lock

[PATCH net] tipc: protect node reset trace dump with node lock

[Chengfeng Ye]

The tipc_node_reset_links trace event asks tipc_node_dump() to walk the
node's link entries. Unlike the other node events that request link data,
this event runs without the node lock.

This permits bearer teardown to free a link while the trace callback is
dumping it:

  CPU 0                                CPU 1
  trace_tipc_node_reset_links()
    tipc_node_dump()
      l = n->links[0].link
                                       tipc_node_write_lock()
                                       kfree(l)
                                       n->links[0].link = NULL
                                       tipc_node_write_unlock()
      tipc_link_dump(l)

tipc_link_dump() then dereferences the stale pointer. KASAN reported:

  BUG: KASAN: slab-use-after-free in tipc_link_dump+0x10cb/0x16b0
  Read of size 4 by task ksoftirqd/0/14
  Call Trace:
   tipc_link_dump+0x10cb/0x16b0
   tipc_node_dump+0x4bb/0x740
   trace_event_raw_event_tipc_node_class+0x258/0x360
   tipc_node_reset_links+0x14d/0x1a0
   tipc_rcv+0x13f5/0x3030
   tipc_udp_recv+0x4e3/0x670
  Allocated by task 0:
   tipc_link_create+0x1e1/0x1020
   tipc_node_check_dest+0x7d2/0x11a0
   tipc_disc_rcv+0xdbf/0x1430
  Freed by task 89:
   kfree+0x131/0x3c0
   tipc_node_link_down+0x267/0x4b0
   tipc_node_delete_links+0xec/0x160
   bearer_disable+0x107/0x260

Take the node read lock around the trace event. This keeps link pointer
loads and all dump dereferences serialized against link deletion while
preserving the trace contents and reset flow.

Fixes: eb18a510b5cd ("tipc: add trace_events for tipc node")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/tipc/node.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 683a136e53ef..127848e8a644 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1333,7 +1333,9 @@ static void tipc_node_reset_links(struct tipc_node *n)
 
 	pr_warn("Resetting all links to %x\n", n->addr);
 
+	tipc_node_read_lock(n);
 	trace_tipc_node_reset_links(n, true, " ");
+	tipc_node_read_unlock(n);
 	for (i = 0; i < MAX_BEARERS; i++) {
 		tipc_node_link_down(n, i, false);
 	}
-- 
2.43.0

Re: [PATCH net] tipc: protect node reset trace dump with node lock

[Tung Quang Nguyen]

> Subject: [PATCH net] tipc: protect node reset trace dump with node lock

  >BUG: KASAN: slab-use-after-free in tipc_link_dump+0x10cb/0x16b0
  >Read of size 4 by task ksoftirqd/0/14
  >Call Trace:
  > tipc_link_dump+0x10cb/0x16b0
  > tipc_node_dump+0x4bb/0x740
  > trace_event_raw_event_tipc_node_class+0x258/0x360
  > tipc_node_reset_links+0x14d/0x1a0
  > tipc_rcv+0x13f5/0x3030
  > tipc_udp_recv+0x4e3/0x670
  > Allocated by task 0:
  > tipc_link_create+0x1e1/0x1020
  > tipc_node_check_dest+0x7d2/0x11a0
  > tipc_disc_rcv+0xdbf/0x1430
  > Freed by task 89:
  >  kfree+0x131/0x3c0
  >  tipc_node_link_down+0x267/0x4b0
  >  tipc_node_delete_links+0xec/0x160
  > bearer_disable+0x107/0x260

Please decode above stack trace (using linux/scripts/decode_stacktrace.sh).

> Take the node read lock around the trace event. This keeps link pointer
> loads and all dump dereferences serialized against link deletion while
> preserving the trace contents and reset flow.

> Fixes: eb18a510b5cd ("tipc: add trace_events for tipc node")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
> net/tipc/node.c | 2 ++
> 1 file changed, 2 insertions(+)

> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 683a136e53ef..127848e8a644 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -1333,7 +1333,9 @@ static void tipc_node_reset_links(struct tipc_node *n)

>       pr_warn("Resetting all links to %x\n", n->addr);

> +       tipc_node_read_lock(n);
>        trace_tipc_node_reset_links(n, true, " ");
> +       tipc_node_read_unlock(n);

It is not correct using read lock because trace_tipc_node_reset_links() accesses link's queues that tipc_rcv() might access concurrently.
Please test this:

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 683a136e53ef..bd91378b7540 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1333,7 +1333,9 @@ static void tipc_node_reset_links(struct tipc_node *n)

        pr_warn("Resetting all links to %x\n", n->addr);

+       tipc_node_write_lock(n);
        trace_tipc_node_reset_links(n, true, " ");
+       tipc_node_write_unlock_fast(n);
        for (i = 0; i < MAX_BEARERS; i++) {
                tipc_node_link_down(n, i, false);
        }

>        for (i = 0; i < MAX_BEARERS; i++) {
>               tipc_node_link_down(n, i, false);
>        }

Re: [PATCH net] tipc: protect node reset trace dump with node lock

[Chengfeng Ye]

On Mon, Aug 24, 2026 at 4:36 PM Tung Quang Nguyen
<tung.quang.nguyen@est.tech> wrote:
>
> Please decode above stack trace (using linux/scripts/decode_stacktrace.sh).
>

Decoded with linux/scripts/decode_stacktrace.sh against
(7.2.0-05126-ga4ff2be345d0-dirty #26).

BUG: KASAN: slab-use-after-free in tipc_link_dump (net/tipc/link.c:2910)
Read of size 4 at addr ffff88811c935800 by task poc/115
Call Trace:
 tipc_link_dump (net/tipc/link.c:2910)
 tipc_node_dump (net/tipc/node.c:3136)
 trace_event_raw_event_tipc_node_class (net/tipc/trace.h:327)
 tipc_node_reset_links (net/tipc/trace.h:360 / net/tipc/node.c:1337)
 tipc_rcv (net/tipc/node.c:1851 / net/tipc/node.c:2159)
 tipc_udp_recv (net/tipc/udp_media.c:389)
Allocated by task 0:
 tipc_link_create (net/tipc/link.c:490)
 tipc_node_check_dest (net/tipc/node.c:1285)
 tipc_disc_rcv (net/tipc/discover.c:252)
Freed by task 117:
 kfree (mm/slub.c:6692)
 tipc_node_link_down (net/tipc/node.c:1090)
 tipc_node_delete_links (net/tipc/node.c:1326)
 bearer_disable (net/tipc/bearer.c:414)

The KASAN is obtained by using the following kernel-side delay()
instrumentation to make the reproduction deterministic.

```
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 683a136e53ef..0e7cdbee9459 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -34,6 +34,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */

+#include <linux/delay.h>
 #include "core.h"
 #include "link.h"
 #include "node.h"
@@ -3129,7 +3130,11 @@ int tipc_node_dump(struct tipc_node *n, bool
more, char *buf)
        i += scnprintf(buf + i, sz - i, " media: ");
        i += tipc_media_addr_printf(buf + i, sz - i, &n->links[0].maddr);
        i += scnprintf(buf + i, sz - i, "\n");
-       i += tipc_link_dump(n->links[0].link, TIPC_DUMP_NONE, buf + i);
+       {
+               struct tipc_link *l0 = n->links[0].link;
+               mdelay(50);
+               i += tipc_link_dump(l0, TIPC_DUMP_NONE, buf + i);
+       }
        i += scnprintf(buf + i, sz - i, " inputq: ");
        i += tipc_list_dump(&n->links[0].inputq, false, buf + i);

@@ -3138,7 +3143,11 @@ int tipc_node_dump(struct tipc_node *n, bool
more, char *buf)
        i += scnprintf(buf + i, sz - i, " media: ");
        i += tipc_media_addr_printf(buf + i, sz - i, &n->links[1].maddr);
        i += scnprintf(buf + i, sz - i, "\n");
-       i += tipc_link_dump(n->links[1].link, TIPC_DUMP_NONE, buf + i);
+       {
+               struct tipc_link *l1 = n->links[1].link;
+               mdelay(50);
+               i += tipc_link_dump(l1, TIPC_DUMP_NONE, buf + i);
+       }
        i += scnprintf(buf + i, sz - i, " inputq: ");
        i += tipc_list_dump(&n->links[1].inputq, false, buf + i);
```

>
> It is not correct using read lock because trace_tipc_node_reset_links() accesses link's queues that tipc_rcv() might access concurrently.
> Please test this:
>
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 683a136e53ef..bd91378b7540 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -1333,7 +1333,9 @@ static void tipc_node_reset_links(struct tipc_node *n)
>
>         pr_warn("Resetting all links to %x\n", n->addr);
>
> +       tipc_node_write_lock(n);
>         trace_tipc_node_reset_links(n, true, " ");
> +       tipc_node_write_unlock_fast(n);
>         for (i = 0; i < MAX_BEARERS; i++) {
>                 tipc_node_link_down(n, i, false);
>         }
>
> >        for (i = 0; i < MAX_BEARERS; i++) {
> >               tipc_node_link_down(n, i, false);
> >        }
>
>

No problem, I will send a v2 to correct the fix.

Best regards,
Chengfeng