Re: can: j1939: unregister_netdevice: waiting for vcan0 to become free.
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: 2026-01-13 03:46:53
Also in:
linux-can
Subsystem:
can network layer, can-j1939 network layer, the rest · Maintainers:
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht, Oleksij Rempel, Linus Torvalds
Currently, the (session->last_cmd != 0) path in j1939_xtp_rx_rts_session_active() is preventing the (session->state == J1939_SESSION_WAITING_ABORT) path in j1939_tp_rxtimer() from being called. This results in two j1939_priv refcounts leak (which in turn results in one net_device refcount leak) due to j1939_session_deactivate_activate_next() being not called. This problem goes away if I do either
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c@@ -1689,16 +1692,18 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session, if (session->last_cmd != 0) { /* we received a second rts on the same connection */ - netdev_alert(priv->ndev, "%s: 0x%p: connection exists (%02x %02x). last cmd: %x\n", + netdev_alert(priv->ndev, "%s (modified): 0x%p: connection exists (%02x %02x). last cmd: %x\n", __func__, session, skcb->addr.sa, skcb->addr.da, session->last_cmd); + /* j1939_session_timers_cancel(session); j1939_session_cancel(session, J1939_XTP_ABORT_BUSY); if (session->transmission) j1939_session_deactivate_activate_next(session); return -EBUSY; + */ } if (session->skcb.addr.sa != skcb->addr.sa ||
or
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c@@ -1697,6 +1700,11 @@ static int j1939_xtp_rx_rts_session_active(struct j1939_session *session, j1939_session_cancel(session, J1939_XTP_ABORT_BUSY); if (session->transmission) j1939_session_deactivate_activate_next(session); + else if (session->state == J1939_SESSION_WAITING_ABORT) { + netdev_alert(priv->ndev, "%s (modified): 0x%p: abort rx timeout. Force session deactivation\n", + __func__, session); + j1939_session_deactivate_activate_next(session); + } return -EBUSY; }
. But what is the correct approach?