RE: [PATCH net v2] tipc: fix use-after-free of broadcast reception link at node release

[Tung Quang Nguyen]

>Subject: [PATCH net v2] tipc: fix use-after-free of broadcast reception link at
>node release
>
>When the last reference to a tipc_node is dropped, the node itself is deferred
>to the RCU callback tipc_node_free(), but the nested broadcast reception link,
>n->bc_entry.link, is kfree()d synchronously in tipc_node_kref_release(), before
>call_rcu(), and the pointer is never cleared.  Readers that obtained the node
>from node_list and then dereference bc_entry.link deep into the link
>(tipc_link_name() in tipc_nl_node_reset_link_stats(), bcl->name/stats in
>tipc_nl_add_bc_link() on the link dump path) therefore touch freed memory
>although the enclosing node stays alive for a full grace period.
>The sibling per-bearer links get "kfree + NULL" under the node write lock in
>tipc_node_clear_links(); the broadcast entry was missed.
>
>The race is reachable through the node cleanup timer: once a peer has been
>unreachable for NODE_CLEANUP_AFTER (300 s), tipc_node_timeout() ->
>tipc_node_cleanup() unlinks the node under the write lock, and the final
>tipc_node_put() at the caller runs outside node_list_lock, so the kfree executes
>in timer softirq context, concurrently with genl dump/ reset walkers that hold
>only rcu_read_lock().  (A netlink "peer remove"
>cannot race those walkers itself: tipc_genl_family has no parallel_ops, so all
>doit/dumpit calls serialize on the global genl lock.)
>
>Reproduced in a QEMU/KVM guest running v7.3.0 with KASAN and
>PREEMPT(full): 10 TIPC peer namespaces on veth bearers, bearer removal
>staggered so the 300 s node cleanup timers expire while other tasks loop link
>dumps with the broadcast filter and reset-link-stats lookups.
>KASAN hit within two cleanup cycles:
>
>  BUG: KASAN: slab-use-after-free in strlen+0x85/0xa0
>  Read of size 1 at addr ffff88800b54a004 by task poc_eth/371
>   strlen+0x85/0xa0
>   tipc_nl_add_bc_link+0x2f9/0xd40
>   __tipc_nl_add_node_links+0x10c/0x140
>   tipc_nl_node_dump_link+0x8d5/0xd50
>  Freed by task 0:
>   kfree+0x121/0x380
>   tipc_node_timeout+0xa33/0xcf0
>   run_timer_softirq+0x144/0x250
>   sysvec_apic_timer_interrupt
>
>with the freed object being the kmalloc-1k tipc_link allocated by
>tipc_link_bc_create() during peer discovery.  The same defect fires in
>strcmp() called from tipc_nl_node_reset_link_stats().
>
>Move the kfree of n->bc_entry.link into the tipc_node_free() RCU callback so
>the broadcast link lives exactly as long as the enclosing node.  All readers are
>covered: the netlink walkers are inside rcu_read_lock(), and the packet
>reception path reaches the node via tipc_node_find(), whose kref defers
>kref_release itself.
>
>Found during a static-analysis-assisted audit: candidate sites were surfaced by
>a Coccinelle rule, and the reader/writer paths were confirmed by LLM-assisted
>(GLM-5.3) code inspection.
>
>Fixes: d25a01257e42 ("tipc: fix crash during node removal")
>Assisted-by: GLM-5.3 coccinelle
>Signed-off-by: Binbin Deng <18983559317@xxxxxxx>

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