RE: [PATCH net] tipc: prevent node timer rearm after peer removal

[Tung Quang Nguyen]

>Subject: [PATCH net] tipc: prevent node timer rearm after peer removal
>
>TIPC node deletion removes the node from lookup tables, calls
>timer_delete_sync(), and drops the timer reference.  This stops an already
>running timer callback, but it does not prevent another racing path from
>rearming the same timer after deletion has started.
>
>Synthetic UDP discovery can race TIPC_NL_PEER_REMOVE and link property
>updates.  When discovery recreates the first link, tipc_node_check_dest() calls
>mod_timer() and retakes the timer reference.  If that happens after
>tipc_node_delete() has deleted the timer, the orphaned timer can later run
>after network namespace teardown has freed the per-net TIPC state and
>broadcast link.
>
>Use timer_shutdown_sync() when deleting a node.  Shutdown has the same
>synchronization effect as timer_delete_sync(), and also prevents future
>rearming through mod_timer().
>
>Fixes: b34040227be7 ("tipc: add peer removal functionality")
>Signed-off-by: David Lee <david.lee@xxxxxxxxxxxxxxx>
>Assisted-by: Codex:gpt-5.5
>---
>Trail of Bits has a reproducer for this bug demonstrating Kernel Panic which can
>be shared further if needed.
>
> net/tipc/node.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/tipc/node.c b/net/tipc/node.c index
>262b39ecf5f8..5b3ee44347be 100644
>--- a/net/tipc/node.c
>+++ b/net/tipc/node.c
>@@ -638,7 +638,7 @@ static void tipc_node_delete(struct tipc_node *node)
> 	trace_tipc_node_delete(node, true, " ");
> 	tipc_node_delete_from_list(node);
>
>-	timer_delete_sync(&node->timer);
>+	timer_shutdown_sync(&node->timer);
 
sashiko reports:

Could this regression introduce a permanent memory leak of the tipc_node 
structure if it races with tipc_node_check_dest()?
If thread A deletes a node and shuts down the timer, and thread B 
concurrently processes a UDP discovery packet and calls 
tipc_node_check_dest(), thread B might execute this sequence:
    if (!mod_timer(&n->timer, intv))
        tipc_node_get(n);
Since the timer is shut down, mod_timer() silently discards the operation,
does not enqueue the timer, and returns 0. Thread B then calls 
tipc_node_get() expecting the timer callback to eventually execute and drop
the reference.
Because the timer is shut down and will never fire, tipc_node_timeout() 
will never execute to drop this reference, leaking the unlinked node 
permanently.

> 	tipc_node_put(node);
> }
>
>--
>2.43.0
>