[PATCH net] can: j1939: avoid address-claim timer deadlock

[PATCH net] can: j1939: avoid address-claim timer deadlock

[Felix Hoffmann]

j1939_ac_process() holds priv->lock while synchronously canceling an
ECU's address-claim hrtimer. The timer callback takes the same lock. If the
callback starts on another CPU after the receive path takes the lock, the
callback waits for priv->lock while hrtimer_cancel() waits for the callback
to finish. This deadlocks both CPUs and makes the system unresponsive.

Do not wait for priv->lock from the soft hrtimer callback. If
address-claim processing currently owns it, move the expiry forward by 1 ms
and restart the timer. This lets a concurrent hrtimer_cancel() finish and
remove the requeued timer. Without a cancellation, mapping is retried
shortly.

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT5.6-Sol
Signed-off-by: Felix Hoffmann <f3lix.dev@gmx.de>
---

The deadlock was reproduced three times on a 2-vCPU kernel with KASAN
and lockdep. Cross-CPU GDB stacks showed hrtimer_cancel() and the timer
callback waiting on the same j1939_priv lock and hrtimer.

With this change, the reproducer completed 420 stress rounds, and the
patched CAN J1939 syzkaller campaign remained operational. The trigger
drops to UID and GID 65534 before opening its CAN sockets. A minimal
reproducer and the complete stack capture are available privately on
request.

 net/can/j1939/bus.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/can/j1939/bus.c b/net/can/j1939/bus.c
index cdc3c0a71937..ac654dc8872e 100644
--- a/net/can/j1939/bus.c
+++ b/net/can/j1939/bus.c
@@ -131,7 +131,17 @@ static enum hrtimer_restart j1939_ecu_timer_handler(struct hrtimer *hrtimer)
 		container_of(hrtimer, struct j1939_ecu, ac_timer);
 	struct j1939_priv *priv = ecu->priv;
 
-	write_lock_bh(&priv->lock);
+	/*
+	 * j1939_ac_process() cancels this timer while holding priv->lock.
+	 * Don't block here, otherwise the timer and receive paths can deadlock
+	 * waiting for each other on different CPUs. Retry shortly if address
+	 * claim processing currently owns the lock.
+	 */
+	if (!write_trylock(&priv->lock)) {
+		hrtimer_forward_now(hrtimer, ms_to_ktime(1));
+		return HRTIMER_RESTART;
+	}
+
 	/* TODO: can we test if ecu->addr is unicast before starting
 	 * the timer?
 	 */
@@ -141,7 +151,7 @@ static enum hrtimer_restart j1939_ecu_timer_handler(struct hrtimer *hrtimer)
 	 * j1939_ecu_timer_start().
 	 */
 	j1939_ecu_put(ecu);
-	write_unlock_bh(&priv->lock);
+	write_unlock(&priv->lock);
 
 	return HRTIMER_NORESTART;
 }
-- 
2.43.0