[PATCH can v1 0/2] can: j1939: resend lost patches for buffer init and netdevice tracking

[PATCH can v1 0/2] can: j1939: resend lost patches for buffer init and netdevice tracking

[Oleksij Rempel]

This series collects and resends two j1939 patches that were previously
lost on their way upstream. They address different, unconnected issues
in the stack:

- Patch 1 prevents residual data leaks by zeroing the allocated receive
  buffer in j1939_session_fresh_new().

- Patch 2 implements netdevice_tracker for j1939_{priv,session,ecu}
  management to help investigate a dev_hold/dev_put refcount leak
  (unregister_netdevice waiting for vcan0) reported by syzbot.

Oleksij Rempel (1):
  can: j1939: transport: initialize receive buffer

Tetsuo Handa (1):
  can: j1939: use netdevice_tracker for j1939_{priv,session,ecu}
    tracking

 net/can/j1939/bus.c        | 2 ++
 net/can/j1939/j1939-priv.h | 3 +++
 net/can/j1939/main.c       | 8 ++++----
 net/can/j1939/transport.c  | 4 +++-
 4 files changed, 12 insertions(+), 5 deletions(-)

--
2.47.3

[PATCH can v1 1/2] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking

[Oleksij Rempel]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

syzbot is still reporting

  unregister_netdevice: waiting for vcan0 to become free. Usage count = 2

problem. A debug printk() patch in linux-next-20260508 identified that
there is dev_hold()/dev_put() imbalance in j1939_priv management.

  Call trace for vcan0[26] +4 at
     __dev_hold include/linux/netdevice.h:4470 [inline]
     netdev_hold include/linux/netdevice.h:4513 [inline]
     dev_hold include/linux/netdevice.h:4536 [inline]
     j1939_priv_create net/can/j1939/main.c:140 [inline]
     j1939_netdev_start+0x36b/0xc10 net/can/j1939/main.c:268
     j1939_sk_bind+0x853/0xb30 net/can/j1939/socket.c:506
     __sys_bind_socket net/socket.c:1948 [inline]
     __sys_bind+0x2e9/0x410 net/socket.c:1979

  Call trace for vcan0[28] -3 at
     __dev_put include/linux/netdevice.h:4456 [inline]
     netdev_put include/linux/netdevice.h:4523 [inline]
     dev_put include/linux/netdevice.h:4548 [inline]
     __j1939_priv_release net/can/j1939/main.c:166 [inline]
     kref_put include/linux/kref.h:65 [inline]
     j1939_priv_put+0x128/0x270 net/can/j1939/main.c:172
     j1939_sk_sock_destruct+0x52/0x90 net/can/j1939/socket.c:388
     __sk_destruct+0x8d/0x9d0 net/core/sock.c:2352
     rcu_do_batch kernel/rcu/tree.c:2617 [inline]
     rcu_core kernel/rcu/tree.c:2869 [inline]
     rcu_cpu_kthread+0x99e/0x1470 kernel/rcu/tree.c:2957
     smpboot_thread_fn+0x541/0xa50 kernel/smpboot.c:160
     kthread+0x388/0x470 kernel/kthread.c:436
     ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
     ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

This refcount leak in j1939_priv might be caused by a refcount leak in
j1939_{session,ecu} because j1939_{session,ecu} holds a ref on j1939_priv.
For further investigation using upstream kernels, enable netdevice_tracker
in j1939_{priv,session,ecu} management.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 net/can/j1939/bus.c        | 2 ++
 net/can/j1939/j1939-priv.h | 3 +++
 net/can/j1939/main.c       | 8 ++++----
 net/can/j1939/transport.c  | 2 ++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/can/j1939/bus.c b/net/can/j1939/bus.c
index dc374286eeb6..cdc3c0a71937 100644
--- a/net/can/j1939/bus.c
+++ b/net/can/j1939/bus.c
@@ -20,6 +20,7 @@ static void __j1939_ecu_release(struct kref *kref)
 	struct j1939_priv *priv = ecu->priv;

 	list_del(&ecu->list);
+	netdev_put(priv->ndev, &ecu->priv_dev_tracker);
 	kfree(ecu);
 	j1939_priv_put(priv);
 }
@@ -155,6 +156,7 @@ struct j1939_ecu *j1939_ecu_create_locked(struct j1939_priv *priv, name_t name)
 	if (!ecu)
 		return ERR_PTR(-ENOMEM);
 	kref_init(&ecu->kref);
+	netdev_hold(priv->ndev, &ecu->priv_dev_tracker, gfp_any());
 	ecu->addr = J1939_IDLE_ADDR;
 	ecu->name = name;

diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
index 81f58924b4ac..cf26352d1d8c 100644
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h
@@ -38,6 +38,7 @@ struct j1939_ecu {
 	struct hrtimer ac_timer;
 	struct kref kref;
 	struct j1939_priv *priv;
+	netdevice_tracker priv_dev_tracker;

 	/* count users, to help transport protocol decide for interaction */
 	int nusers;
@@ -60,6 +61,7 @@ struct j1939_priv {
 	rwlock_t lock;

 	struct net_device *ndev;
+	netdevice_tracker dev_tracker;

 	/* list of 256 ecu ptrs, that cache the claimed addresses.
 	 * also protected by the above lock
@@ -230,6 +232,7 @@ enum j1939_session_state {

 struct j1939_session {
 	struct j1939_priv *priv;
+	netdevice_tracker priv_dev_tracker;
 	struct list_head active_session_list_entry;
 	struct list_head sk_session_queue_entry;
 	struct kref kref;
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index 9937c04241bc..5e5e6c228f22 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -137,7 +137,7 @@ static struct j1939_priv *j1939_priv_create(struct net_device *ndev)
 	priv->ndev = ndev;
 	kref_init(&priv->kref);
 	kref_init(&priv->rx_kref);
-	dev_hold(ndev);
+	netdev_hold(ndev, &priv->dev_tracker, GFP_KERNEL);

 	netdev_dbg(priv->ndev, "%s : 0x%p\n", __func__, priv);

@@ -163,7 +163,7 @@ static void __j1939_priv_release(struct kref *kref)
 	WARN_ON_ONCE(!list_empty(&priv->ecus));
 	WARN_ON_ONCE(!list_empty(&priv->j1939_socks));

-	dev_put(ndev);
+	netdev_put(ndev, &priv->dev_tracker);
 	kfree(priv);
 }

@@ -281,7 +281,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
 		 */
 		kref_get(&priv_new->rx_kref);
 		mutex_unlock(&j1939_netdev_lock);
-		dev_put(ndev);
+		netdev_put(ndev, &priv->dev_tracker);
 		kfree(priv);
 		return priv_new;
 	}
@@ -298,7 +298,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
 	j1939_priv_set(ndev, NULL);
 	mutex_unlock(&j1939_netdev_lock);

-	dev_put(ndev);
+	netdev_put(ndev, &priv->dev_tracker);
 	kfree(priv);

 	return ERR_PTR(ret);
diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 8a31cb23bc76..98f96362b20f 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -283,6 +283,7 @@ static void j1939_session_destroy(struct j1939_session *session)
 		kfree_skb(skb);
 	}
 	__j1939_session_drop(session);
+	netdev_put(session->priv->ndev, &session->priv_dev_tracker);
 	j1939_priv_put(session->priv);
 	kfree(session);
 }
@@ -1526,6 +1527,7 @@ static struct j1939_session *j1939_session_new(struct j1939_priv *priv,
 	INIT_LIST_HEAD(&session->active_session_list_entry);
 	INIT_LIST_HEAD(&session->sk_session_queue_entry);
 	kref_init(&session->kref);
+	netdev_hold(priv->ndev, &session->priv_dev_tracker, gfp_any());

 	j1939_priv_get(priv);
 	session->priv = priv;
--
2.47.3

Re: [PATCH can v1 1/2] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] ECU list `priv->ecus` is modified locklessly during concurrent unmapping.
--

--- Patch [1]: [PATCH can v1 1/2] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking ---
commit fa6ae711cbc8e468a294fc1a54de5efa19c0b5b3
Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking
    
This patch enables netdevice_tracker for j1939_priv, j1939_session, and
j1939_ecu structures. It aims to help identify the source of refcount
leaks reported by syzbot during unregister_netdevice().

> diff --git a/net/can/j1939/bus.c b/net/can/j1939/bus.c
> index dc374286eeb6e..cdc3c0a719379 100644
> --- a/net/can/j1939/bus.c
> +++ b/net/can/j1939/bus.c
> @@ -20,6 +20,7 @@ static void __j1939_ecu_release(struct kref *kref)
>  	struct j1939_priv *priv = ecu->priv;
>  
>  	list_del(&ecu->list);

[Severity: High]
This is a pre-existing issue, but is it safe to modify priv->ecus here without
holding priv->lock?

If a process unbinds a socket and drops its ECU reference concurrently with the
network receive path processing a packet:

CPU1 (Socket thread)
j1939_local_ecu_put()
  /* drops refcount to 1 */

CPU2 (SoftIRQ)
j1939_ac_recv()
  j1939_ecu_put(ecu); /* locklessly drops final reference */

This leads to __j1939_ecu_release() executing list_del(&ecu->list) and
kfree(ecu) without holding priv->lock. 

Can this happen concurrently with other threads iterating priv->ecus under
priv->lock, potentially resulting in data races, list corruption, and
use-after-free bugs?

> +	netdev_put(priv->ndev, &ecu->priv_dev_tracker);
>  	kfree(ecu);
>  	j1939_priv_put(priv);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728055835.1151785-1-o.rempel@pengutronix.de?part=1

Re: [PATCH can v1 1/2] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking

[Oleksij Rempel]

On Tue, Jul 28, 2026 at 06:13:38AM +0000, sashiko-bot@kernel.org wrote:
> --- Patch [1]: [PATCH can v1 1/2] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking ---
> commit fa6ae711cbc8e468a294fc1a54de5efa19c0b5b3
> Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> 
> can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking
>     
> This patch enables netdevice_tracker for j1939_priv, j1939_session, and
> j1939_ecu structures. It aims to help identify the source of refcount
> leaks reported by syzbot during unregister_netdevice().
> 
> > diff --git a/net/can/j1939/bus.c b/net/can/j1939/bus.c
> > index dc374286eeb6e..cdc3c0a719379 100644
> > --- a/net/can/j1939/bus.c
> > +++ b/net/can/j1939/bus.c
> > @@ -20,6 +20,7 @@ static void __j1939_ecu_release(struct kref *kref)
> >  	struct j1939_priv *priv = ecu->priv;
> >  
> >  	list_del(&ecu->list);
> 
> [Severity: High]
> This is a pre-existing issue, but is it safe to modify priv->ecus here without
> holding priv->lock?
> 
> If a process unbinds a socket and drops its ECU reference concurrently with the
> network receive path processing a packet:

Ack - we will keep it mind. This is not blocker for current patch set.

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 |

[PATCH v1 can 2/2] can: j1939: transport: initialize receive buffer

[Oleksij Rempel]

Zero the allocated buffer in j1939_session_fresh_new() to ensure it
contains no residual data.

While there is a potential performance impact if users allocate maximum
sized ETP buffers, most real-world use cases are not noticeably affected
since the maximum known buffer size is typically around 65K.

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Reported-by: Ji'an Zhou <eilaimemedsnaimel@gmail.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 net/can/j1939/transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 98f96362b20f..8fcfd13e5e6f 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1581,7 +1581,7 @@ j1939_session *j1939_session_fresh_new(struct j1939_priv *priv,
 	}

 	/* alloc data area */
-	skb_put(skb, size);
+	skb_put_zero(skb, size);
 	/* skb is recounted in j1939_session_new() */
 	return session;
 }
--
2.47.3