rose: use timer_shutdown_sync() for t0timer teardown

include/net/rose.h

@@ -161,17 +161,25 @@ static inline void rose_neigh_put(struct rose_neigh *rose_neigh)
 {
 	if (refcount_dec_and_test(&rose_neigh->use)) {
 		/* We are dropping the last reference, so we are about to free the
-		 * neighbour.  Its timers may still be armed -- t0timer in particular
-		 * re-arms itself in rose_t0timer_expiry().  rose_remove_neigh()
-		 * cancels them before its own put, but callers that drop the final
-		 * reference without first calling rose_remove_neigh() (the socket
-		 * heartbeat reaping path) would otherwise kfree() a neighbour with a
-		 * live timer -> use-after-free.  timer_delete_sync() (not the async
-		 * variant) is required: it waits out a concurrently running handler
-		 * and loops until the self-rearming timer stays stopped.
+		 * neighbour.  t0timer is self-rearming: rose_t0timer_expiry() calls
+		 * rose_start_t0timer() at its own tail, so a plain timer_delete_sync()
+		 * is not enough here.  It only guarantees that the callback is not
+		 * running *at the moment it returns* -- it does nothing to stop the
+		 * very invocation we just waited out from re-arming the timer on its
+		 * way out, which races the kfree() below (syzbot: use-after-free read
+		 * in ax25_find_cb(), reached via rose_t0timer_expiry() ->
+		 * rose_transmit_restart_request() -> rose_send_frame() ->
+		 * ax25_send_frame(), dereferencing the freed neigh->digipeat).
+		 * timer_shutdown_sync() closes that hole: once it returns, any
+		 * further add_timer()/mod_timer() on this timer is silently ignored,
+		 * so a self-rearm racing the free can no longer bring the timer back
+		 * to life on freed memory.  ftimer's handler is a no-op and never
+		 * re-arms, but it is shut down the same way here for consistency --
+		 * this is final teardown, neither timer has any business firing
+		 * again.
 		 */
-		timer_delete_sync(&rose_neigh->ftimer);
-		timer_delete_sync(&rose_neigh->t0timer);
+		timer_shutdown_sync(&rose_neigh->ftimer);
+		timer_shutdown_sync(&rose_neigh->t0timer);
 		if (rose_neigh->ax25)
 			ax25_cb_put(rose_neigh->ax25);
 		kfree(rose_neigh->digipeat);

net/rose/rose_route.c

@@ -229,8 +229,14 @@ static void rose_remove_neigh(struct rose_neigh *rose_neigh)
 {
 	struct rose_neigh *s;
 
-	timer_delete_sync(&rose_neigh->ftimer);
-	timer_delete_sync(&rose_neigh->t0timer);
+	/* Being removed from the list is final teardown for this neighbour:
+	 * shut the timers down rather than just deleting them, so a t0timer
+	 * self-rearm losing the race with this removal can't bring it back to
+	 * life later.  See the comment in rose_neigh_put() (include/net/rose.h)
+	 * for the full story.
+	 */
+	timer_shutdown_sync(&rose_neigh->ftimer);
+	timer_shutdown_sync(&rose_neigh->t0timer);
 
 	skb_queue_purge(&rose_neigh->queue);