netdev - Re: [can/j1939] unregister_netdevice: waiting for vcan0 to become free. Usage count = 2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aSA4JMyFNdliTpli@pengutronix.de>
Date: Fri, 21 Nov 2025 11:00:04 +0100
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: linux-can@...r.kernel.org, Network Development <netdev@...r.kernel.org>,
	Marc Kleine-Budde <mkl@...gutronix.de>
Subject: Re: [can/j1939] unregister_netdevice: waiting for vcan0 to become
 free. Usage count = 2

On Fri, Nov 21, 2025 at 06:33:02PM +0900, Tetsuo Handa wrote:
> On 2025/11/21 18:06, Oleksij Rempel wrote:
> > Hm, looks like we have a race where new session is created in
> > j1939_xtp_rx_rts(), just at the moment where we call
> > j1939_can_rx_unregister().
> > 
> > Haw about following change:
> > 
> > --- a/net/can/j1939/main.c
> > +++ b/net/can/j1939/main.c
> > @@ -214,6 +214,7 @@ static void __j1939_rx_release(struct kref *kref)
> >                                                rx_kref);
> >  
> >         j1939_can_rx_unregister(priv);
> > +       j1939_cancel_active_session(priv, NULL);
> >         j1939_ecu_unmap_all(priv);
> >         j1939_priv_set(priv->ndev, NULL);
> >         mutex_unlock(&j1939_netdev_lock);
> > 
> 
> Well, j1939_cancel_active_session(priv, NULL) is already called from
> j1939_netdev_notify(NETDEV_UNREGISTER). Unless a session is recreated
> after NETDEV_UNREGISTER event was handled, I can't imagine such race.
> 
> We can see that there are three j1939_session_new() calls but only
> two j1939_session_destroy() calls. There might be a refcount leak on
> j1939_session which prevents j1939_priv from dropping final refcount.
> 
>   Call trace for vcan0@...f888031c9c000 +2 at
>        j1939_session_new+0x127/0x450 net/can/j1939/transport.c:1503
>        j1939_tp_send+0x338/0x8c0 net/can/j1939/transport.c:2018
> 
>   Call trace for vcan0@...f888031c9c000 +1 at
>        j1939_session_new+0x127/0x450 net/can/j1939/transport.c:1503
>        j1939_session_fresh_new net/can/j1939/transport.c:1543 [inline]
>        j1939_xtp_rx_rts_session_new net/can/j1939/transport.c:1628 [inline]
>        j1939_xtp_rx_rts+0xd16/0x18b0 net/can/j1939/transport.c:1749
> 
>   Call trace for vcan0@...f888031c9c000 -2 at
>        j1939_priv_put+0x23/0x370 net/can/j1939/main.c:184
>        j1939_session_destroy net/can/j1939/transport.c:285 [inline]
>        __j1939_session_release net/can/j1939/transport.c:294 [inline]
>        kref_put include/linux/kref.h:65 [inline]
> 
> Do we want to update
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/net/can/j1939?id=5ac798f79b48065b0284216c7a0057271185a882
> in order to also try tracing refcount for j1939_session ?

Ack.

-- 
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 |