Re: can: j1939: unregister_netdevice: waiting for vcan0 to become free.
From: Oleksij Rempel <o.rempel@pengutronix.de>
Date: 2026-01-13 14:32:07
Also in:
linux-can
Hi, On Tue, Jan 13, 2026 at 12:46:30PM +0900, Tetsuo Handa wrote:
quoted hunk ↗ jump to hunk
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 eitherdiff --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 ||ordiff --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) {
This way looks better for me. May be add a comment like this:
/* Force deactivation for the receiver.
* If we rely on the timer starting in j1939_session_cancel,
* a second RTS call here will cancel that timer and fail
* to restart it because the state is already 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?The second one. Thank you for your work. Best Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |