All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/20] pull-request: can 2026-07-29
@ 2026-07-29 10:12 Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking Marc Kleine-Budde
                   ` (19 more replies)
  0 siblings, 20 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello netdev-team,

this is a pull request of 20 patches for net/main.

The first 2 patches fix problems in the CAN J1939 protocol and are by
Tetsuo Handa and Oleksij Rempel.

The next 2 patches fix problems in the CAN ISOTP protocol and are by
Oliver Hartkopp and Minhong He.

Avi Weiss contributes contributed 4 fixes for the ctucanfd, Pengpeng
Hou's patch adds a missing MODULE_DEVICE_TABLE.

The patches for the peak_usb driver are contributed by James Gao,
Maoyi Xie, Maoyi Xie and add sanity checks for the USB bulk data
parsing and fix a double free.

2 fixes for the kvaser_usb driver are provided by Abdun Nihaal and
Pengpeng Hou, a mem leak is fixed and sanity checks for the USB bulk
data parsing.

Tu Nguyen's patch for the rcar_canfd driver fixes the initializing
flow.

Pengpeng Hou contributes a patch for the softing driver to validate
the firmware record spans.

Lucas Martins Alves's patch for the c_can driver keeps the controller
in init mode until configuration is complete.

A patch by my add missing URB resubmission on skb allocation failure
to the gs_usb driver.

Guangshuo Li's patch for the etas_es58x driver fixes a RX buffer leak.

The last patch is by Pengpeng Hou and adds sanity checks to the USB
bulk data parsing of the ems_usb driver.

regards,
Marc

---

The following changes since commit 3bd438a58e910db5dc369aa25dfed1fc95f1b596:

  octeontx2-af: Block VFs from clobbering special CGX PKIND state (2026-07-28 18:52:58 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-7.2-20260729

for you to fetch changes up to 02925f51377f2a42a6724f00549167499c9302e5:

  can: ems_usb: validate CPC message lengths (2026-07-29 12:00:18 +0200)

----------------------------------------------------------------
linux-can-fixes-for-7.2-20260729

----------------------------------------------------------------
Abdun Nihaal (1):
      can: kvaser_usb: kvaser_usb_hydra_get_busparams(): fix memory leak in kvaser_usb_hydra_get_busparams()

Avi Weiss (4):
      can: ctucanfd: unmap BAR0 using base address
      can: ctucanfd: mark error-active controller status valid
      can: ctucanfd: handle bus error interrupts
      can: ctucanfd: use self-test mode for PRESUME_ACK

Guangshuo Li (1):
      can: etas_es58x: es58x_read_bulk_callback(): fix RX buffer leak on URB resubmit failure

James Gao (1):
      can: peak_usb: add bounds check for USB channel index

Lucas Martins Alves (1):
      can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured

Maoyi Xie (1):
      can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error

Marc Kleine-Budde (2):
      Merge patch series "can: j1939: resend lost patches for buffer init and netdevice tracking"
      can: gs_usb: gs_usb_receive_bulk_callback(): resubmit URB on skb allocation failure

Minhong He (1):
      can: isotp: check register_netdevice_notifier() error in module init

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

Oliver Hartkopp (1):
      can: isotp: fix timer drain order, wakeup handling and tx_gen ordering

Pengpeng Hou (5):
      can: ctucanfd: add missing MODULE_DEVICE_TABLE()
      can: peak_usb: validate uCAN receive record lengths
      can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents
      can: softing: fw_parse(): validate firmware record spans
      can: ems_usb: validate CPC message lengths

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

Tu Nguyen (1):
      can: rcar_canfd: change the initializing flow for clocks and resets

 drivers/net/can/c_can/c_can_main.c                |   8 +-
 drivers/net/can/ctucanfd/ctucanfd_base.c          |  14 +-
 drivers/net/can/ctucanfd/ctucanfd_pci.c           |   3 +-
 drivers/net/can/rcar/rcar_canfd.c                 |  32 +--
 drivers/net/can/softing/softing_fw.c              |  46 ++-
 drivers/net/can/usb/ems_usb.c                     |  43 +++
 drivers/net/can/usb/etas_es58x/es58x_core.c       |   1 -
 drivers/net/can/usb/gs_usb.c                      |   4 +-
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c |   1 +
 drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c  |  13 +-
 drivers/net/can/usb/peak_usb/pcan_usb_core.c      |   1 -
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c        |  40 ++-
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c       |  20 +-
 net/can/isotp.c                                   | 334 ++++++++++++++++------
 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 +-
 18 files changed, 428 insertions(+), 149 deletions(-)

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 02/20] can: j1939: transport: j1939_session_fresh_new(): initialize receive buffer Marc Kleine-Budde
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Tetsuo Handa, Oleksij Rempel,
	stable, Marc Kleine-Budde

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>
Link: https://patch.msgid.link/20260728055835.1151785-2-o.rempel@pengutronix.de
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@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;

base-commit: 3bd438a58e910db5dc369aa25dfed1fc95f1b596
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 02/20] can: j1939: transport: j1939_session_fresh_new(): initialize receive buffer
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering Marc Kleine-Budde
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Oleksij Rempel, Ji'an Zhou,
	stable, Marc Kleine-Budde

From: Oleksij Rempel <o.rempel@pengutronix.de>

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>
Message-ID: <CAPAUci5dykCLjoijqkUtFqJFesgncrD7+S6y_V=gjbFkY2Tifg@mail.gmail.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://patch.msgid.link/20260728055835.1151785-3-o.rempel@pengutronix.de
Cc: stable@kernel.org
[mkl: add Message-ID]
Signed-off-by: Marc Kleine-Budde <mkl@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.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 02/20] can: j1939: transport: j1939_session_fresh_new(): initialize receive buffer Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 04/20] can: isotp: check register_netdevice_notifier() error in module init Marc Kleine-Budde
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, stable,
	Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

This patch is a follow-up to commit cf070fe33bfb ("can: isotp: serialize
TX state transitions under so->rx_lock") which addresses following
sashiko-bot findings:

- isotp_sendmsg(): drain so->txfrtimer first so a stale callback can't
  re-arm echotimer after the claim

- isotp_release(): wake so->wait after forcing ISOTP_SHUTDOWN so a
  sleeping sendmsg() claim isn't stranded

- isotp_sendmsg(): have both wait_event_interruptible() calls in
  isotp_sendmsg() also wake on ISOTP_SHUTDOWN and do not return claim to
  IDLE to avoid corrupting a concurrent isotp_release() process.

- isotp_sendmsg(): handle potential claim of a new transfer when
  the wait_event_interruptible() call returns in CAN_ISOTP_WAIT_TX_DONE
  mode. Don't touch timers and states of the new transfer if a new thread
  incremented so->tx_gen before getting the lock at err_event_drop.

- isotp_sendmsg(): handle a stuck can_send() and omit timer and state
  changes if a new transfer was claimed. wait_tx_done() returns the error
  recorded in so->tx_result[], tagged with the caller's own generation.

- isotp_tx_timeout(): on a claimed timeout, record the ECOMM error for
  the timed-out transfer's own generation in so->tx_result[]; sk->sk_err
  is raised unconditionally, same as every other error path here.

- isotp_tx_gen_done()/isotp_tx_timeout(): always read tx.state (acquire)
  before tx_gen - the reverse order let a weakly ordered CPU pair a fresh
  tx.state with a stale tx_gen/tx_result slot.

- isotp_sendmsg(): wait_tx_done: drain sk_err via sock_error() once we
  have read the result from so->tx_result[], so an already-reported error
  doesn't stay latched for a later poll()/SO_ERROR.

Also align the remaining lock-free so->tx.state/rx.state/cfecho accesses
and use skb->hash as unique loopback echo frame indicator.

Fixes: cf070fe33bfb ("can: isotp: serialize TX state transitions under so->rx_lock")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260724181525.43556-1-socketcan@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 317 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 230 insertions(+), 87 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 54becaf6898f..1f11c66b343c 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -127,6 +127,15 @@ MODULE_PARM_DESC(max_pdu_size, "maximum isotp pdu size (default "
 #define ISOTP_FC_TIMEOUT 1	/* 1 sec */
 #define ISOTP_ECHO_TIMEOUT 2	/* 2 secs */
 
+/* so->tx_result[so->tx_gen % ISOTP_TX_RESULT_SLOTS] holds the packed value
+ * (err << ISOTP_TX_RESULT_GEN_BITS | gen) for each tx generation slot, so it
+ * can be handled with a single READ_ONCE()/WRITE_ONCE() access.
+ */
+#define ISOTP_TX_RESULT_SLOTS 4
+#define ISOTP_TX_RESULT_GEN_BITS 24
+#define ISOTP_TX_RESULT_GEN_MASK ((1U << ISOTP_TX_RESULT_GEN_BITS) - 1)
+#define ISOTP_TX_RESULT_ERR_MASK 0xFF
+
 enum {
 	ISOTP_IDLE = 0,
 	ISOTP_WAIT_FIRST_FC,
@@ -166,7 +175,8 @@ struct isotp_sock {
 	u32 force_tx_stmin;
 	u32 force_rx_stmin;
 	u32 cfecho; /* consecutive frame echo tag */
-	u32 tx_gen; /* generation, bumped per new tx transfer */
+	u32 tx_gen; /* transfer generation, increased per new tx transfer */
+	u32 tx_result[ISOTP_TX_RESULT_SLOTS]; /* per-generation result slots */
 	struct tpcon rx, tx;
 	struct list_head notifier;
 	wait_queue_head_t wait;
@@ -177,6 +187,65 @@ static LIST_HEAD(isotp_notifier_list);
 static DEFINE_SPINLOCK(isotp_notifier_lock);
 static struct isotp_sock *isotp_busy_notifier;
 
+/* increase (24 bit) tx generation value */
+static u32 isotp_inc_tx_gen(u32 gen)
+{
+	return (gen + 1) & ISOTP_TX_RESULT_GEN_MASK;
+}
+
+/* store 8 bit error and 24 bit tx generation values in packed u32 element */
+static u32 isotp_pack_tx_result(u32 gen, int err)
+{
+	return gen | ((u32)err << ISOTP_TX_RESULT_GEN_BITS);
+}
+
+/* get the 24 bit tx generation value from the tx result */
+static u32 isotp_get_tx_gen(u32 gen_err)
+{
+	return gen_err & ISOTP_TX_RESULT_GEN_MASK;
+}
+
+/* get the 8 bit error value from the tx result */
+static u32 isotp_get_tx_err(u32 gen_err)
+{
+	return (gen_err >> ISOTP_TX_RESULT_GEN_BITS) & ISOTP_TX_RESULT_ERR_MASK;
+}
+
+/* store transfer result in per-generation%4 so->tx_result[] slot */
+static void isotp_set_tx_result(struct isotp_sock *so, u32 gen, int err)
+{
+	WRITE_ONCE(so->tx_result[gen % ISOTP_TX_RESULT_SLOTS],
+		   isotp_pack_tx_result(gen, err));
+}
+
+/* fetch the result recorded for 'gen', as a (negative) errno (0 for success) */
+static int isotp_get_tx_result(struct isotp_sock *so, u32 gen)
+{
+	u32 result = READ_ONCE(so->tx_result[gen % ISOTP_TX_RESULT_SLOTS]);
+
+	if (isotp_get_tx_gen(result) != gen) {
+		pr_notice_once("can-isotp: tx_result[] slot reused before read\n");
+
+		/* report failure rather than risk a false success */
+		return -ECOMM;
+	}
+
+	return -(isotp_get_tx_err(result));
+}
+
+/* true if done, shut down or superseded ('gen' is no longer the active
+ * transfer). Reads tx.state first (acquire) so tx_gen/tx_result reads
+ * below see at least what that state write published (common sequence).
+ */
+static bool isotp_tx_gen_done(struct isotp_sock *so, u32 gen)
+{
+	/* read tx.state first for the common sequence */
+	u32 state = smp_load_acquire(&so->tx.state);
+
+	return state == ISOTP_IDLE || state == ISOTP_SHUTDOWN ||
+	       READ_ONCE(so->tx_gen) != gen;
+}
+
 static inline struct isotp_sock *isotp_sk(const struct sock *sk)
 {
 	return (struct isotp_sock *)sk;
@@ -199,7 +268,7 @@ static enum hrtimer_restart isotp_rx_timer_handler(struct hrtimer *hrtimer)
 					     rxtimer);
 	struct sock *sk = &so->sk;
 
-	if (so->rx.state == ISOTP_WAIT_DATA) {
+	if (READ_ONCE(so->rx.state) == ISOTP_WAIT_DATA) {
 		/* we did not get new data frames in time */
 
 		/* report 'connection timed out' */
@@ -208,7 +277,7 @@ static enum hrtimer_restart isotp_rx_timer_handler(struct hrtimer *hrtimer)
 			sk_error_report(sk);
 
 		/* reset rx state */
-		so->rx.state = ISOTP_IDLE;
+		WRITE_ONCE(so->rx.state, ISOTP_IDLE);
 	}
 
 	return HRTIMER_NORESTART;
@@ -372,20 +441,19 @@ static void isotp_send_cframe(struct isotp_sock *so);
 static int isotp_rcv_fc(struct isotp_sock *so, struct canfd_frame *cf, int ae)
 {
 	struct sock *sk = &so->sk;
+	int tx_err = EBADMSG; /* default for unknown FC status */
 
-	if (so->tx.state != ISOTP_WAIT_FC &&
-	    so->tx.state != ISOTP_WAIT_FIRST_FC)
+	if (READ_ONCE(so->tx.state) != ISOTP_WAIT_FC &&
+	    READ_ONCE(so->tx.state) != ISOTP_WAIT_FIRST_FC)
 		return 0;
 
 	hrtimer_cancel(&so->txtimer);
 
 	/* isotp_tx_timeout() may have given up on this job while
-	 * hrtimer_cancel() above waited for it to finish; so->rx_lock
-	 * (held by our caller isotp_rcv()) rules out a concurrent claim,
-	 * so a plain recheck is enough here.
+	 * hrtimer_cancel() above waited for it to finish => recheck
 	 */
-	if (so->tx.state != ISOTP_WAIT_FC &&
-	    so->tx.state != ISOTP_WAIT_FIRST_FC)
+	if (READ_ONCE(so->tx.state) != ISOTP_WAIT_FC &&
+	    READ_ONCE(so->tx.state) != ISOTP_WAIT_FIRST_FC)
 		return 1;
 
 	if ((cf->len < ae + FC_CONTENT_SZ) ||
@@ -396,13 +464,15 @@ static int isotp_rcv_fc(struct isotp_sock *so, struct canfd_frame *cf, int ae)
 		if (!sock_flag(sk, SOCK_DEAD))
 			sk_error_report(sk);
 
-		so->tx.state = ISOTP_IDLE;
+		isotp_set_tx_result(so, so->tx_gen, EBADMSG);
+		/* set to IDLE after publishing tx_result */
+		smp_store_release(&so->tx.state, ISOTP_IDLE);
 		wake_up_interruptible(&so->wait);
 		return 1;
 	}
 
 	/* get static/dynamic communication params from first/every FC frame */
-	if (so->tx.state == ISOTP_WAIT_FIRST_FC ||
+	if (READ_ONCE(so->tx.state) == ISOTP_WAIT_FIRST_FC ||
 	    so->opt.flags & CAN_ISOTP_DYN_FC_PARMS) {
 		so->txfc.bs = cf->data[ae + 1];
 		so->txfc.stmin = cf->data[ae + 2];
@@ -426,13 +496,13 @@ static int isotp_rcv_fc(struct isotp_sock *so, struct canfd_frame *cf, int ae)
 			so->tx_gap = ktime_add_ns(so->tx_gap,
 						  (so->txfc.stmin - 0xF0)
 						  * 100000);
-		so->tx.state = ISOTP_WAIT_FC;
+		WRITE_ONCE(so->tx.state, ISOTP_WAIT_FC);
 	}
 
 	switch (cf->data[ae] & 0x0F) {
 	case ISOTP_FC_CTS:
 		so->tx.bs = 0;
-		so->tx.state = ISOTP_SENDING;
+		WRITE_ONCE(so->tx.state, ISOTP_SENDING);
 		/* send CF frame and enable echo timeout handling */
 		hrtimer_start(&so->echotimer, ktime_set(ISOTP_ECHO_TIMEOUT, 0),
 			      HRTIMER_MODE_REL_SOFT);
@@ -447,14 +517,19 @@ static int isotp_rcv_fc(struct isotp_sock *so, struct canfd_frame *cf, int ae)
 
 	case ISOTP_FC_OVFLW:
 		/* overflow on receiver side - report 'message too long' */
-		sk->sk_err = EMSGSIZE;
-		if (!sock_flag(sk, SOCK_DEAD))
-			sk_error_report(sk);
+		tx_err = EMSGSIZE;
 		fallthrough;
 
 	default:
-		/* stop this tx job */
-		so->tx.state = ISOTP_IDLE;
+		/* reserved/unknown flow status (tx_err defaults to EBADMSG) */
+
+		sk->sk_err = tx_err;
+		if (!sock_flag(sk, SOCK_DEAD))
+			sk_error_report(sk);
+
+		isotp_set_tx_result(so, so->tx_gen, tx_err);
+		/* set to IDLE after publishing tx_result */
+		smp_store_release(&so->tx.state, ISOTP_IDLE);
 		wake_up_interruptible(&so->wait);
 	}
 	return 0;
@@ -467,7 +542,7 @@ static int isotp_rcv_sf(struct sock *sk, struct canfd_frame *cf, int pcilen,
 	struct sk_buff *nskb;
 
 	hrtimer_cancel(&so->rxtimer);
-	so->rx.state = ISOTP_IDLE;
+	WRITE_ONCE(so->rx.state, ISOTP_IDLE);
 
 	if (!len || len > cf->len - pcilen)
 		return 1;
@@ -501,7 +576,7 @@ static int isotp_rcv_ff(struct sock *sk, struct canfd_frame *cf, int ae)
 	int ff_pci_sz;
 
 	hrtimer_cancel(&so->rxtimer);
-	so->rx.state = ISOTP_IDLE;
+	WRITE_ONCE(so->rx.state, ISOTP_IDLE);
 
 	/* get the used sender LL_DL from the (first) CAN frame data length */
 	so->rx.ll_dl = padlen(cf->len);
@@ -555,7 +630,7 @@ static int isotp_rcv_ff(struct sock *sk, struct canfd_frame *cf, int ae)
 
 	/* initial setup for this pdu reception */
 	so->rx.sn = 1;
-	so->rx.state = ISOTP_WAIT_DATA;
+	WRITE_ONCE(so->rx.state, ISOTP_WAIT_DATA);
 
 	/* no creation of flow control frames */
 	if (so->opt.flags & CAN_ISOTP_LISTEN_MODE)
@@ -573,7 +648,7 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 	struct sk_buff *nskb;
 	int i;
 
-	if (so->rx.state != ISOTP_WAIT_DATA)
+	if (READ_ONCE(so->rx.state) != ISOTP_WAIT_DATA)
 		return 0;
 
 	/* drop if timestamp gap is less than force_rx_stmin nano secs */
@@ -588,11 +663,9 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 	hrtimer_cancel(&so->rxtimer);
 
 	/* isotp_rx_timer_handler() may have raced us for so->rx.state
-	 * while hrtimer_cancel() above waited for it to finish, already
-	 * reporting ETIMEDOUT and resetting the reception; don't process
-	 * this CF into a reassembly that has already been given up on.
+	 * while hrtimer_cancel() above waited for it to finish => recheck
 	 */
-	if (so->rx.state != ISOTP_WAIT_DATA)
+	if (READ_ONCE(so->rx.state) != ISOTP_WAIT_DATA)
 		return 1;
 
 	/* CFs are never longer than the FF */
@@ -613,7 +686,7 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 			sk_error_report(sk);
 
 		/* reset rx state */
-		so->rx.state = ISOTP_IDLE;
+		WRITE_ONCE(so->rx.state, ISOTP_IDLE);
 		return 1;
 	}
 	so->rx.sn++;
@@ -627,7 +700,7 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 
 	if (so->rx.idx >= so->rx.len) {
 		/* we are done */
-		so->rx.state = ISOTP_IDLE;
+		WRITE_ONCE(so->rx.state, ISOTP_IDLE);
 
 		if ((so->opt.flags & ISOTP_CHECK_PADDING) &&
 		    check_pad(so, cf, i + 1, so->opt.rxpad_content)) {
@@ -698,8 +771,10 @@ static void isotp_rcv(struct sk_buff *skb, void *data)
 
 	if (so->opt.flags & CAN_ISOTP_HALF_DUPLEX) {
 		/* check rx/tx path half duplex expectations */
-		if ((so->tx.state != ISOTP_IDLE && n_pci_type != N_PCI_FC) ||
-		    (so->rx.state != ISOTP_IDLE && n_pci_type == N_PCI_FC))
+		if ((READ_ONCE(so->tx.state) != ISOTP_IDLE &&
+		     n_pci_type != N_PCI_FC) ||
+		    (READ_ONCE(so->rx.state) != ISOTP_IDLE &&
+		     n_pci_type == N_PCI_FC))
 			goto out_unlock;
 	}
 
@@ -794,6 +869,7 @@ static void isotp_send_cframe(struct isotp_sock *so)
 	struct canfd_frame *cf;
 	int can_send_ret;
 	int ae = (so->opt.flags & CAN_ISOTP_EXTEND_ADDR) ? 1 : 0;
+	u32 old_cfecho;
 
 	dev = dev_get_by_index(sock_net(sk), so->ifindex);
 	if (!dev)
@@ -814,6 +890,9 @@ static void isotp_send_cframe(struct isotp_sock *so)
 
 	csx->can_iif = dev->ifindex;
 
+	/* set uid in tx skb to identify CF echo frames */
+	can_set_skb_uid(skb);
+
 	cf = (struct canfd_frame *)skb->data;
 	skb_put_zero(skb, so->ll.mtu);
 
@@ -830,12 +909,15 @@ static void isotp_send_cframe(struct isotp_sock *so)
 	skb->dev = dev;
 	can_skb_set_owner(skb, sk);
 
-	/* cfecho should have been zero'ed by init/isotp_rcv_echo() */
-	if (so->cfecho)
-		pr_notice_once("can-isotp: cfecho is %08X != 0\n", so->cfecho);
+	/* zero'ed by init/isotp_rcv_echo(); reached lock-free via
+	 * isotp_txfr_timer_handler() too, so use READ_ONCE()/WRITE_ONCE()
+	 */
+	old_cfecho = READ_ONCE(so->cfecho);
+	if (old_cfecho)
+		pr_notice_once("can-isotp: cfecho is %08X != 0\n", old_cfecho);
 
 	/* set consecutive frame echo tag */
-	so->cfecho = *(u32 *)cf->data;
+	WRITE_ONCE(so->cfecho, skb->hash);
 
 	/* send frame with local echo enabled */
 	can_send_ret = can_send(skb, 1);
@@ -887,7 +969,6 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
 {
 	struct sock *sk = (struct sock *)data;
 	struct isotp_sock *so = isotp_sk(sk);
-	struct canfd_frame *cf = (struct canfd_frame *)skb->data;
 
 	/* only handle my own local echo CF/SF skb's (no FF!) */
 	if (skb->sk != sk)
@@ -899,32 +980,35 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
 	spin_lock(&so->rx_lock);
 
 	/* so->cfecho may since belong to a new transfer; recheck under lock */
-	if (so->cfecho != *(u32 *)cf->data)
+	if (READ_ONCE(so->cfecho) != skb->hash)
 		goto out_unlock;
 
 	/* cancel local echo timeout */
 	hrtimer_cancel(&so->echotimer);
 
 	/* local echo skb with consecutive frame has been consumed */
-	so->cfecho = 0;
+	WRITE_ONCE(so->cfecho, 0);
 
 	/* claiming a transfer also takes so->rx_lock, so a plain recheck
 	 * is enough: so->tx.state can't have flipped to ISOTP_SENDING for
 	 * a new claim while we're still in here
 	 */
-	if (so->tx.state != ISOTP_SENDING)
+	if (READ_ONCE(so->tx.state) != ISOTP_SENDING)
 		goto out_unlock;
 
 	if (so->tx.idx >= so->tx.len) {
 		/* we are done */
-		so->tx.state = ISOTP_IDLE;
+
+		isotp_set_tx_result(so, so->tx_gen, 0);
+		/* set to IDLE after publishing tx_result */
+		smp_store_release(&so->tx.state, ISOTP_IDLE);
 		wake_up_interruptible(&so->wait);
 		goto out_unlock;
 	}
 
 	if (so->txfc.bs && so->tx.bs >= so->txfc.bs) {
 		/* stop and wait for FC with timeout */
-		so->tx.state = ISOTP_WAIT_FC;
+		WRITE_ONCE(so->tx.state, ISOTP_WAIT_FC);
 		hrtimer_start(&so->txtimer, ktime_set(ISOTP_FC_TIMEOUT, 0),
 			      HRTIMER_MODE_REL_SOFT);
 		goto out_unlock;
@@ -946,16 +1030,20 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
 	spin_unlock(&so->rx_lock);
 }
 
-/* shared by so->txtimer's and so->echotimer's callbacks. Both timers get
- * cancelled under so->rx_lock elsewhere, so this must stay lock-free to
- * avoid deadlocking with that; uses so->tx_gen instead to avoid tainting
- * a new transfer with an error from the one that just timed out.
+/* isotp_tx_timeout: we did not get any flow control or echo frame in time
+ *
+ * Shared by so->txtimer's and so->echotimer's callbacks. Both timers get
+ * cancelled under so->rx_lock elsewhere, so this must stay lock-free.
+ *
+ * tx.state is acquired before tx_gen. Common sequence in isotp_tx_gen_done().
+ * cmpxchg() only orders itself, not the two preceding loads.
  */
 static enum hrtimer_restart isotp_tx_timeout(struct isotp_sock *so)
 {
 	struct sock *sk = &so->sk;
+	/* read tx.state first for the common sequence */
+	u32 old_state = smp_load_acquire(&so->tx.state);
 	u32 gen = READ_ONCE(so->tx_gen);
-	u32 old_state = READ_ONCE(so->tx.state);
 
 	/* don't handle timeouts in IDLE or SHUTDOWN state */
 	if (old_state == ISOTP_IDLE || old_state == ISOTP_SHUTDOWN)
@@ -965,14 +1053,14 @@ static enum hrtimer_restart isotp_tx_timeout(struct isotp_sock *so)
 	if (cmpxchg(&so->tx.state, old_state, ISOTP_IDLE) != old_state)
 		return HRTIMER_NORESTART;
 
-	/* we did not get any flow control or echo frame in time */
+	/* detected timeout: report 'communication error on send' */
 
-	if (READ_ONCE(so->tx_gen) == gen) {
-		/* report 'communication error on send' */
-		sk->sk_err = ECOMM;
-		if (!sock_flag(sk, SOCK_DEAD))
-			sk_error_report(sk);
-	}
+	/* a stale read of this slot by a waiter still falls back to ECOMM */
+	isotp_set_tx_result(so, gen, ECOMM);
+
+	sk->sk_err = ECOMM;
+	if (!sock_flag(sk, SOCK_DEAD))
+		sk_error_report(sk);
 
 	wake_up_interruptible(&so->wait);
 
@@ -1007,7 +1095,7 @@ static enum hrtimer_restart isotp_txfr_timer_handler(struct hrtimer *hrtimer)
 		      HRTIMER_MODE_REL_SOFT);
 
 	/* cfecho should be consumed by isotp_rcv_echo() here */
-	if (so->tx.state == ISOTP_SENDING && !so->cfecho)
+	if (READ_ONCE(so->tx.state) == ISOTP_SENDING && !READ_ONCE(so->cfecho))
 		isotp_send_cframe(so);
 
 	return HRTIMER_NORESTART;
@@ -1026,10 +1114,12 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	s64 hrtimer_sec = ISOTP_ECHO_TIMEOUT;
 	struct hrtimer *tx_hrt = &so->echotimer;
 	u32 new_state = ISOTP_SENDING;
+	u32 my_gen;
+	u32 old_cfecho;
 	int off;
 	int err;
 
-	if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
+	if (!so->bound || READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN)
 		return -EADDRNOTAVAIL;
 
 	/* claim the socket under so->rx_lock: this serializes the claim
@@ -1046,29 +1136,33 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		if (msg->msg_flags & MSG_DONTWAIT)
 			return -EAGAIN;
 
-		if (so->tx.state == ISOTP_SHUTDOWN)
+		if (READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN)
 			return -EADDRNOTAVAIL;
 
 		/* wait for complete transmission of current pdu */
 		err = wait_event_interruptible(so->wait,
-					       so->tx.state == ISOTP_IDLE);
+					       READ_ONCE(so->tx.state) == ISOTP_IDLE ||
+					       READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN);
 		if (err)
 			return err;
 	}
 
-	/* new transfer: bump so->tx_gen and drain the old one's timers,
-	 * still under the so->rx_lock we just claimed the socket with
-	 */
-	WRITE_ONCE(so->tx.state, ISOTP_SENDING);
-	WRITE_ONCE(so->tx_gen, READ_ONCE(so->tx_gen) + 1);
+	/* txfrtimer's callback re-arms echotimer lock-free: drain it first */
+	hrtimer_cancel(&so->txfrtimer);
 	hrtimer_cancel(&so->txtimer);
 	hrtimer_cancel(&so->echotimer);
-	hrtimer_cancel(&so->txfrtimer);
-	so->cfecho = 0;
+
+	/* new transfer: increment so->tx_gen and set tx.state after barrier */
+	my_gen = isotp_inc_tx_gen(READ_ONCE(so->tx_gen));
+	isotp_set_tx_result(so, my_gen, ECOMM); /* prevent stale slot matching */
+	WRITE_ONCE(so->tx_gen, my_gen);
+	smp_wmb(); /* see smp_load_acquire() in isotp_tx_[timeout|gen_done] */
+	WRITE_ONCE(so->tx.state, ISOTP_SENDING);
+	WRITE_ONCE(so->cfecho, 0);
 	spin_unlock_bh(&so->rx_lock);
 
 	/* so->bound is only checked once above - a wakeup may have
-	 * unbound/rebound the socket meanwhile, so re-validate it
+	 * unbound/rebound the socket meanwhile => recheck
 	 */
 	if (!so->bound) {
 		err = -EADDRNOTAVAIL;
@@ -1127,6 +1221,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 
 	csx->can_iif = dev->ifindex;
 
+	/* set uid in tx skb to identify CF echo frames */
+	can_set_skb_uid(skb);
+
 	so->tx.len = size;
 	so->tx.idx = 0;
 
@@ -1134,8 +1231,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	skb_put_zero(skb, so->ll.mtu);
 
 	/* cfecho should have been zero'ed by init / former isotp_rcv_echo() */
-	if (so->cfecho)
-		pr_notice_once("can-isotp: uninit cfecho %08X\n", so->cfecho);
+	old_cfecho = READ_ONCE(so->cfecho);
+	if (old_cfecho)
+		pr_notice_once("can-isotp: uninit cfecho %08X\n", old_cfecho);
 
 	/* check for single frame transmission depending on TX_DL */
 	if (size <= so->tx.ll_dl - SF_PCI_SZ4 - ae - off) {
@@ -1163,7 +1261,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			cf->data[ae] |= size;
 
 		/* set CF echo tag for isotp_rcv_echo() (SF-mode) */
-		so->cfecho = *(u32 *)cf->data;
+		WRITE_ONCE(so->cfecho, skb->hash);
 	} else {
 		/* send first frame */
 
@@ -1180,7 +1278,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			so->txfc.bs = 0;
 
 			/* set CF echo tag for isotp_rcv_echo() (CF-mode) */
-			so->cfecho = *(u32 *)cf->data;
+			WRITE_ONCE(so->cfecho, skb->hash);
 		} else {
 			/* standard flow control check */
 			new_state = ISOTP_WAIT_FIRST_FC;
@@ -1190,12 +1288,12 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			tx_hrt = &so->txtimer;
 
 			/* no CF echo tag for isotp_rcv_echo() (FF-mode) */
-			so->cfecho = 0;
+			WRITE_ONCE(so->cfecho, 0);
 		}
 	}
 
 	spin_lock_bh(&so->rx_lock);
-	if (so->tx.state == ISOTP_SHUTDOWN) {
+	if (READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN) {
 		/* isotp_release() has since taken over and already drained
 		 * our timers - don't send into a socket that's going away
 		 */
@@ -1206,7 +1304,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		return -EADDRNOTAVAIL;
 	}
 	/* WAIT_FIRST_FC for standard FF, else stays ISOTP_SENDING */
-	so->tx.state = new_state;
+	WRITE_ONCE(so->tx.state, new_state);
 	hrtimer_start(tx_hrt, ktime_set(hrtimer_sec, 0),
 		      HRTIMER_MODE_REL_SOFT);
 	spin_unlock_bh(&so->rx_lock);
@@ -1223,20 +1321,49 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			       __func__, ERR_PTR(err));
 
 		spin_lock_bh(&so->rx_lock);
+
+		/* new transfer already claimed by a concurrent completion,
+		 * timeout or sendmsg() while we were stuck in can_send()?
+		 */
+		if (READ_ONCE(so->tx_gen) != my_gen) {
+			/* don't touch timers and state of the new transfer */
+			spin_unlock_bh(&so->rx_lock);
+			return err;
+		}
+
 		/* no transmission -> no timeout monitoring */
 		hrtimer_cancel(tx_hrt);
 		goto err_out_drop_locked;
 	}
 
 	if (wait_tx_done) {
-		/* wait for complete transmission of current pdu */
-		err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+		/* wake up for:
+		 * - concurrent sendmsg() claiming a new transfer
+		 * - complete transmission of current PDU
+		 * - shutdown state change in isotp_release()
+		 * isotp_tx_gen_done() uses common tx.state/tx_gen read sequence
+		 */
+		err = wait_event_interruptible(so->wait,
+					       isotp_tx_gen_done(so, my_gen));
 		if (err)
 			goto err_event_drop;
 
-		err = sock_error(sk);
-		if (err)
-			return err;
+		/* still our claim, but isotp_release() force-shut it down */
+		if (smp_load_acquire(&so->tx.state) == ISOTP_SHUTDOWN &&
+		    READ_ONCE(so->tx_gen) == my_gen) {
+			err = -EADDRNOTAVAIL;
+			goto err_event_drop;
+		}
+
+		/* own completion, or tx_gen moved on - either way this is
+		 * what isotp_get_tx_result() recorded for my_gen
+		 */
+		err = isotp_get_tx_result(so, my_gen);
+
+		/* drain to avoid stale error for a later poll()/SO_ERROR */
+		sock_error(sk);
+
+		return err ? err : size;
 	}
 
 	return size;
@@ -1246,15 +1373,26 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	spin_lock_bh(&so->rx_lock);
 	goto err_out_drop_locked;
 err_event_drop:
-	/* interrupted waiting on our own transfer - drain its timers */
+	/* interrupted or shut down while waiting on our own transfer */
 	spin_lock_bh(&so->rx_lock);
+
+	/* new transfer already started by concurrent sendmsg()? */
+	if (READ_ONCE(so->tx_gen) != my_gen) {
+		/* don't touch timers and states of the new transfer */
+		spin_unlock_bh(&so->rx_lock);
+		return err;
+	}
+
 	hrtimer_cancel(&so->txfrtimer);
 	hrtimer_cancel(&so->txtimer);
 	hrtimer_cancel(&so->echotimer);
 err_out_drop_locked:
 	/* release the claim; so->rx_lock still held from above */
-	so->cfecho = 0;
-	so->tx.state = ISOTP_IDLE;
+	WRITE_ONCE(so->cfecho, 0);
+
+	/* only claim to IDLE if isotp_release() has not taken over */
+	if (READ_ONCE(so->tx.state) != ISOTP_SHUTDOWN)
+		WRITE_ONCE(so->tx.state, ISOTP_IDLE);
 	spin_unlock_bh(&so->rx_lock);
 	wake_up_interruptible(&so->wait);
 
@@ -1320,8 +1458,9 @@ static int isotp_release(struct socket *sock)
 	/* best-effort: wait for a running pdu to finish, but don't block on
 	 * it forever - give up after the first signal
 	 */
-	while (so->tx.state != ISOTP_IDLE &&
-	       wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE) == 0)
+	while (READ_ONCE(so->tx.state) != ISOTP_IDLE &&
+	       wait_event_interruptible(so->wait,
+					READ_ONCE(so->tx.state) == ISOTP_IDLE) == 0)
 		;
 
 	/* claim the socket under so->rx_lock like sendmsg() does, so its
@@ -1329,9 +1468,12 @@ static int isotp_release(struct socket *sock)
 	 * unconditionally, even when a signal cut the wait above short
 	 */
 	spin_lock_bh(&so->rx_lock);
-	so->tx.state = ISOTP_SHUTDOWN;
+	WRITE_ONCE(so->tx.state, ISOTP_SHUTDOWN);
 	spin_unlock_bh(&so->rx_lock);
-	so->rx.state = ISOTP_IDLE;
+	WRITE_ONCE(so->rx.state, ISOTP_IDLE);
+
+	/* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */
+	wake_up_interruptible(&so->wait);
 
 	spin_lock(&isotp_notifier_lock);
 	while (isotp_busy_notifier == so) {
@@ -1447,7 +1589,8 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
 	 * with so->bound in the same lock_sock() section above, so there is
 	 * no window in which a concurrent isotp_notify() could be missed.
 	 */
-	if (so->tx.state != ISOTP_IDLE || so->rx.state != ISOTP_IDLE) {
+	if (READ_ONCE(so->tx.state) != ISOTP_IDLE ||
+	    READ_ONCE(so->rx.state) != ISOTP_IDLE) {
 		err = -EAGAIN;
 		goto out;
 	}
@@ -1481,7 +1624,7 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
 				isotp_rcv, sk, "isotp", sk);
 
 	/* no consecutive frame echo skb in flight */
-	so->cfecho = 0;
+	WRITE_ONCE(so->cfecho, 0);
 
 	/* register for echo skb's */
 	can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id),
@@ -1847,7 +1990,7 @@ static __poll_t isotp_poll(struct file *file, struct socket *sock, poll_table *w
 	poll_wait(file, &so->wait, wait);
 
 	/* Check for false positives due to TX state */
-	if ((mask & EPOLLWRNORM) && (so->tx.state != ISOTP_IDLE))
+	if ((mask & EPOLLWRNORM) && (READ_ONCE(so->tx.state) != ISOTP_IDLE))
 		mask &= ~(EPOLLOUT | EPOLLWRNORM);
 
 	return mask;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 04/20] can: isotp: check register_netdevice_notifier() error in module init
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 05/20] can: ctucanfd: unmap BAR0 using base address Marc Kleine-Budde
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Minhong He, Marc Kleine-Budde

From: Minhong He <heminhong@kylinos.cn>

Register the netdevice notifier before can_proto_register() and check the
return value. If protocol registration fails, unregister the notifier
before returning the error.

Align isotp_module_init() with the reordering already done for raw.c
(commit c28b3bffe49e ("can: raw: process optimization in raw_init()")) and
bcm.c (commit edd1a7e42f1d ("can: bcm: registration process optimization
in bcm_module_init()")).

Fixes: 8d0caedb7596 ("can: bcm/raw/isotp: use per module netdevice notifier")
Signed-off-by: Minhong He <heminhong@kylinos.cn>
Link: https://patch.msgid.link/20260729085656.134523-1-heminhong@kylinos.cn
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 1f11c66b343c..155530aedce2 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -2050,13 +2050,18 @@ static __init int isotp_module_init(void)
 
 	pr_info("can: isotp protocol (max_pdu_size %d)\n", max_pdu_size);
 
-	err = can_proto_register(&isotp_can_proto);
-	if (err < 0)
-		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
-	else
-		register_netdevice_notifier(&canisotp_notifier);
+	err = register_netdevice_notifier(&canisotp_notifier);
+	if (err)
+		return err;
 
-	return err;
+	err = can_proto_register(&isotp_can_proto);
+	if (err < 0) {
+		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
+		unregister_netdevice_notifier(&canisotp_notifier);
+		return err;
+	}
+
+	return 0;
 }
 
 static __exit void isotp_module_exit(void)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 05/20] can: ctucanfd: unmap BAR0 using base address
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 04/20] can: isotp: check register_netdevice_notifier() error in module init Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid Marc Kleine-Budde
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Avi Weiss, Pavel Pisa, stable,
	Marc Kleine-Budde

From: Avi Weiss <thnkslprpt@gmail.com>

BAR0 is mapped into bar0_base, while cra_addr points to an offset
within that mapping and is used for other purposes.

Pass bar0_base to pci_iounmap(), instead of cra_addr, on the probe error
path so the address returned by pci_iomap() is used for unmapping.

Fixes: 792a5b678e81 ("can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Acked-by: Pavel Pisa <pisa@fel.cvut.cz>
Link: https://patch.msgid.link/20260723095934.181042-1-thnkslprpt@gmail.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c
index 625788fa8976..f845951c5d51 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_pci.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c
@@ -194,7 +194,7 @@ static int ctucan_pci_probe(struct pci_dev *pdev,
 	pci_set_drvdata(pdev, NULL);
 	kfree(bdata);
 err_pci_iounmap_bar0:
-	pci_iounmap(pdev, cra_addr);
+	pci_iounmap(pdev, bar0_base);
 err_pci_iounmap_bar1:
 	pci_iounmap(pdev, addr);
 err_release_regions:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 05/20] can: ctucanfd: unmap BAR0 using base address Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 07/20] can: ctucanfd: handle bus error interrupts Marc Kleine-Budde
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Avi Weiss, stable,
	Marc Kleine-Budde

From: Avi Weiss <thnkslprpt@gmail.com>

In the CAN_STATE_ERROR_ACTIVE case, cf->data[1] is set to
CAN_ERR_CRTL_ACTIVE, but cf->can_id is not set with CAN_ERR_CRTL in
that path.

Set CAN_ERR_CRTL so consumers know the controller-status information
in cf->data[1] is valid.

Fixes: 9bd24927e3ee ("can: ctucanfd: handle skb allocation failure")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Link: https://patch.msgid.link/20260723155543.318414-1-thnkslprpt@gmail.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 0ea1ff28dfce..8f8b1c097ec6 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -869,7 +869,7 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)
 			break;
 		case CAN_STATE_ERROR_ACTIVE:
 			if (skb) {
-				cf->can_id |= CAN_ERR_CNT;
+				cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
 				cf->data[1] = CAN_ERR_CRTL_ACTIVE;
 				cf->data[6] = bec.txerr;
 				cf->data[7] = bec.rxerr;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 07/20] can: ctucanfd: handle bus error interrupts
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (5 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK Marc Kleine-Budde
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Avi Weiss, Pavel Pisa, stable,
	Marc Kleine-Budde

From: Avi Weiss <thnkslprpt@gmail.com>

Include REG_INT_STAT_BEI in the top-level error interrupt condition.

BEI is enabled when CAN_CTRLMODE_BERR_REPORTING is requested and
ctucan_err_interrupt() already handles it. Without checking and
clearing BEI in the top-level handler, bus error interrupts are not
handled or acknowledged.

Fixes: 2dcb8e8782d8 ("can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Acked-by: Pavel Pisa <pisa@fel.cvut.cz>
Link: https://patch.msgid.link/20260723074403.131575-1-thnkslprpt@gmail.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 8f8b1c097ec6..10ebcc13ea65 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -1136,8 +1136,12 @@ static irqreturn_t ctucan_interrupt(int irq, void *dev_id)
 		/* Error interrupts */
 		if (FIELD_GET(REG_INT_STAT_EWLI, isr) ||
 		    FIELD_GET(REG_INT_STAT_FCSI, isr) ||
-		    FIELD_GET(REG_INT_STAT_ALI, isr)) {
-			icr = isr & (REG_INT_STAT_EWLI | REG_INT_STAT_FCSI | REG_INT_STAT_ALI);
+		    FIELD_GET(REG_INT_STAT_ALI, isr) ||
+		    FIELD_GET(REG_INT_STAT_BEI, isr)) {
+			icr = isr & (REG_INT_STAT_EWLI |
+				     REG_INT_STAT_FCSI |
+				     REG_INT_STAT_ALI |
+				     REG_INT_STAT_BEI);
 
 			ctucan_netdev_dbg(ndev, "some ERR interrupt: clearing 0x%08x\n", icr);
 			ctucan_write32(priv, CTUCANFD_INT_STAT, icr);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (6 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 07/20] can: ctucanfd: handle bus error interrupts Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 09/20] can: ctucanfd: add missing MODULE_DEVICE_TABLE() Marc Kleine-Budde
                   ` (11 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Avi Weiss, Pavel Pisa, stable,
	Marc Kleine-Budde

From: Avi Weiss <thnkslprpt@gmail.com>

Use self-test mode for CAN_CTRLMODE_PRESUME_ACK so transmitted
frames can complete without receiving an ACK.

ACK forbidden mode prevents the controller from acknowledging
received frames and does not implement the presume-ack behavior.

Fixes: 2dcb8e8782d8 ("can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Acked-by: Pavel Pisa <pisa@fel.cvut.cz>
Link: https://patch.msgid.link/20260722192726.230729-1-thnkslprpt@gmail.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 10ebcc13ea65..07d4aa43c700 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -340,8 +340,8 @@ static void ctucan_set_mode(struct ctucan_priv *priv, const struct can_ctrlmode
 			(mode_reg & ~REG_MODE_FDE);
 
 	mode_reg = (mode->flags & CAN_CTRLMODE_PRESUME_ACK) ?
-			(mode_reg | REG_MODE_ACF) :
-			(mode_reg & ~REG_MODE_ACF);
+			(mode_reg | REG_MODE_STM) :
+			(mode_reg & ~REG_MODE_STM);
 
 	mode_reg = (mode->flags & CAN_CTRLMODE_FD_NON_ISO) ?
 			(mode_reg | REG_MODE_NISOFD) :
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 09/20] can: ctucanfd: add missing MODULE_DEVICE_TABLE()
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (7 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index Marc Kleine-Budde
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Pengpeng Hou, Pavel Pisa, stable,
	Marc Kleine-Budde

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

The driver has a match table for the pci bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().

Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.

This is a source-level fix.  It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Acked-by: Pavel Pisa <pisa@fel.cvut.cz>
Link: https://patch.msgid.link/20260704151957.48194-1-pengpeng@iscas.ac.cn
Fixes: 792a5b678e81 ("can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.")
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ctucanfd/ctucanfd_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c
index f845951c5d51..4b6db28f7b67 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_pci.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c
@@ -266,6 +266,7 @@ static const struct pci_device_id ctucan_pci_tbl[] = {
 		CTUCAN_WITH_CTUCAN_ID)},
 	{},
 };
+MODULE_DEVICE_TABLE(pci, ctucan_pci_tbl);
 
 static struct pci_driver ctucan_pci_driver = {
 	.name = KBUILD_MODNAME,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (8 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 09/20] can: ctucanfd: add missing MODULE_DEVICE_TABLE() Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error Marc Kleine-Budde
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, James Gao, Vincent Mailhol,
	stable, Marc Kleine-Budde

From: James Gao <jamesgao5@outlook.com>

The channel control index ctrl_idx is derived from rx->len which comes
directly from a device USB payload. The mask 0x0f allows values 0-15, but
the array size of usb_if->dev[] is only 2. Values 2-15 cause heap
out-of-bounds read, eventually causing kernel panic in the IRQ context.

Add bounds checking for ctrl_idx before the array access in both
pcan_usb_pro_handle_canmsg() and pcan_usb_pro_handle_error().

Fixes: d8a199355f8f ("can: usb: PEAK-System Technik PCAN-USB Pro specific part")
Signed-off-by: James Gao <jamesgao5@outlook.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Link: https://patch.msgid.link/TYWPR01MB8559DBAAAA6A7F410400329CF0012@TYWPR01MB8559.jpnprd01.prod.outlook.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index aefcded8e12a..b6be8c19e537 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -534,12 +534,18 @@ static int pcan_usb_pro_handle_canmsg(struct pcan_usb_pro_interface *usb_if,
 				      struct pcan_usb_pro_rxmsg *rx)
 {
 	const unsigned int ctrl_idx = (rx->len >> 4) & 0x0f;
-	struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
-	struct net_device *netdev = dev->netdev;
+	struct peak_usb_device *dev;
+	struct net_device *netdev;
 	struct can_frame *can_frame;
 	struct sk_buff *skb;
 	struct skb_shared_hwtstamps *hwts;
 
+	if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
+		return -EINVAL;
+
+	dev = usb_if->dev[ctrl_idx];
+	netdev = dev->netdev;
+
 	skb = alloc_can_skb(netdev, &can_frame);
 	if (!skb)
 		return -ENOMEM;
@@ -573,14 +579,20 @@ static int pcan_usb_pro_handle_error(struct pcan_usb_pro_interface *usb_if,
 {
 	const u16 raw_status = le16_to_cpu(er->status);
 	const unsigned int ctrl_idx = (er->channel >> 4) & 0x0f;
-	struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
-	struct net_device *netdev = dev->netdev;
+	struct peak_usb_device *dev;
+	struct net_device *netdev;
 	struct can_frame *can_frame;
 	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
 	u8 err_mask = 0;
 	struct sk_buff *skb;
 	struct skb_shared_hwtstamps *hwts;
 
+	if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
+		return -EINVAL;
+
+	dev = usb_if->dev[ctrl_idx];
+	netdev = dev->netdev;
+
 	/* nothing should be sent while in BUS_OFF state */
 	if (dev->can.state == CAN_STATE_BUS_OFF)
 		return 0;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (9 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths Marc Kleine-Budde
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Maoyi Xie, stable,
	Vincent Mailhol, Marc Kleine-Budde

From: Maoyi Xie <maoyixie.tju@gmail.com>

In peak_usb_start(), each RX URB transfer buffer is allocated with kmalloc()
and the URB is flagged URB_FREE_BUFFER so that the final usb_free_urb() also
frees the transfer buffer.

If usb_submit_urb() fails, the error path frees the buffer explicitly with
kfree(buf) and then calls usb_free_urb(urb). Because URB_FREE_BUFFER is set,
usb_free_urb() -> urb_destroy() frees the same buffer a second time, a double
free of the transfer buffer.

  BUG: KASAN: double-free in usb_free_urb.part.0+0x91/0xb0
  Free of addr ffff8881069ccb80 by task trigger.sh/285

  Call Trace:
   kfree+0x113/0x3c0
   usb_free_urb.part.0+0x91/0xb0

Drop the redundant kfree(buf); usb_free_urb() already releases the transfer
buffer. This mirrors commit 03819abbeb11 ("net: usb: lan78xx: Fix double free
issue with interrupt buffer allocation").

Fixes: bb4785551f64 ("can: usb: PEAK-System Technik USB adapters driver core")
Closes: https://lore.kernel.org/linux-can/178159320216.2154888.16953451793788581739@maoyixie.com/T/#u
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Link: https://patch.msgid.link/178163373110.2507866.216458825145756798@maoyixie.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index c7933d1acc99..55aad01cd8ca 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -470,7 +470,6 @@ static int peak_usb_start(struct peak_usb_device *dev)
 				netif_device_detach(dev->netdev);
 
 			usb_unanchor_urb(urb);
-			kfree(buf);
 			usb_free_urb(urb);
 			break;
 		}
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (10 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 13/20] can: kvaser_usb: kvaser_usb_hydra_get_busparams(): fix memory leak in kvaser_usb_hydra_get_busparams() Marc Kleine-Budde
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Pengpeng Hou, stable,
	Marc Kleine-Budde

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

pcan_usb_fd_decode_buf() walks uCAN records packed in one USB
receive buffer.

Require each record to contain the fixed header for its type, and verify
CAN payload bytes before copying them into the skb.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260706092836.79754-1-pengpeng@iscas.ac.cn
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 40 +++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index ef9fd693e9bd..0d46f4ce5dca 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -566,6 +566,13 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
 				     dev->can.ctrlmode);
 	}
 
+	if (!(rx_msg_flags & PUCAN_MSG_RTR) &&
+	    le16_to_cpu(rx_msg->size) - offsetof(struct pucan_rx_msg, d) <
+	    cfd->len) {
+		kfree_skb(skb);
+		return -EBADMSG;
+	}
+
 	cfd->can_id = le32_to_cpu(rm->can_id);
 
 	if (rx_msg_flags & PUCAN_MSG_EXT_ID)
@@ -714,6 +721,24 @@ static void pcan_usb_fd_decode_ts(struct pcan_usb_fd_if *usb_if,
 		peak_usb_set_ts_now(&usb_if->time_ref, le32_to_cpu(ts->ts_low));
 }
 
+static size_t pcan_usb_fd_rx_msg_min_size(u16 rx_msg_type)
+{
+	switch (rx_msg_type) {
+	case PUCAN_MSG_CAN_RX:
+		return offsetof(struct pucan_rx_msg, d);
+	case PCAN_UFD_MSG_CALIBRATION:
+		return sizeof(struct pcan_ufd_ts_msg);
+	case PUCAN_MSG_ERROR:
+		return sizeof(struct pucan_error_msg);
+	case PUCAN_MSG_STATUS:
+		return sizeof(struct pucan_status_msg);
+	case PCAN_UFD_MSG_OVERRUN:
+		return sizeof(struct pcan_ufd_ovr_msg);
+	default:
+		return sizeof(struct pucan_msg);
+	}
+}
+
 /* callback for bulk IN urb */
 static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 {
@@ -728,6 +753,12 @@ static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 	msg_end = urb->transfer_buffer + urb->actual_length;
 	for (; msg_ptr < msg_end;) {
 		u16 rx_msg_type, rx_msg_size;
+		size_t rx_msg_min_size;
+
+		if (msg_end - msg_ptr < sizeof(*rx_msg)) {
+			err = -EBADMSG;
+			break;
+		}
 
 		rx_msg = (struct pucan_msg *)msg_ptr;
 		if (!rx_msg->size) {
@@ -739,13 +770,20 @@ static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
 		rx_msg_type = le16_to_cpu(rx_msg->type);
 
 		/* check if the record goes out of current packet */
-		if (msg_ptr + rx_msg_size > msg_end) {
+		if (rx_msg_size > msg_end - msg_ptr) {
 			netdev_err(netdev,
 				   "got frag rec: should inc usb rx buf sze\n");
 			err = -EBADMSG;
 			break;
 		}
 
+		rx_msg_min_size = pcan_usb_fd_rx_msg_min_size(rx_msg_type);
+		if (rx_msg_size < rx_msg_min_size) {
+			netdev_err(netdev, "got short rec\n");
+			err = -EBADMSG;
+			break;
+		}
+
 		switch (rx_msg_type) {
 		case PUCAN_MSG_CAN_RX:
 			err = pcan_usb_fd_decode_canmsg(usb_if, rx_msg);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 13/20] can: kvaser_usb: kvaser_usb_hydra_get_busparams(): fix memory leak in kvaser_usb_hydra_get_busparams()
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (11 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents Marc Kleine-Budde
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Abdun Nihaal, stable,
	Marc Kleine-Budde

From: Abdun Nihaal <nihaal@cse.iitm.ac.in>

The memory allocated for cmd is not freed after the call to
kvaser_usb_send_cmd() in both the normal and error paths.
Fix that by adding a kfree() immediately after the call.

Fixes: 39d3df6b0ea8 ("can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming")
Cc: stable@vger.kernel.org
Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
Link: https://patch.msgid.link/20260722103906.108571-1-nihaal@cse.iitm.ac.in
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index e09d663e362f..efbb7bed34c9 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1626,6 +1626,7 @@ static int kvaser_usb_hydra_get_busparams(struct kvaser_usb_net_priv *priv,
 	reinit_completion(&priv->get_busparams_comp);
 
 	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
+	kfree(cmd);
 	if (err)
 		return err;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (12 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 13/20] can: kvaser_usb: kvaser_usb_hydra_get_busparams(): fix memory leak in kvaser_usb_hydra_get_busparams() Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets Marc Kleine-Budde
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Pengpeng Hou, stable,
	Marc Kleine-Budde

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

The wait and bulk receive paths walk variable-length commands from a
USB buffer. A nonzero command shorter than CMD_HEADER_LEN can still be
dispatched, and the wait path copies a matching command into a fixed
caller-owned struct kvaser_cmd using the device-provided length.

Reject nonzero commands that do not contain the fixed header or that
extend beyond the current USB buffer item. In the wait path, also reject
a matching command that exceeds the destination before copying it.

Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260722042221.44066-1-pengpeng@iscas.ac.cn
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
index df737cfc5ea0..a876c7819b81 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -691,13 +691,22 @@ static int kvaser_usb_leaf_wait_cmd(const struct kvaser_usb *dev, u8 id,
 				continue;
 			}
 
-			if (pos + tmp->len > actual_len) {
+			if (tmp->len < CMD_HEADER_LEN ||
+			    tmp->len > actual_len - pos) {
 				dev_err_ratelimited(&dev->intf->dev,
 						    "Format error\n");
 				break;
 			}
 
 			if (tmp->id == id) {
+				if (tmp->len > sizeof(*cmd)) {
+					dev_err_ratelimited(&dev->intf->dev,
+							    "Received command %u too large (%u)\n",
+							    tmp->id, tmp->len);
+					err = -EIO;
+					goto end;
+				}
+
 				memcpy(cmd, tmp, tmp->len);
 				goto end;
 			}
@@ -1737,7 +1746,7 @@ static void kvaser_usb_leaf_read_bulk_callback(struct kvaser_usb *dev,
 			continue;
 		}
 
-		if (pos + cmd->len > len) {
+		if (cmd->len < CMD_HEADER_LEN || cmd->len > len - pos) {
 			dev_err_ratelimited(&dev->intf->dev, "Format error\n");
 			break;
 		}
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (13 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans Marc Kleine-Budde
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Tu Nguyen, Biju Das,
	Claudiu Beznea, Geert Uytterhoeven, Vincent Mailhol, stable,
	Marc Kleine-Budde

From: Tu Nguyen <tu.nguyen.xg@renesas.com>

Testing CANFD on RZ/G3E shows that many registers do not reset to their
initial values with the current flow of deasserting resets first and then
enabling clocks.

Based on the HW manual, clocks should be supplied first and the
resets deasserted afterward.

 section 7.4.3 Procedure for Activating Modules: RZ/G2L
 section 4.4.9.3 Procedure for Starting up Units: RZ/G3E

So, update the order of the initializing flow for resets and clocks
to match the hardware manual, resetting all CANFD registers to their
initial values. Also update rcar_canfd_global_deinit() to assert
resets before disabling clocks, so the teardown path mirrors the new
init ordering.

Fixes: 76e9353a80e9 ("can: rcar_canfd: Add support for RZ/G2L family")
Signed-off-by: Tu Nguyen <tu.nguyen.xg@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Link: https://patch.msgid.link/20260625135216.130450-1-biju.das.jz@bp.renesas.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 32 +++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index eaf8cac78038..fcc37b73ed43 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -2003,20 +2003,12 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
 	u32 ch, sts;
 	int err;
 
-	err = reset_control_reset(gpriv->rstc1);
-	if (err)
-		return err;
-
-	err = reset_control_reset(gpriv->rstc2);
-	if (err)
-		goto fail_reset1;
-
 	/* Enable peripheral clock for register access */
 	err = clk_prepare_enable(gpriv->clkp);
 	if (err) {
 		dev_err(dev, "failed to enable peripheral clock: %pe\n",
 			ERR_PTR(err));
-		goto fail_reset2;
+		return err;
 	}
 
 	/* Enable RAM clock */
@@ -2027,10 +2019,18 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
 		goto fail_clk;
 	}
 
+	err = reset_control_reset(gpriv->rstc1);
+	if (err)
+		goto fail_ram_clk;
+
+	err = reset_control_reset(gpriv->rstc2);
+	if (err)
+		goto fail_reset1;
+
 	err = rcar_canfd_reset_controller(gpriv);
 	if (err) {
 		dev_err(dev, "reset controller failed: %pe\n", ERR_PTR(err));
-		goto fail_ram_clk;
+		goto fail_reset2;
 	}
 
 	/* Controller in Global reset & Channel reset mode */
@@ -2068,14 +2068,14 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
 
 fail_mode:
 	rcar_canfd_disable_global_interrupts(gpriv);
-fail_ram_clk:
-	clk_disable_unprepare(gpriv->clk_ram);
-fail_clk:
-	clk_disable_unprepare(gpriv->clkp);
 fail_reset2:
 	reset_control_assert(gpriv->rstc2);
 fail_reset1:
 	reset_control_assert(gpriv->rstc1);
+fail_ram_clk:
+	clk_disable_unprepare(gpriv->clk_ram);
+fail_clk:
+	clk_disable_unprepare(gpriv->clkp);
 	return err;
 }
 
@@ -2090,10 +2090,10 @@ static void rcar_canfd_global_deinit(struct rcar_canfd_global *gpriv, bool full)
 		rcar_canfd_set_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GSLPR);
 	}
 
-	clk_disable_unprepare(gpriv->clk_ram);
-	clk_disable_unprepare(gpriv->clkp);
 	reset_control_assert(gpriv->rstc2);
 	reset_control_assert(gpriv->rstc1);
+	clk_disable_unprepare(gpriv->clk_ram);
+	clk_disable_unprepare(gpriv->clkp);
 }
 
 static int rcar_canfd_probe(struct platform_device *pdev)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (14 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:29   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured Marc Kleine-Budde
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Pengpeng Hou, stable,
	Marc Kleine-Budde

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

fw_parse() reads a fixed record header, a firmware-provided payload,
and a trailing checksum without knowing the end of the firmware blob. A
truncated record can therefore make those reads exceed the blob.

The same record also supplies addresses and lengths for writes into
DPRAM. The generic loader uses wrap-prone mixed signed arithmetic for its
bounds check, while the application loader does not bound the staging
copy at all.

Pass the firmware end to the parser and validate the full source record.
Use a signed wide offset for generic DPRAM records and validate the
application staging span against the mapped DPRAM before copying.

Fixes: 03fd3cf5a179 ("can: add driver for Softing card")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260722044347.2708-1-pengpeng@iscas.ac.cn
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/softing/softing_fw.c | 46 +++++++++++++++++++---------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
index 721df91cdbfb..282570daf3ef 100644
--- a/drivers/net/can/softing/softing_fw.c
+++ b/drivers/net/can/softing/softing_fw.c
@@ -91,12 +91,12 @@ int softing_bootloader_command(struct softing *card, int16_t cmd,
 	return ret;
 }
 
-static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
-		uint16_t *plen, const uint8_t **pdat)
+static int fw_parse(const u8 **pmem, const u8 *limit, u16 *ptype,
+		    u32 *paddr, u16 *plen, const u8 **pdat)
 {
 	uint16_t checksum[2];
-	const uint8_t *mem;
-	const uint8_t *end;
+	const u8 *mem;
+	const u8 *record_end;
 
 	/*
 	 * firmware records are a binary, unaligned stream composed of:
@@ -114,14 +114,21 @@ static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
 	 * endianness & alignment.
 	 */
 	mem = *pmem;
+	/* A record needs an 8-byte prefix and a 2-byte checksum. */
+	if (mem > limit || limit - mem < 10)
+		return -EINVAL;
+
 	*ptype = le16_to_cpup((void *)&mem[0]);
 	*paddr = le32_to_cpup((void *)&mem[2]);
 	*plen = le16_to_cpup((void *)&mem[6]);
+	if (*plen > limit - mem - 10)
+		return -EINVAL;
+
 	*pdat = &mem[8];
 	/* verify checksum */
-	end = &mem[8 + *plen];
-	checksum[0] = le16_to_cpup((void *)end);
-	for (checksum[1] = 0; mem < end; ++mem)
+	record_end = &mem[8 + *plen];
+	checksum[0] = le16_to_cpup((void *)record_end);
+	for (checksum[1] = 0; mem < record_end; ++mem)
 		checksum[1] += *mem;
 	if (checksum[0] != checksum[1])
 		return -EINVAL;
@@ -139,6 +146,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	uint16_t type, len;
 	uint32_t addr;
 	uint8_t *buf = NULL, *new_buf;
+	s64 dpram_offset;
 	int buflen = 0;
 	int8_t type_end = 0;
 
@@ -153,7 +161,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	mem = fw->data;
 	end = &mem[fw->size];
 	/* look for header record */
-	ret = fw_parse(&mem, &type, &addr, &len, &dat);
+	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 	if (ret < 0)
 		goto failed;
 	if (type != 0xffff)
@@ -164,7 +172,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	}
 	/* ok, we had a header */
 	while (mem < end) {
-		ret = fw_parse(&mem, &type, &addr, &len, &dat);
+		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 		if (ret < 0)
 			goto failed;
 		if (type == 3) {
@@ -179,9 +187,13 @@ int softing_load_fw(const char *file, struct softing *card,
 			goto failed;
 		}
 
-		if ((addr + len + offset) > size)
+		dpram_offset = (s64)addr + offset;
+		if (dpram_offset < 0 || dpram_offset > size ||
+		    len > size - dpram_offset) {
+			ret = -EINVAL;
 			goto failed;
-		memcpy_toio(&dpram[addr + offset], dat, len);
+		}
+		memcpy_toio(&dpram[dpram_offset], dat, len);
 		/* be sure to flush caches from IO space */
 		mb();
 		if (len > buflen) {
@@ -195,7 +207,7 @@ int softing_load_fw(const char *file, struct softing *card,
 			buf = new_buf;
 		}
 		/* verify record data */
-		memcpy_fromio(buf, &dpram[addr + offset], len);
+		memcpy_fromio(buf, &dpram[dpram_offset], len);
 		if (memcmp(buf, dat, len)) {
 			/* is not ok */
 			dev_alert(&card->pdev->dev, "DPRAM readback failed\n");
@@ -237,7 +249,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
 	mem = fw->data;
 	end = &mem[fw->size];
 	/* look for header record */
-	ret = fw_parse(&mem, &type, &addr, &len, &dat);
+	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 	if (ret)
 		goto failed;
 	ret = -EINVAL;
@@ -253,7 +265,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
 	}
 	/* ok, we had a header */
 	while (mem < end) {
-		ret = fw_parse(&mem, &type, &addr, &len, &dat);
+		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 		if (ret)
 			goto failed;
 
@@ -279,6 +291,12 @@ int softing_load_app_fw(const char *file, struct softing *card)
 		/* work in 16bit (target) */
 		sum &= 0xffff;
 
+		if (card->pdat->app.offs > card->dpram_size ||
+		    len > card->dpram_size - card->pdat->app.offs) {
+			ret = -EINVAL;
+			goto failed;
+		}
+
 		memcpy_toio(&card->dpram[card->pdat->app.offs], dat, len);
 		iowrite32(card->pdat->app.offs + card->pdat->app.addr,
 				&card->dpram[DPRAM_COMMAND + 2]);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (15 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-30 10:30   ` sashiko-bot
  2026-07-29 10:12 ` [PATCH net 18/20] can: gs_usb: gs_usb_receive_bulk_callback(): resubmit URB on skb allocation failure Marc Kleine-Budde
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Lucas Martins Alves, stable,
	Marc Kleine-Budde

From: Lucas Martins Alves <lucas.alves@lumal21.com.br>

c_can_chip_config() was programming C_CAN_CTRL_REG without CONTROL_INIT,
which may allow the controller to become active before
c_can_set_bittiming() finishes.

That creates a short timing window where the peripheral can interact with
the bus using a different/default bitrate, potentially generating bus
errors and corrupting traffic.

Set CONTROL_INIT together with the control-mode writes in
c_can_chip_config() (normal, loopback and listen-only paths), so the
controller stays halted until bit timing is fully programmed.

This prevents transient bus disturbance during startup when the configured
bitrate differs from the active bus bitrate.

Signed-off-by: Lucas Martins Alves <lucas.alves@lumal21.com.br>
Link: https://patch.msgid.link/20260714164839.771123-1-lucas.alves@lumal21.com.br
Fixes: 881ff67ad450 ("can: c_can: Added support for Bosch C_CAN controller")
Cc: stable@kernel.org
[mkl: remove space before close parenthesis]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/c_can/c_can_main.c b/drivers/net/can/c_can/c_can_main.c
index 3702cac7fbf0..b3b321d9ce68 100644
--- a/drivers/net/can/c_can/c_can_main.c
+++ b/drivers/net/can/c_can/c_can_main.c
@@ -597,20 +597,20 @@ static int c_can_chip_config(struct net_device *dev)
 		return err;
 
 	/* enable automatic retransmission */
-	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
+	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR | CONTROL_INIT);
 
 	if ((priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) &&
 	    (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)) {
 		/* loopback + silent mode : useful for hot self-test */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
 		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK | TEST_SILENT);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
 		/* loopback mode : useful for self-test function */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
 		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		/* silent mode : bus-monitoring mode */
-		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
 		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
 	}
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 18/20] can: gs_usb: gs_usb_receive_bulk_callback(): resubmit URB on skb allocation failure
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (16 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:12 ` [PATCH net 19/20] can: etas_es58x: es58x_read_bulk_callback(): fix RX buffer leak on URB resubmit failure Marc Kleine-Budde
  2026-07-29 10:13 ` [PATCH net 20/20] can: ems_usb: validate CPC message lengths Marc Kleine-Budde
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, stable

If the allocation of the SKB in gs_usb_receive_bulk_callback() fails, the
driver returns from the callback without resubmitting the URB in order to
receive further USB in URBs.

This results in a silent performance degradation which, if it occurs
repeatedly, results in starvation of USB in traffic.

Instead of returning immediately, try to resend the URB. If this also
fails, this is logged as an info message.

Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
Fixes: 26949ac935e3 ("can: gs_usb: add CAN-FD support")
Link: https://patch.msgid.link/20260709-gs_usb-resubmit-urb-v1-1-4dd40030cc84@pengutronix.de
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index ec9a7cbbbc69..82508a865095 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -674,7 +674,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		if (hf->flags & GS_CAN_FLAG_FD) {
 			skb = alloc_canfd_skb(netdev, &cfd);
 			if (!skb)
-				return;
+				goto resubmit_urb;
 
 			cfd->can_id = le32_to_cpu(hf->can_id);
 			cfd->len = data_length;
@@ -687,7 +687,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		} else {
 			skb = alloc_can_skb(netdev, &cf);
 			if (!skb)
-				return;
+				goto resubmit_urb;
 
 			cf->can_id = le32_to_cpu(hf->can_id);
 			can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 19/20] can: etas_es58x: es58x_read_bulk_callback(): fix RX buffer leak on URB resubmit failure
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (17 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 18/20] can: gs_usb: gs_usb_receive_bulk_callback(): resubmit URB on skb allocation failure Marc Kleine-Budde
@ 2026-07-29 10:12 ` Marc Kleine-Budde
  2026-07-29 10:13 ` [PATCH net 20/20] can: ems_usb: validate CPC message lengths Marc Kleine-Budde
  19 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:12 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Guangshuo Li, Vincent Mailhol,
	stable, Marc Kleine-Budde

From: Guangshuo Li <lgs201920130244@gmail.com>

es58x_read_bulk_callback() resubmits the RX URB after processing a received
packet. If the resubmit succeeds, the URB remains anchored and will be
handled by the normal RX path or by teardown.

However, if usb_submit_urb() fails, the callback unanchors the URB and then
returns directly. This skips the existing free_urb path, so the coherent
transfer buffer allocated with usb_alloc_coherent() is not released.

Reuse the existing free_urb path after a resubmit failure so that the RX
coherent buffer is freed before leaving the callback.

Fixes: 5eaad4f76826 ("can: usb: etas_es58x: correctly anchor the urb in the read bulk callback")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Link: https://patch.msgid.link/20260706014601.415445-1-lgs201920130244@gmail.com
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/etas_es58x/es58x_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.c b/drivers/net/can/usb/etas_es58x/es58x_core.c
index b259f6109808..e1724ae79c5a 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_core.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
@@ -1476,7 +1476,6 @@ static void es58x_read_bulk_callback(struct urb *urb)
 		dev_err_ratelimited(dev,
 				    "Failed resubmitting read bulk urb: %pe\n",
 				    ERR_PTR(ret));
-	return;
 
  free_urb:
 	usb_free_coherent(urb->dev, urb->transfer_buffer_length,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH net 20/20] can: ems_usb: validate CPC message lengths
  2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
                   ` (18 preceding siblings ...)
  2026-07-29 10:12 ` [PATCH net 19/20] can: etas_es58x: es58x_read_bulk_callback(): fix RX buffer leak on URB resubmit failure Marc Kleine-Budde
@ 2026-07-29 10:13 ` Marc Kleine-Budde
  2026-07-30 10:30   ` sashiko-bot
  19 siblings, 1 reply; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-29 10:13 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Pengpeng Hou, stable,
	Marc Kleine-Budde

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

ems_usb_read_bulk_callback() walks CPC messages packed in one USB
receive buffer.

Check that each declared message fits in the URB payload. Also require the
type-specific payload to cover the fields used by the CAN, state, error and
overrun handlers.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260706092752.79600-1-pengpeng@iscas.ac.cn
Fixes: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface")
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/ems_usb.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 9b25dda7c183..24cf8f651f8f 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -409,6 +409,40 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
 		netif_rx(skb);
 }
 
+static bool ems_usb_rx_msg_len_valid(struct ems_cpc_msg *msg)
+{
+	size_t len = msg->length;
+	size_t can_len;
+
+	switch (msg->type) {
+	case CPC_MSG_TYPE_CAN_STATE:
+		return len >= sizeof(msg->msg.can_state);
+
+	case CPC_MSG_TYPE_CAN_FRAME:
+	case CPC_MSG_TYPE_EXT_CAN_FRAME:
+	case CPC_MSG_TYPE_RTR_FRAME:
+	case CPC_MSG_TYPE_EXT_RTR_FRAME:
+		if (len < CPC_CAN_MSG_MIN_SIZE)
+			return false;
+
+		if (msg->type == CPC_MSG_TYPE_RTR_FRAME ||
+		    msg->type == CPC_MSG_TYPE_EXT_RTR_FRAME)
+			return true;
+
+		can_len = can_cc_dlc2len(msg->msg.can_msg.length & 0xf);
+		return len >= CPC_CAN_MSG_MIN_SIZE + can_len;
+
+	case CPC_MSG_TYPE_CAN_FRAME_ERROR:
+		return len >= sizeof(msg->msg.error);
+
+	case CPC_MSG_TYPE_OVERRUN:
+		return len >= sizeof(msg->msg.overrun);
+
+	default:
+		return true;
+	}
+}
+
 /*
  * callback for bulk IN urb
  */
@@ -451,6 +485,15 @@ static void ems_usb_read_bulk_callback(struct urb *urb)
 			}
 
 			msg = (struct ems_cpc_msg *)&ibuf[start];
+			if (msg->length >
+			    urb->actual_length - start - CPC_MSG_HEADER_LEN) {
+				netdev_err(netdev, "format error\n");
+				break;
+			}
+			if (!ems_usb_rx_msg_len_valid(msg)) {
+				netdev_err(netdev, "format error\n");
+				break;
+			}
 
 			switch (msg->type) {
 			case CPC_MSG_TYPE_CAN_STATE:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking
  2026-07-29 10:12 ` [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: linux-can, Vincent Mailhol, Marc Kleine-Budde, Oliver Hartkopp,
	Oleksij Rempel

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

Pre-existing issues:
- [High] Teardown race in j1939_ecu_timer_handler causes j1939_priv and net_device leak.
- [High] __j1939_ecu_release() modifies the priv->ecus list via list_del(&ecu->list) without holding priv->lock, causing list corruption if the last reference is dropped concurrently.
--

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

can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking

To investigate a dev_hold()/dev_put() imbalance in j1939_priv management,
this commit adds netdevice_tracker support to j1939_{priv,session,ecu}.
This helps debug refcount leaks reported by syzbot when unregistering
vcan0 devices.

Link: https://patch.msgid.link/20260728055835.1151785-2-o.rempel@pengutronix.de

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

[Severity: High]
This is a pre-existing issue, but regarding the syzbot report about the
unregister_netdevice hang and the refcount leak mentioned above:

Could this leak be caused by a teardown race involving
j1939_ecu_timer_handler()?

When the interface is brought down, __j1939_rx_release() unmaps existing
ECUs via j1939_ecu_unmap_all(), but does it cancel the pending ac_timer
hrtimers?

If the timer fires after teardown, j1939_ecu_timer_handler() unconditionally
maps the ECU and takes a reference:

	j1939_ecu_map_locked(ecu);

Since the unmap phase has already passed, the ECU permanently holds a
reference to priv, which holds a reference to the net_device, preventing
unregister_netdevice from completing.

> 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 does __j1939_ecu_release() safely modify
priv->ecus without holding priv->lock here?

If j1939_ecu_put() is called from lockless contexts like j1939_ac_recv() or
j1939_ac_fixup(), and another thread drops the penultimate reference while
holding priv->lock, could the final reference be dropped in the lockless
context?

This would cause list_del() to modify the list concurrently with other
threads iterating or modifying it.

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

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering
  2026-07-29 10:12 ` [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  2026-07-30 12:28     ` Oliver Hartkopp
  0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Marc Kleine-Budde, Oliver Hartkopp, Oleksij Rempel,
	Vincent Mailhol, linux-can

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silently dropped asynchronous RX socket errors in isotp_sendmsg() due to ignored return value of sock_error(sk).
--

--- Patch [3]: [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering ---
commit 3e2ba28e6e9e34bb3ea9b3075735283b9c9490c0
Author: Oliver Hartkopp <socketcan@hartkopp.net>
can: isotp: fix timer drain order, wakeup handling and tx_gen ordering

This commit addresses several concurrent state and timer handling issues in
the ISOTP protocol. It introduces a generation counter and packed result
array to track transfer state accurately across lock-free accesses.

Link: https://patch.msgid.link/20260724181525.43556-1-socketcan@hartkopp.net

> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 54becaf6898f1..1f11c66b343c8 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
[ ... ]
> @@ -1223,20 +1321,49 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
[ ... ]
>  	if (wait_tx_done) {
> -		/* wait for complete transmission of current pdu */
> -		err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
> +		/* wake up for:
> +		 * - concurrent sendmsg() claiming a new transfer
> +		 * - complete transmission of current PDU
> +		 * - shutdown state change in isotp_release()
> +		 * isotp_tx_gen_done() uses common tx.state/tx_gen read sequence
> +		 */
> +		err = wait_event_interruptible(so->wait,
> +					       isotp_tx_gen_done(so, my_gen));
>  		if (err)
>  			goto err_event_drop;
>  
> -		err = sock_error(sk);
> -		if (err)
> -			return err;
> +		/* still our claim, but isotp_release() force-shut it down */
> +		if (smp_load_acquire(&so->tx.state) == ISOTP_SHUTDOWN &&
> +		    READ_ONCE(so->tx_gen) == my_gen) {
> +			err = -EADDRNOTAVAIL;
> +			goto err_event_drop;
> +		}
> +
> +		/* own completion, or tx_gen moved on - either way this is
> +		 * what isotp_get_tx_result() recorded for my_gen
> +		 */
> +		err = isotp_get_tx_result(so, my_gen);
> +
> +		/* drain to avoid stale error for a later poll()/SO_ERROR */
> +		sock_error(sk);

[Severity: High]
Does this unconditionally clear and discard concurrent asynchronous socket
errors? 

By ignoring the return value of sock_error() here in isotp_sendmsg(), it
appears any unrelated RX errors (such as ETIMEDOUT set by
isotp_rx_timer_handler()) would be permanently lost without being reported
to the user.

Could this cause applications to hang in recvmsg() or poll() indefinitely
because they never see the RX connection failure?

> +
> +		return err ? err : size;
>  	}
>  
>  	return size;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=3

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid
  2026-07-29 10:12 ` [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  2026-07-30 11:14     ` Marc Kleine-Budde
  0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Marc Kleine-Budde, Oliver Hartkopp, linux-can, Oleksij Rempel,
	Vincent Mailhol

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

Pre-existing issues:
- [High] The `REG_INT_STAT_BEI` (Bus Error) interrupt is enabled when `CAN_CTRLMODE_BERR_REPORTING` is requested, but it is never checked or cleared in the main `ctucan_interrupt()` ISR loop, resulting in an unhandled interrupt storm and device shutdown.
--

--- Patch [6]: [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid ---
commit 528d2823639b052dd9d6f154ca1675ba5d2b7470
Author: Avi Weiss <thnkslprpt@gmail.com>

can: ctucanfd: mark error-active controller status valid

This commit modifies the error handling path for CAN_STATE_ERROR_ACTIVE
to correctly set the CAN_ERR_CRTL flag in the CAN ID. This ensures
consumers know the controller-status information in cf->data[1] is valid.

Link: https://patch.msgid.link/20260723155543.318414-1-thnkslprpt@gmail.com

> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
> index 0ea1ff28dfce8..8f8b1c097ec6d 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -869,7 +869,7 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)
>  			break;
>  		case CAN_STATE_ERROR_ACTIVE:
>  			if (skb) {
> -				cf->can_id |= CAN_ERR_CNT;
> +				cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
>  				cf->data[1] = CAN_ERR_CRTL_ACTIVE;
>  				cf->data[6] = bec.txerr;
>  				cf->data[7] = bec.rxerr;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this error
handling code I noticed a potential interrupt storm in ctucan_interrupt().

When the interface is configured with CAN_CTRLMODE_BERR_REPORTING, a CAN bus
error will set the REG_INT_STAT_BEI bit. Since any external device on the CAN
bus can intentionally or accidentally cause a bus error, this condition is
reachable from the bus.

However, ctucan_interrupt() does not include BEI in the filter condition or
the clear mask:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_interrupt() {
		/* Error interrupts */
		if (FIELD_GET(REG_INT_STAT_EWLI, isr) ||
		    FIELD_GET(REG_INT_STAT_FCSI, isr) ||
		    FIELD_GET(REG_INT_STAT_ALI, isr)) {
			icr = isr & (REG_INT_STAT_EWLI | REG_INT_STAT_FCSI | REG_INT_STAT_ALI);

			ctucan_netdev_dbg(ndev, "some ERR interrupt: clearing 0x%08x\n", icr);
			ctucan_write32(priv, CTUCANFD_INT_STAT, icr);
			ctucan_err_interrupt(ndev, isr);
		}
}

Since BEI is never cleared from the status register, will this cause the ISR
to loop until it detects a stuck interrupt and permanently disables all device
interrupts, leading to a denial of service?

Should REG_INT_STAT_BEI be included in both the condition and the icr mask
to ensure the interrupt is properly cleared?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=6

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 07/20] can: ctucanfd: handle bus error interrupts
  2026-07-29 10:12 ` [PATCH net 07/20] can: ctucanfd: handle bus error interrupts Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  2026-07-30 11:18     ` Marc Kleine-Budde
  0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Vincent Mailhol, Marc Kleine-Budde, linux-can, Oleksij Rempel,
	Oliver Hartkopp

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

Pre-existing issues:
- [Critical] Missing bounds check on hardware-provided word count can lead to a severe heap buffer overflow.
- [High] Missing rate-limiting on bus error log messages causes a kernel log flood and system lockup.
--

--- Patch [7]: [PATCH net 07/20] can: ctucanfd: handle bus error interrupts ---
commit d64540d1abcd187254c2c60ea1d9abba7c60b2be
Author: Avi Weiss <thnkslprpt@gmail.com>

can: ctucanfd: handle bus error interrupts

Include REG_INT_STAT_BEI in the top-level error interrupt condition. BEI is
enabled when CAN_CTRLMODE_BERR_REPORTING is requested and
ctucan_err_interrupt() already handles it. Without checking and clearing
BEI, bus error interrupts are not handled or acknowledged.

Link: https://patch.msgid.link/20260723074403.131575-1-thnkslprpt@gmail.com

> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
> index 8f8b1c097ec6d..10ebcc13ea656 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -1136,8 +1136,12 @@ static irqreturn_t ctucan_interrupt(int irq, void *dev_id)
>  		/* Error interrupts */
>  		if (FIELD_GET(REG_INT_STAT_EWLI, isr) ||
>  		    FIELD_GET(REG_INT_STAT_FCSI, isr) ||
> -		    FIELD_GET(REG_INT_STAT_ALI, isr)) {
> -			icr = isr & (REG_INT_STAT_EWLI | REG_INT_STAT_FCSI | REG_INT_STAT_ALI);
> +		    FIELD_GET(REG_INT_STAT_ALI, isr) ||
> +		    FIELD_GET(REG_INT_STAT_BEI, isr)) {
> +			icr = isr & (REG_INT_STAT_EWLI |
> +				     REG_INT_STAT_FCSI |
> +				     REG_INT_STAT_ALI |
> +				     REG_INT_STAT_BEI);
>  
>  			ctucan_netdev_dbg(ndev, "some ERR interrupt: clearing 0x%08x\n", icr);
>  			ctucan_write32(priv, CTUCANFD_INT_STAT, icr);

[Severity: High]
This isn't a bug introduced by this patch, but this patch explicitly unmasks
and clears the BEI interrupt in the top-level handler. Does this prevent the
driver from hitting the previous stuck-interrupt failsafe and inadvertently
unleash an infinite log storm whenever BERR reporting is enabled?

Looking at ctucan_err_interrupt(), the unconditional bus error logging
bypasses the dologerr rate-limit check:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_err_interrupt() {
    ...
    if (FIELD_GET(REG_INT_STAT_BEI, isr)) {
        netdev_info(ndev, "bus error\n");
    ...
}

If the CAN bus is faulty or experiencing heavy interference, will the
controller continuously generate Bus Error Interrupts and overwhelm the
printk subsystem causing a CPU lockup?


[Severity: Critical]
This is a pre-existing issue in the same file, but could the lack of bounds
checking on the hardware-provided word count lead to a severe heap buffer
overflow?

In ctucan_read_rx_frame(), the driver derives the word count directly from
hardware:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_read_rx_frame() {
    ...
    wc = FIELD_GET(REG_FRAME_FORMAT_W_RWCNT, ffw) - 3;
    ...
}

If a buggy or malicious hardware controller provides an unexpected word
count, could this integer calculation underflow or result in a large value
that exceeds the maximum 64-byte payload limit?

Later in the same function, the driver copies data into the cf->data
buffer using the unchecked len variable:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_read_rx_frame() {
    ...
    for (i = 0; i < len; i += 4) {
        u32 data = ctucan_read32(priv, CTUCANFD_RX_DATA);
        *(__le32 *)(cf->data + i) = cpu_to_le32(data);
    }
    ...
}

Would this blindly write out of bounds into the heap-allocated SKB buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=7

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK
  2026-07-29 10:12 ` [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  2026-07-30 11:25     ` Marc Kleine-Budde
  0 siblings, 1 reply; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Marc Kleine-Budde, linux-can, Oliver Hartkopp, Vincent Mailhol,
	Oleksij Rempel

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

Pre-existing issues:
- [High] NAPI poll loop permanently deadlocks on SKB allocation failure because it returns less than `quota` without calling `napi_complete_done()`.
--

--- Patch [8]: [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK ---
commit 2ad480a34ed756c09679dabe564744f0df8a351d
Author: Avi Weiss <thnkslprpt@gmail.com>

can: ctucanfd: use self-test mode for PRESUME_ACK

Use self-test mode for CAN_CTRLMODE_PRESUME_ACK so transmitted
frames can complete without receiving an ACK.

ACK forbidden mode prevents the controller from acknowledging
received frames and does not implement the presume-ack behavior.

Link: https://patch.msgid.link/20260722192726.230729-1-thnkslprpt@gmail.com

> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
> index 10ebcc13ea656..07d4aa43c700a 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -340,8 +340,8 @@ static void ctucan_set_mode(struct ctucan_priv *priv, const struct can_ctrlmode
>  			(mode_reg & ~REG_MODE_FDE);
>  
>  	mode_reg = (mode->flags & CAN_CTRLMODE_PRESUME_ACK) ?
> -			(mode_reg | REG_MODE_ACF) :
> -			(mode_reg & ~REG_MODE_ACF);
> +			(mode_reg | REG_MODE_STM) :
> +			(mode_reg & ~REG_MODE_STM);
>  
>  	mode_reg = (mode->flags & CAN_CTRLMODE_FD_NON_ISO) ?
>  			(mode_reg | REG_MODE_NISOFD) :

[Severity: High]
This is a pre-existing issue, but I noticed a potential napi deadlock in
ctucan_rx_poll() when reviewing this file.

If skb allocation fails due to memory pressure, ctucan_rx() returns 0:

drivers/net/can/ctucanfd/ctucanfd_base.c:ctucan_rx_poll() {
	while (framecnt && work_done < quota && res > 0) {
		res = ctucan_rx(ndev);
		work_done++;
		framecnt = FIELD_GET(REG_RX_STATUS_RXFRC, ctucan_read32(priv, CTUCANFD_RX_STATUS));
	}
[ ... ]
	if (!framecnt && res != 0) {
		if (napi_complete_done(napi, work_done)) {
[ ... ]

When res is 0, the loop breaks early and work_done is less than quota. However,
because res is 0, the check if (!framecnt && res != 0) evaluates to false,
which prevents napi_complete_done() from being called.

Does this leave the napi instance in the scheduled state without being
on the core's poll list, permanently deadlocking the receive path until the
interface is restarted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=8

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index
  2026-07-29 10:12 ` [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Marc Kleine-Budde, linux-can, Oleksij Rempel, Vincent Mailhol,
	Oliver Hartkopp

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

Pre-existing issues:
- [High] Unvalidated CAN payload length from the USB packet causes a heap buffer overflow when copying data into the SKB and reading from the URB buffer.
- [High] Inadequate bounds checking allows a NULL pointer dereference of uninitialized channel device pointers.
- [High] Race condition during probe exposes the network interface before driver initialization is complete, leading to a NULL pointer dereference.
--

--- Patch [10]: [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index ---
commit bda9ca94a61ede6bd59945cb6b8e4a5d2a8b5017
Author: James Gao <jamesgao5@outlook.com>

can: peak_usb: add bounds check for USB channel index

The channel control index ctrl_idx is derived from rx->len which comes
directly from a device USB payload. The mask 0x0f allows values 0-15, but
the array size of usb_if->dev[] is only 2. Values 2-15 cause heap
out-of-bounds read, eventually causing kernel panic in the IRQ context.

Link: https://patch.msgid.link/TYWPR01MB8559DBAAAA6A7F410400329CF0012@TYWPR01MB8559.jpnprd01.prod.outlook.com

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> index aefcded8e12a8..b6be8c19e537f 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> @@ -534,12 +534,18 @@ static int pcan_usb_pro_handle_canmsg(struct pcan_usb_pro_interface *usb_if,
>  				      struct pcan_usb_pro_rxmsg *rx)
>  {
>  	const unsigned int ctrl_idx = (rx->len >> 4) & 0x0f;
> -	struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
> -	struct net_device *netdev = dev->netdev;
> +	struct peak_usb_device *dev;
> +	struct net_device *netdev;
>  	struct can_frame *can_frame;
>  	struct sk_buff *skb;
>  	struct skb_shared_hwtstamps *hwts;
>  
> +	if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
> +		return -EINVAL;
> +
> +	dev = usb_if->dev[ctrl_idx];
> +	netdev = dev->netdev;

[Severity: High]
This isn't a bug introduced by this patch, but does checking against
ARRAY_SIZE() fully protect against NULL pointer dereferences here?

If a single-channel adapter is used, usb_if->dev[1] might be left
uninitialized (NULL). If a device sends URBs for channel 1, wouldn't it
bypass the ARRAY_SIZE() check and cause a kernel panic when dereferencing
dev->netdev?

There appears to be an identical inadequate bounds check in the FD variant
of the driver at pcan_usb_fd_decode_canmsg().

[Severity: High]
This is a pre-existing issue, but does this code overflow can_frame->data[]
when copying the payload?

Looking further down in pcan_usb_pro_handle_canmsg():

    can_frame->len = rx->len & 0x0f;
    ...
    } else {
        memcpy(can_frame->data, rx->data, can_frame->len);
    }

Since rx->len & 0x0f allows values up to 15, and can_frame->data[] is only an
8-byte array, could a malicious device send an oversized payload length and
overflow can_frame->data[]?

> +
>  	skb = alloc_can_skb(netdev, &can_frame);
>  	if (!skb)
>  		return -ENOMEM;

[ ... ]

> @@ -573,14 +579,20 @@ static int pcan_usb_pro_handle_error(struct pcan_usb_pro_interface *usb_if,
>  {
>  	const u16 raw_status = le16_to_cpu(er->status);
>  	const unsigned int ctrl_idx = (er->channel >> 4) & 0x0f;
> -	struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
> -	struct net_device *netdev = dev->netdev;
> +	struct peak_usb_device *dev;
> +	struct net_device *netdev;
>  	struct can_frame *can_frame;
>  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
>  	u8 err_mask = 0;
>  	struct sk_buff *skb;
>  	struct skb_shared_hwtstamps *hwts;
>  
> +	if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
> +		return -EINVAL;
> +
> +	dev = usb_if->dev[ctrl_idx];
> +	netdev = dev->netdev;
> +
>  	/* nothing should be sent while in BUS_OFF state */
>  	if (dev->can.state == CAN_STATE_BUS_OFF)
>  		return 0;

[Severity: High]
This is a pre-existing issue, but is there a race condition during probe
that could expose the network interface before driver initialization is
complete?

In peak_usb_create_dev(), register_candev() is called before the interface
is fully initialized:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_create_dev() {
    ...
    err = register_candev(netdev);
    ...
}

If userspace immediately brings the interface up (e.g., via udev),
pcan_usb_pro_start() is called and dereferences pdev->usb_if:

drivers/net/can/usb/peak_usb/pcan_usb_pro.c:pcan_usb_pro_start() {
    ...
    if (pdev->usb_if->dev_opened_count == 0) {
    ...
}

Since pdev->usb_if is only allocated and assigned later in
pcan_usb_pro_init(), wouldn't this cause a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=10

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error
  2026-07-29 10:12 ` [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oleksij Rempel, Oliver Hartkopp, linux-can, Vincent Mailhol,
	Marc Kleine-Budde

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

Pre-existing issues:
- [High] NULL pointer dereference in TX path if TX URBs were partially allocated during device start.
- [Medium] In-flight TX URBs and their transfer buffers are permanently leaked when the CAN interface is brought down.
--

--- Patch [11]: [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error ---
commit 0de6a57bc3019e9140f22ba4065f241bf08735cd
Author: Maoyi Xie <maoyixie.tju@gmail.com>

can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error

This patch removes a redundant kfree() in the RX URB error path to prevent a
double-free. Because URB_FREE_BUFFER is set, usb_free_urb() already releases
the transfer buffer.

Closes: https://lore.kernel.org/linux-can/178159320216.2154888.16953451793788581739@maoyixie.com/T/#u
Link: https://patch.msgid.link/178163373110.2507866.216458825145756798@maoyixie.com

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> index c7933d1acc99a..55aad01cd8cac 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -470,7 +470,6 @@ static int peak_usb_start(struct peak_usb_device *dev)
>  				netif_device_detach(dev->netdev);
>  
>  			usb_unanchor_urb(urb);
> -			kfree(buf);
>  			usb_free_urb(urb);
>  			break;
>  		}

[Severity: High]
This is a pre-existing issue, but does this code cause a NULL pointer
dereference when accessing urb->transfer_buffer?

If usb_alloc_urb() fails after the first allocation in peak_usb_start(), it
leaves unallocated contexts with urb == NULL and
echo_index == PCAN_USB_MAX_TX_URBS:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_start() {
    ...
	/* pre-alloc tx buffers and corresponding urbs */
	for (i = 0; i < PCAN_USB_MAX_TX_URBS; i++) {
        ...
		urb = usb_alloc_urb(0, GFP_KERNEL);
		if (!urb) {
			err = -ENOMEM;
			break;
		}
        ...
	}

	/* warn if we were not able to allocate enough tx contexts */
	if (i < PCAN_USB_MAX_TX_URBS) {
		if (i == 0) {
			netdev_err(netdev, "couldn't setup any tx URB\n");
			goto err_tx;
		}
		netdev_warn(netdev, "tx performance may be slow\n");
	}
    ...
}

Since the network queue isn't stopped based on the successful allocation count,
can peak_usb_ndo_start_xmit() pick an unallocated context and dereference
a NULL urb pointer?

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_ndo_start_xmit() {
    ...
	for (i = 0; i < PCAN_USB_MAX_TX_URBS; i++)
		if (dev->tx_contexts[i].echo_index == PCAN_USB_MAX_TX_URBS) {
			context = dev->tx_contexts + i;
			break;
		}

	if (!context) {
		/* should not occur except during restart */
		return NETDEV_TX_BUSY;
	}

	urb = context->urb;
	obuf = urb->transfer_buffer;
    ...
}


[Severity: Medium]
This is a pre-existing issue, but does this leak struct urb and
transfer_buffer allocations?

When the interface is brought down, peak_usb_unlink_all_urbs() skips calling
usb_free_urb() on in-flight TX URBs, assuming the USB core will free them:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_unlink_all_urbs() {
    ...
		if (!urb ||
		    dev->tx_contexts[i].echo_index != PCAN_USB_MAX_TX_URBS) {
			/*
			 * this urb is already released or always submitted,
			 * let usb core free by itself
			 */
			continue;
		}
    ...
}

However, the driver holds a base reference from usb_alloc_urb(). When the
interface is brought back up, peak_usb_start() allocates new URBs and
overwrites the pointers, permanently leaking the previous allocations:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_start() {
    ...
		context = dev->tx_contexts + i;
		context->dev = dev;
		context->urb = urb;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=11

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths
  2026-07-29 10:12 ` [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oliver Hartkopp, linux-can, Marc Kleine-Budde, Vincent Mailhol,
	Oleksij Rempel

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

New issues:
- [High] The new buffer length check prevents graceful loop termination for valid 4-byte padding records, leading to severe log spam.

Pre-existing issues:
- [Critical] Missing NULL check for uninitialized channel devices leads to a NULL pointer dereference.
--

--- Patch [12]: [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths ---
commit ae45d78fc07951e0485a1ba7921e266e598fe209
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

can: peak_usb: validate uCAN receive record lengths

This patch validates the length of uCAN receive records before copying
their payloads into socket buffers. It requires each record to contain
the fixed header for its type and verifies CAN payload bytes.

Link: https://patch.msgid.link/20260706092836.79754-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> index ef9fd693e9bd3..0d46f4ce5dca7 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> @@ -566,6 +566,13 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
>  				     dev->can.ctrlmode);
>  	}
>  
> +	if (!(rx_msg_flags & PUCAN_MSG_RTR) &&
> +	    le16_to_cpu(rx_msg->size) - offsetof(struct pucan_rx_msg, d) <
> +	    cfd->len) {
> +		kfree_skb(skb);
> +		return -EBADMSG;
> +	}
> +

[Severity: Critical]
This is a pre-existing issue, but is there a missing NULL pointer check when
extracting the channel device in this function?

In pcan_usb_fd_decode_canmsg(), the channel index is checked against the
array size:

        if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
                return -ENOMEM;

        dev = usb_if->dev[pucan_msg_get_channel(rm)];
        netdev = dev->netdev;

For single-channel adapters, usb_if->dev[1] is never initialized and remains
NULL. If a malicious or malfunctioning USB device sends a CAN RX message
specifying channel 1, will the driver fetch the NULL pointer and immediately
dereference it when accessing dev->netdev?

This same pattern appears to be present in pcan_usb_fd_decode_status(),
pcan_usb_fd_decode_error(), and pcan_usb_fd_decode_overrun().

[ ... ]

> @@ -728,6 +753,12 @@ static int pcan_usb_fd_decode_buf(struct peak_usb_device *dev, struct urb *urb)
>  	msg_end = urb->transfer_buffer + urb->actual_length;
>  	for (; msg_ptr < msg_end;) {
>  		u16 rx_msg_type, rx_msg_size;
> +		size_t rx_msg_min_size;
> +
> +		if (msg_end - msg_ptr < sizeof(*rx_msg)) {
> +			err = -EBADMSG;
> +			break;
> +		}

[Severity: High]
Will this check fail on valid 4-byte null tags used for padding at the end
of a transfer buffer?

If the hardware or driver uses a 4-byte null tag to mark the end of valid
records (where size = 0), this check for a full 12-byte struct pucan_msg 
will evaluate to true. 

This breaks the loop with err = -EBADMSG, which triggers the fail path and
calls pcan_dump_mem(). Because pcan_dump_mem() uses un-ratelimited logging,
could this result in severe kernel log spam for every receive transfer that
ends in 4-byte padding?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=12

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents
  2026-07-29 10:12 ` [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Vincent Mailhol, Oleksij Rempel, linux-can, Marc Kleine-Budde,
	Oliver Hartkopp

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

Pre-existing issues:
- [High] Processing a zero-length extended command or encountering a bulk endpoint with a zero wMaxPacketSize causes an infinite loop and CPU soft lockup.
--

--- Patch [14]: [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents ---
commit ad6fd8163961ebd7ee69b192c06c6c10803d720a
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
Subject: can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents

The wait and bulk receive paths walk variable-length commands from a
USB buffer. A nonzero command shorter than CMD_HEADER_LEN can still be
dispatched, and the wait path copies a matching command into a fixed
caller-owned struct kvaser_cmd using the device-provided length.

Link: https://patch.msgid.link/20260722042221.44066-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> index df737cfc5ea0c..a876c7819b81c 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -691,13 +691,22 @@ static int kvaser_usb_leaf_wait_cmd(const struct kvaser_usb *dev, u8 id,
>  				continue;
>  			}
>  
> -			if (pos + tmp->len > actual_len) {
> +			if (tmp->len < CMD_HEADER_LEN ||
> +			    tmp->len > actual_len - pos) {

[Severity: High]
This isn't a bug introduced by this patch, but might the logic directly above
this bounds check still allow an infinite loop?

If a USB device provides a bulk IN endpoint with wMaxPacketSize == 0, the check
for tmp->len == 0 behaves as follows:

			if (tmp->len == 0) {
				pos = round_up(pos + 1,
					       le16_to_cpu
						(dev->bulk_in->wMaxPacketSize));
				continue;
			}

Here, round_up(pos + 1, 0) evaluates to 0, which resets pos to 0. Since this
executes before the new tmp->len < CMD_HEADER_LEN check, the loop would fail to
advance and hang the process.

This same pattern appears in kvaser_usb_leaf_read_bulk_callback() modified
later in this patch.

Additionally, a similar pre-existing issue seems to be present in the
kvaser_usb_hydra driver variant. In kvaser_usb_hydra_wait_cmd() and
kvaser_usb_hydra_read_bulk_callback(), if an extended command specifies a length
of 0, kvaser_usb_hydra_cmd_size() returns 0. The loop then increments pos by 0,
failing to advance and causing a soft lockup.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=14

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets
  2026-07-29 10:12 ` [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oliver Hartkopp, Oleksij Rempel, Marc Kleine-Budde, linux-can,
	Vincent Mailhol

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

Pre-existing issues:
- [High] If `rcar_canfd_global_init()` fails, `rcar_canfd_probe()` incorrectly calls `rcar_canfd_global_deinit()`, causing a kernel panic from accessing registers with disabled clocks, followed by a double-disable of clocks and double-assertion of resets.
- [High] If `register_candev()` fails during channel probe, the network device is freed, but the global context retains a dangling pointer to it, which is subsequently dereferenced and double-freed during the driver probe error teardown.
--

--- Patch [15]: [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets ---
commit 4a859a221c1b4bd83a846c702809583b08a6af18
Author: Tu Nguyen <tu.nguyen.xg@renesas.com>

can: rcar_canfd: change the initializing flow for clocks and resets

This patch updates the initialization order for clocks and resets to match
the hardware manual, ensuring clocks are supplied before resets are
deasserted. It also updates the teardown path in rcar_canfd_global_deinit()
to mirror this new init ordering.

Link: https://patch.msgid.link/20260625135216.130450-1-biju.das.jz@bp.renesas.com

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
[ ... ]
> @@ -2068,14 +2068,14 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
>  
>  fail_mode:
>  	rcar_canfd_disable_global_interrupts(gpriv);
[ ... ]
> +fail_ram_clk:
> +	clk_disable_unprepare(gpriv->clk_ram);
> +fail_clk:
> +	clk_disable_unprepare(gpriv->clkp);
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path
cause a panic and double-disable of clocks here?

When rcar_canfd_global_init() fails, it correctly cleans up its state and
disables the clocks. However, in rcar_canfd_probe(), the failure jumps to
the fail_mode label:

rcar_canfd_probe() {
    ...
    err = rcar_canfd_global_init(gpriv);
    if (err)
        goto fail_mode;
    ...
fail_mode:
    rcar_canfd_global_deinit(gpriv, false);
    ...
}

Since the clocks were already disabled by the initialization failure path,
calling rcar_canfd_global_deinit() will access hardware registers with
disabled clocks via rcar_canfd_disable_global_interrupts(). Can this trigger
a precise or imprecise external abort? It also looks like it will redundantly
assert the resets and disable the clocks a second time.

[Severity: High]
This is also a pre-existing issue, but could this probe error cleanup sequence
trigger a double-free on a channel probe failure?

In rcar_canfd_channel_probe(), the device pointer is saved to the global
array before registration:

rcar_canfd_channel_probe() {
    ...
    gpriv->ch[priv->channel] = priv;
    err = register_candev(ndev);
    if (err) {
        ...
        goto fail_candev;
    }
    ...
fail:
    free_candev(ndev);
}

If register_candev() fails, the memory is freed but the dangling pointer
remains in the global array. Back in rcar_canfd_probe(), the error is caught
and rcar_canfd_channel_remove() is called for all channels in the mask. Does
the cleanup loop find this dangling pointer and try to operate on the
already-freed network device, causing a use-after-free and double-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=15

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans
  2026-07-29 10:12 ` [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans Marc Kleine-Budde
@ 2026-07-30 10:29   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oleksij Rempel, Vincent Mailhol, linux-can, Marc Kleine-Budde,
	Oliver Hartkopp

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

Pre-existing issues:
- [High] Unaligned memory dereference in `fw_parse()` triggers alignment faults.
- [High] Truncated or malformed firmware causes silent failure by returning 0 on error paths.
- [High] Unaligned MMIO accesses to DPRAM via `iowrite32` cause hardware exceptions.
--

--- Patch [16]: [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans ---
commit dd9e23c1084e29145c02ec85df264fc5faf868da
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

can: softing: fw_parse(): validate firmware record spans

This commit improves bounds checking when parsing softing firmware records
to prevent out-of-bounds reads and writes. It passes the firmware end to
the parser and validates the source record length before doing DPRAM staging
copies.

Link: https://patch.msgid.link/20260722044347.2708-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
> index 721df91cdbfbc..282570daf3efe 100644
> --- a/drivers/net/can/softing/softing_fw.c
> +++ b/drivers/net/can/softing/softing_fw.c

[ ... ]

> @@ -114,14 +114,21 @@ static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
>  	 * endianness & alignment.
>  	 */
>  	mem = *pmem;
> +	/* A record needs an 8-byte prefix and a 2-byte checksum. */
> +	if (mem > limit || limit - mem < 10)
> +		return -EINVAL;
> +
>  	*ptype = le16_to_cpup((void *)&mem[0]);
>  	*paddr = le32_to_cpup((void *)&mem[2]);
>  	*plen = le16_to_cpup((void *)&mem[6]);

[Severity: High]
This is a pre-existing issue, but does using le16_to_cpup() and
le32_to_cpup() on an unaligned byte stream cause alignment faults?

Firmware records are packed without padding, meaning mem could rest on an odd
address. The leXX_to_cpup functions directly dereference the pointer, which
can trigger a fatal alignment fault exception (kernel oops) on architectures
that lack hardware unaligned access support. Should fw_parse() use
get_unaligned_le16() and get_unaligned_le32() here instead?

> +	if (*plen > limit - mem - 10)
> +		return -EINVAL;
> +
>  	*pdat = &mem[8];
>  	/* verify checksum */
> -	end = &mem[8 + *plen];
> -	checksum[0] = le16_to_cpup((void *)end);
> -	for (checksum[1] = 0; mem < end; ++mem)
> +	record_end = &mem[8 + *plen];
> +	checksum[0] = le16_to_cpup((void *)record_end);
> +	for (checksum[1] = 0; mem < record_end; ++mem)
>  		checksum[1] += *mem;

[ ... ]

> @@ -279,6 +291,12 @@ int softing_load_app_fw(const char *file, struct softing *card)
>  		/* work in 16bit (target) */
>  		sum &= 0xffff;
>  
> +		if (card->pdat->app.offs > card->dpram_size ||
> +		    len > card->dpram_size - card->pdat->app.offs) {
> +			ret = -EINVAL;
> +			goto failed;
> +		}
> +
>  		memcpy_toio(&card->dpram[card->pdat->app.offs], dat, len);
>  		iowrite32(card->pdat->app.offs + card->pdat->app.addr,
>  				&card->dpram[DPRAM_COMMAND + 2]);

[Severity: High]
This is a pre-existing issue, but does using iowrite32() on unaligned
offsets cause hardware exceptions?

These iowrite32() calls target DPRAM addresses shifted by unaligned offsets
like DPRAM_COMMAND + 2 and DPRAM_COMMAND + 6. Standard 32-bit I/O accessors
mandate 4-byte alignment on the target memory address. Executing an
unaligned MMIO write triggers a hardware alignment fault or bus error on
non-x86 architectures. Should these operations be split into sequential
16-bit writes using iowrite16()?

[Severity: High]
This is a pre-existing issue, but does a truncated or malformed firmware
blob cause a silent failure by returning 0?

In both softing_load_fw() and softing_load_app_fw(), if the firmware blob
does not contain an EOF record or an entrypoint record, the parsing loop
terminates without encountering an error:

softing_fw.c:softing_load_app_fw() {
    ...
    while (mem < end) {
        ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
        if (ret)
            goto failed;
        ...
    }
    if (!type_end || !type_entrypoint)
        goto failed;
    ...
failed:
    ...
    return ret;
}

Because fw_parse() returns 0 on success, ret is left at 0 when the loop
finishes naturally. The code then detects the missing record and jumps to
the failed label, but since ret is 0, it incorrectly returns success to
the caller. This leaves the device uninitialized. Could ret be set to an
error code like -EINVAL before the goto?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=16

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured
  2026-07-29 10:12 ` [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured Marc Kleine-Budde
@ 2026-07-30 10:30   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:30 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
	Oleksij Rempel, linux-can

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

Pre-existing issues:
- [High] c_can_set_bittiming() inadvertently resets C_CAN_TEST_REG, breaking loopback and listen-only modes.
--

--- Patch [17]: [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured ---
commit 43c179b24ec630378edf5e7aee260bd073975b00
Author: Lucas Martins Alves <lucas.alves@lumal21.com.br>

can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured

c_can_chip_config() was programming C_CAN_CTRL_REG without CONTROL_INIT,
which may allow the controller to become active before
c_can_set_bittiming() finishes.

Link: https://patch.msgid.link/20260714164839.771123-1-lucas.alves@lumal21.com.br

> diff --git a/drivers/net/can/c_can/c_can_main.c b/drivers/net/can/c_can/c_can_main.c
> index 3702cac7fbf0f..b3b321d9ce68d 100644
> --- a/drivers/net/can/c_can/c_can_main.c
> +++ b/drivers/net/can/c_can/c_can_main.c
> @@ -597,20 +597,20 @@ static int c_can_chip_config(struct net_device *dev)
>  		return err;
>  
>  	/* enable automatic retransmission */
> -	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
> +	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR | CONTROL_INIT);
>  
>  	if ((priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) &&
>  	    (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)) {
>  		/* loopback + silent mode : useful for hot self-test */
> -		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
>  		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK | TEST_SILENT);
>  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
>  		/* loopback mode : useful for self-test function */
> -		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
>  		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
>  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
>  		/* silent mode : bus-monitoring mode */
> -		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST | CONTROL_INIT);
>  		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but doesn't the subsequent call to
c_can_set_bittiming() at the end of c_can_chip_config() unconditionally clear
the CONTROL_TEST bit?

c_can_set_bittiming() executes this:
	priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_CCE | CONTROL_INIT);

Because this write does not include CONTROL_TEST, it gets cleared. According to
the Bosch C_CAN hardware manual, clearing the Test bit instantly resets the
Test Register to its default value (0x00).

Does this silently erase the loopback and listen-only modes configured here,
causing the controller to incorrectly operate in normal mode and potentially
cause active disruption on a live CAN bus?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=17

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 20/20] can: ems_usb: validate CPC message lengths
  2026-07-29 10:13 ` [PATCH net 20/20] can: ems_usb: validate CPC message lengths Marc Kleine-Budde
@ 2026-07-30 10:30   ` sashiko-bot
  0 siblings, 0 replies; 38+ messages in thread
From: sashiko-bot @ 2026-07-30 10:30 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Oleksij Rempel, linux-can, Oliver Hartkopp, Vincent Mailhol,
	Marc Kleine-Budde

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

Pre-existing issues:
- [High] TX context leak in ems_usb_start_xmit() error path.
--

--- Patch [20]: [PATCH net 20/20] can: ems_usb: validate CPC message lengths ---
commit 2db7f239e46ba1c12e2761c8d9fa8b163a632adc
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

can: ems_usb: validate CPC message lengths

ems_usb_read_bulk_callback() walks CPC messages packed in one USB
receive buffer. Check that each declared message fits in the URB payload
and require type-specific payloads to cover the fields used by handlers.

Link: https://patch.msgid.link/20260706092752.79600-1-pengpeng@iscas.ac.cn

> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index 9b25dda7c1838..24cf8f651f8f1 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c

[ ... ]

While reviewing this patch, I noticed a pre-existing issue in the
ems_usb_start_xmit() error path:

drivers/net/can/usb/ems_usb.c:ems_usb_start_xmit()
>	err = usb_submit_urb(urb, GFP_ATOMIC);
>	if (unlikely(err)) {
>		can_free_echo_skb(netdev, context->echo_index, NULL);
>
>		usb_unanchor_urb(urb);
>		usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
>
>		atomic_dec(&dev->active_tx_urbs);

[Severity: High]
This isn't a bug introduced by this patch, but does this error path
permanently leak the transmission context slot?

If usb_submit_urb() fails here, it cleans up the URB and echo_skb, but
doesn't reset context->echo_index to MAX_TX_URBS. After enough failures,
could this cause context allocations to fail and wedge the transmission path
indefinitely, causing the driver to always return NETDEV_TX_BUSY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729102802.505168-1-mkl@pengutronix.de?part=20

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid
  2026-07-30 10:29   ` sashiko-bot
@ 2026-07-30 11:14     ` Marc Kleine-Budde
  0 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-30 11:14 UTC (permalink / raw)
  To: sashiko-bot; +Cc: Oliver Hartkopp, linux-can, Oleksij Rempel, Vincent Mailhol

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On 30.07.2026 10:29:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The `REG_INT_STAT_BEI` (Bus Error) interrupt is enabled when `CAN_CTRLMODE_BERR_REPORTING` is requested, but it is never checked or cleared in the main `ctucan_interrupt()` ISR loop, resulting in an unhandled interrupt storm and device shutdown.

This is fixed with "[PATCH net 07/20] can: ctucanfd: handle bus error
interrupts"

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 07/20] can: ctucanfd: handle bus error interrupts
  2026-07-30 10:29   ` sashiko-bot
@ 2026-07-30 11:18     ` Marc Kleine-Budde
  0 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-30 11:18 UTC (permalink / raw)
  To: sashiko-bot; +Cc: Vincent Mailhol, linux-can, Oleksij Rempel, Oliver Hartkopp

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

On 30.07.2026 10:29:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing bounds check on hardware-provided word count can lead to a severe heap buffer overflow.

The hardware is trusted at this point in the driver.

> - [High] Missing rate-limiting on bus error log messages causes a
> kernel log flood and system lockup.

I'll send a follow up patch.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK
  2026-07-30 10:29   ` sashiko-bot
@ 2026-07-30 11:25     ` Marc Kleine-Budde
  0 siblings, 0 replies; 38+ messages in thread
From: Marc Kleine-Budde @ 2026-07-30 11:25 UTC (permalink / raw)
  To: sashiko-bot
  Cc: linux-can, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel,
	Pavel Pisa, Ondrej Ille

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On 30.07.2026 10:29:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] NAPI poll loop permanently deadlocks on SKB allocation failure because it returns less than `quota` without calling `napi_complete_done()`.

Should be fixed in a follow up patch.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering
  2026-07-30 10:29   ` sashiko-bot
@ 2026-07-30 12:28     ` Oliver Hartkopp
  0 siblings, 0 replies; 38+ messages in thread
From: Oliver Hartkopp @ 2026-07-30 12:28 UTC (permalink / raw)
  To: sashiko-reviews, Marc Kleine-Budde
  Cc: Oleksij Rempel, Vincent Mailhol, linux-can



On 30.07.26 12:29, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Silently dropped asynchronous RX socket errors in isotp_sendmsg() due to ignored return value of sock_error(sk).
> --
> 
> --- Patch [3]: [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering ---
> commit 3e2ba28e6e9e34bb3ea9b3075735283b9c9490c0
> Author: Oliver Hartkopp <socketcan@hartkopp.net>
> can: isotp: fix timer drain order, wakeup handling and tx_gen ordering
> 
> This commit addresses several concurrent state and timer handling issues in
> the ISOTP protocol. It introduces a generation counter and packed result
> array to track transfer state accurately across lock-free accesses.
> 
> Link: https://patch.msgid.link/20260724181525.43556-1-socketcan@hartkopp.net
> 
>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index 54becaf6898f1..1f11c66b343c8 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
> [ ... ]
>> @@ -1223,20 +1321,49 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
> [ ... ]
>>   	if (wait_tx_done) {
>> -		/* wait for complete transmission of current pdu */
>> -		err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
>> +		/* wake up for:
>> +		 * - concurrent sendmsg() claiming a new transfer
>> +		 * - complete transmission of current PDU
>> +		 * - shutdown state change in isotp_release()
>> +		 * isotp_tx_gen_done() uses common tx.state/tx_gen read sequence
>> +		 */
>> +		err = wait_event_interruptible(so->wait,
>> +					       isotp_tx_gen_done(so, my_gen));
>>   		if (err)
>>   			goto err_event_drop;
>>   
>> -		err = sock_error(sk);
>> -		if (err)
>> -			return err;
>> +		/* still our claim, but isotp_release() force-shut it down */
>> +		if (smp_load_acquire(&so->tx.state) == ISOTP_SHUTDOWN &&
>> +		    READ_ONCE(so->tx_gen) == my_gen) {
>> +			err = -EADDRNOTAVAIL;
>> +			goto err_event_drop;
>> +		}
>> +
>> +		/* own completion, or tx_gen moved on - either way this is
>> +		 * what isotp_get_tx_result() recorded for my_gen
>> +		 */
>> +		err = isotp_get_tx_result(so, my_gen);
>> +
>> +		/* drain to avoid stale error for a later poll()/SO_ERROR */
>> +		sock_error(sk);
> 
> [Severity: High]
> Does this unconditionally clear and discard concurrent asynchronous socket
> errors?
> 
> By ignoring the return value of sock_error() here in isotp_sendmsg(), it
> appears any unrelated RX errors (such as ETIMEDOUT set by
> isotp_rx_timer_handler()) would be permanently lost without being reported
> to the user.
> 
> Could this cause applications to hang in recvmsg() or poll() indefinitely
> because they never see the RX connection failure?
> 

No.

sk_err is a single per-socket slot shared by the RX and TX paths, so it 
cannot cleanly separate a TX result from a concurrent RX error.

Note that a plain unreliable datagram protocol like UDP/IP gives no 
feedback for bad or missing received data at all - it just silently 
drops it. ISO-TP does a bit more and surfaces RX problems 
(EBADMSG/EILSEQ/ETIMEDOUT) via sk_err on a best-effort basis, which is 
fine: for an unreliable protocol these are ultimately caught by timeouts 
at a higher ISO layer anyway.

What this patch fixes to 100% is the send path's POSIX contract: 
sendmsg() now returns the TX-specific result from tx_result[] (>0 bytes 
on success, <0 on a real TX error), instead of returning whatever 
happened to sit in sk_err - which could misreport an unrelated 
concurrent RX error as the TX outcome. Draining sk_err afterwards is 
required in CAN_ISOTP_WAIT_TX_DONE mode, since the result is delivered 
synchronously via the return value.

So the send path is now correct and race-free; the shared-sk_err RX 
notification stays best-effort by design. Truly separating the two would 
need distinct RX/TX error channels (e.g. MSG_ERRQUEUE), which is a 
feature, not a fix, and out of scope here.

Best regards,
Oliver


^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2026-07-30 12:28 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 10:12 [PATCH net 0/20] pull-request: can 2026-07-29 Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 01/20] can: j1939: use netdevice_tracker for j1939_{priv,session,ecu} tracking Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 02/20] can: j1939: transport: j1939_session_fresh_new(): initialize receive buffer Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 03/20] can: isotp: fix timer drain order, wakeup handling and tx_gen ordering Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-30 12:28     ` Oliver Hartkopp
2026-07-29 10:12 ` [PATCH net 04/20] can: isotp: check register_netdevice_notifier() error in module init Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 05/20] can: ctucanfd: unmap BAR0 using base address Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 06/20] can: ctucanfd: mark error-active controller status valid Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-30 11:14     ` Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 07/20] can: ctucanfd: handle bus error interrupts Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-30 11:18     ` Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 08/20] can: ctucanfd: use self-test mode for PRESUME_ACK Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-30 11:25     ` Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 09/20] can: ctucanfd: add missing MODULE_DEVICE_TABLE() Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 10/20] can: peak_usb: add bounds check for USB channel index Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 11/20] can: peak_usb: peak_usb_start(): fix double free of transfer buffer on URB submit error Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 12/20] can: peak_usb: validate uCAN receive record lengths Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 13/20] can: kvaser_usb: kvaser_usb_hydra_get_busparams(): fix memory leak in kvaser_usb_hydra_get_busparams() Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 14/20] can: kvaser_usb_leaf: kvaser_usb_leaf_wait_cmd(): validate received command extents Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 15/20] can: rcar_canfd: change the initializing flow for clocks and resets Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 16/20] can: softing: fw_parse(): validate firmware record spans Marc Kleine-Budde
2026-07-30 10:29   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 17/20] can: c_can: c_can_chip_config(): keep controller in init mode until bittiming is configured Marc Kleine-Budde
2026-07-30 10:30   ` sashiko-bot
2026-07-29 10:12 ` [PATCH net 18/20] can: gs_usb: gs_usb_receive_bulk_callback(): resubmit URB on skb allocation failure Marc Kleine-Budde
2026-07-29 10:12 ` [PATCH net 19/20] can: etas_es58x: es58x_read_bulk_callback(): fix RX buffer leak on URB resubmit failure Marc Kleine-Budde
2026-07-29 10:13 ` [PATCH net 20/20] can: ems_usb: validate CPC message lengths Marc Kleine-Budde
2026-07-30 10:30   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.