[PATCH v2] can: j1939: cancel all pending ECUs on device stop
From: Dmitry Antipov <hidden>
Date: 2026-08-27 11:21:52
Subsystem:
can network layer, can-j1939 network layer, the rest · Maintainers:
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht, Oleksij Rempel, Linus Torvalds
Cancel all pending ECUs in j1939_netdev_stop(). This is needed to prevent the case when stopped device no longer processes ECUs and, since ECU holds the reference to 'struct j1939_priv', the latter (and ECU itself) is never freed. Reported-by: syzbot+489e907b2a026a6f5fa0@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=489e907b2a026a6f5fa0 Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Dmitry Antipov <redacted> --- v2: add extra precaution to avoid ABBA deadlock between j1939_ecu_cancel_all() and timer callbacks (Sashiko) --- net/can/j1939/bus.c | 39 ++++++++++++++++++++++++++------------ net/can/j1939/j1939-priv.h | 6 ++++++ net/can/j1939/main.c | 2 ++ 3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/net/can/j1939/bus.c b/net/can/j1939/bus.c
index cdc3c0a71937..e02454a4c64c 100644
--- a/net/can/j1939/bus.c
+++ b/net/can/j1939/bus.c@@ -96,6 +96,16 @@ void j1939_ecu_unmap(struct j1939_ecu *ecu) write_unlock_bh(&ecu->priv->lock); } +void j1939_ecu_cancel_all(struct j1939_priv *priv) +{ + struct j1939_ecu *ecu, *tmp; + + write_lock_bh(&priv->lock); + list_for_each_entry_safe(ecu, tmp, &priv->ecus, list) + j1939_ecu_timer_cancel(ecu); + write_unlock_bh(&priv->lock); +} + void j1939_ecu_unmap_all(struct j1939_priv *priv) { int i;
@@ -131,18 +141,23 @@ 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); - /* TODO: can we test if ecu->addr is unicast before starting - * the timer? - */ - j1939_ecu_map_locked(ecu); - - /* The corresponding j1939_ecu_get() is in - * j1939_ecu_timer_start(). - */ - j1939_ecu_put(ecu); - write_unlock_bh(&priv->lock); - + /* We're asked to stop from j1939_netdev_stop(). */ + if (unlikely(atomic_read(&priv->stop))) { + lockdep_assert_held(&priv->lock); + j1939_ecu_put(ecu); + } else { + write_lock_bh(&priv->lock); + /* TODO: can we test if ecu->addr is unicast before starting + * the timer? + */ + j1939_ecu_map_locked(ecu); + + /* The corresponding j1939_ecu_get() is in + * j1939_ecu_timer_start(). + */ + j1939_ecu_put(ecu); + write_unlock_bh(&priv->lock); + } return HRTIMER_NORESTART; }
diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
index cf26352d1d8c..1e8f018b7c8b 100644
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h@@ -60,6 +60,11 @@ struct j1939_priv { /* segments need a lock to protect the above list */ rwlock_t lock; + /* Used to avoid deadlock between j1939_ecu_cancel_all() + * and timer callbacks. + */ + atomic_t stop; + struct net_device *ndev; netdevice_tracker dev_tracker;
@@ -204,6 +209,7 @@ struct j1939_ecu *j1939_ecu_create_locked(struct j1939_priv *priv, name_t name); void j1939_ecu_timer_start(struct j1939_ecu *ecu); void j1939_ecu_timer_cancel(struct j1939_ecu *ecu); +void j1939_ecu_cancel_all(struct j1939_priv *priv); void j1939_ecu_unmap_all(struct j1939_priv *priv); struct j1939_priv *j1939_netdev_start(struct net_device *ndev);
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index 5e5e6c228f22..22ea2b000185 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c@@ -306,6 +306,8 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev) void j1939_netdev_stop(struct j1939_priv *priv) { + atomic_set(&priv->stop, 1); + j1939_ecu_cancel_all(priv); kref_put_mutex(&priv->rx_kref, __j1939_rx_release, &j1939_netdev_lock); j1939_priv_put(priv); }
--
2.55.0