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

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Fri, Nov 21, 2025 at 07:19:24PM +0900, Tetsuo Handa wrote:
> On 2025/11/21 19:00, Oleksij Rempel wrote:
> >> 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.
> > 
> 
> I see.
> 
> By the way, I am thinking
> 
> diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
> index 88e7160d4248..f12679446990 100644
> --- a/net/can/j1939/socket.c
> +++ b/net/can/j1939/socket.c
> @@ -477,16 +477,22 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
>  		struct net_device *ndev;
>  
>  		ndev = dev_get_by_index(net, addr->can_ifindex);
>  		if (!ndev) {
>  			ret = -ENODEV;
>  			goto out_release_sock;
>  		}
>  
> +		if (ndev->reg_state != NETREG_REGISTERED) {
> +			dev_put(ndev);
> +			ret = -ENODEV;
> +			goto out_release_sock;
> +		}
> +
>  		can_ml = can_get_ml_priv(ndev);
>  		if (!can_ml) {
>  			dev_put(ndev);
>  			ret = -ENODEV;
>  			goto out_release_sock;
>  		}
>  
>  		if (!(ndev->flags & IFF_UP)) {
> 
> as an alternative approach for
> https://lkml.kernel.org/r/9a3f9a95-1f58-4d67-9ab4-1ca360f86f79@I-love.SAKURA.ne.jp
> because I consider that getting a new refcount on net_device should be avoided
> when NETDEV_UNREGISTER event has already started.
> 
> Maybe we can do similar thing for j1939_session in order to avoid getting a new
> refcount on j1939_priv when NETDEV_UNREGISTER event has already started.
> 
> 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.

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