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: <85b701a9-511d-4cf2-8c9c-5fade945f187@I-love.SAKURA.ne.jp>
Date: Sat, 22 Nov 2025 16:00:36 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Oleksij Rempel <o.rempel@...gutronix.de>
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 2025/11/21 19:31, Oleksij Rempel wrote:
>> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
>> index fbf5c8001c9d..b22568fecba5 100644
>> --- a/net/can/j1939/transport.c
>> +++ b/net/can/j1939/transport.c
>> @@ -1492,6 +1492,8 @@ static struct j1939_session *j1939_session_new(struct j1939_priv *priv,
>>  	struct j1939_session *session;
>>  	struct j1939_sk_buff_cb *skcb;
>>  
>> +	if (priv->ndev->reg_state != NETREG_REGISTERED)
>> +		return NULL;
>>  	session = kzalloc(sizeof(*session), gfp_any());
>>  	if (!session)
>>  		return NULL;
>>
> 
> Yes, it make sense to proactively prevent the session. Good idea.
> 

Done updating my patch. But it turned out that your guess was correct before
syzbot tries my patch. A sample j1939 program (generated by Google AI search)
succeeded to trigger this race if below delay injection patch is applied.

--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1492,6 +1494,9 @@ static struct j1939_session *j1939_session_new(struct j1939_priv *priv,
        struct j1939_session *session;
        struct j1939_sk_buff_cb *skcb;

+       printk("j1939_session_new() delay start\n");
+       mdelay(5000); // <= Run "ip link del dev vcan0 type vcan" here.
+       printk("j1939_session_new() delay end\n");
        session = kzalloc(sizeof(*session), gfp_any());
        if (!session)
                return NULL;

So, not only we need to make sure that all existing j1939_session are destroyed
but also we need to make sure that no new j1939_session is created if underlying
net_device is no longer in NETREG_REGISTERED state.

F.Y.I. Below change did not help. Also, although j1939_netdev_notify(NETDEV_UNREGISTER)
is called periodically, j1939_cancel_active_session(priv, NULL) is called only once
because j1939_sk_netdev_event_unregister(priv) calls j1939_priv_set(priv->ndev, NULL)
 from __j1939_rx_release(). There is currently no mechanism to retry when we hit
the race above.

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