From: Vincent Mailhol Sending a PF_PACKET bypasses the CAN framework logic and can directly reach a CAN driver's xmit() function. The PF_PACKET framework only checks that skb->len does not exceed the net_device MTU. For a CAN device that is not CAN XL capable, anything above CANFD_MTU (72 bytes) is therefore dropped before it reaches the driver. However, CAN XL frames are variable length. can_is_canxl_skb() accepts lengths in the range CANXL_HDR_SIZE + CANXL_MIN_DLEN up to CANXL_MTU, i.e. 13 to 2060 bytes. As a result, an ETH_P_CANXL skb with a length between 13 and 72 bytes can pass both the MTU and the can_dropped_invalid_skb() checks. A driver that does not support CAN XL will interpret canxl_frame->flags as a length because of the overlap with can_frame->len. And because CANXL_XLF is set, the resulting length is between 128 and 255. For drivers that do not check can_frame->len before copying can_frame->data, as most drivers do, this results in a buffer overflow of up to 247 bytes. Drop ETH_P_CANXL skbs if the device does not have the CAN_CAP_XL capability. Keep can_is_canxl_skb() for the validation of CAN XL skbs. Closes: https://sashiko.dev/#/patchset/20260731-master-v5-0-5b27029dee20@qq.com?part=1 Fixes: fb08cba12b52 ("can: canxl: update CAN infrastructure for CAN XL frames") Signed-off-by: Vincent Mailhol Link: https://patch.msgid.link/20260731-drop_canxl_frames-v1-1-7387b70353b3@kernel.org Cc: stable@kernel.org Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/skb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c index 95fcdc1026f8..4f7a189de265 100644 --- a/drivers/net/can/dev/skb.c +++ b/drivers/net/can/dev/skb.c @@ -4,6 +4,7 @@ * Copyright (C) 2008-2009 Wolfgang Grandegger */ +#include #include #include #include @@ -384,7 +385,7 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb) break; case ETH_P_CANXL: - if (!can_is_canxl_skb(skb)) + if (!can_cap_enabled(dev, CAN_CAP_XL) || !can_is_canxl_skb(skb)) goto inval_skb; break; base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7 -- 2.53.0 From: Cunhao Lu <1579567540@qq.com> can_put_echo_skb() can be called with hardware interrupts disabled. Its direct drop paths use kfree_skb(), while can_create_echo_skb() uses kfree_skb() when cloning fails and consume_skb() after a successful clone. None of these helpers is safe in every IRQ context. Use dev_kfree_skb_any() for all drop paths and dev_consume_skb_any() when consuming a successfully cloned skb. This preserves the respective skb drop and consumed semantics regardless of the caller IRQ context. Signed-off-by: Cunhao Lu <1579567540@qq.com> Link: https://patch.msgid.link/tencent_E84809CF236D0137885E7E4E4D58340B3208@qq.com [mkl: also convert can_dropped_invalid_skb(), can_dev_dropped_skb()] Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/skb.c | 6 +++--- include/linux/can/dev.h | 2 +- include/linux/can/skb.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c index 4f7a189de265..e0eac3678a77 100644 --- a/drivers/net/can/dev/skb.c +++ b/drivers/net/can/dev/skb.c @@ -63,7 +63,7 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, (skb->protocol != htons(ETH_P_CAN) && skb->protocol != htons(ETH_P_CANFD) && skb->protocol != htons(ETH_P_CANXL))) { - kfree_skb(skb); + dev_kfree_skb_any(skb); return 0; } @@ -91,7 +91,7 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, } else { /* locking problem with netif_stop_queue() ?? */ netdev_err(dev, "%s: BUG! echo_skb %d is occupied!\n", __func__, idx); - kfree_skb(skb); + dev_kfree_skb_any(skb); return -EBUSY; } @@ -399,7 +399,7 @@ bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb) return false; inval_skb: - kfree_skb(skb); + dev_kfree_skb_any(skb); dev->stats.tx_dropped++; return true; } diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index 6d0710d6f571..346faddee02e 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -176,7 +176,7 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s return can_dropped_invalid_skb(dev, skb); invalid_skb: - kfree_skb(skb); + dev_kfree_skb_any(skb); dev->stats.tx_dropped++; return true; } diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h index a70a02967071..78c5870e2f9a 100644 --- a/include/linux/can/skb.h +++ b/include/linux/can/skb.h @@ -76,12 +76,12 @@ static inline struct sk_buff *can_create_echo_skb(struct sk_buff *skb) nskb = skb_clone(skb, GFP_ATOMIC); if (unlikely(!nskb)) { - kfree_skb(skb); + dev_kfree_skb_any(skb); return NULL; } can_skb_set_owner(nskb, skb->sk); - consume_skb(skb); + dev_consume_skb_any(skb); return nskb; } -- 2.53.0 From: Cunhao Lu <1579567540@qq.com> The CAN skb allocation helpers are used from hardware interrupt receive handlers. If can_skb_ext_add() fails, they release the newly allocated skb with kfree_skb(), which is not safe in hardware interrupt context. Use dev_kfree_skb_any() for the allocation failure paths in alloc_can_skb(), alloc_canfd_skb(), and alloc_canxl_skb(). Fixes: 96ea3a1e2d31 ("can: add CAN skb extension infrastructure") Cc: stable@vger.kernel.org Signed-off-by: Cunhao Lu <1579567540@qq.com> Link: https://patch.msgid.link/tencent_C825C17D442F801351CE2FBC4984064B4605@qq.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/skb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c index e0eac3678a77..d1f797d3778d 100644 --- a/drivers/net/can/dev/skb.c +++ b/drivers/net/can/dev/skb.c @@ -224,7 +224,7 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf) csx = can_skb_ext_add(skb); if (!csx) { - kfree_skb(skb); + dev_kfree_skb_any(skb); goto out_error_cc; } @@ -255,7 +255,7 @@ struct sk_buff *alloc_canfd_skb(struct net_device *dev, csx = can_skb_ext_add(skb); if (!csx) { - kfree_skb(skb); + dev_kfree_skb_any(skb); goto out_error_fd; } @@ -293,7 +293,7 @@ struct sk_buff *alloc_canxl_skb(struct net_device *dev, csx = can_skb_ext_add(skb); if (!csx) { - kfree_skb(skb); + dev_kfree_skb_any(skb); goto out_error_xl; } -- 2.53.0 From: Cunhao Lu <1579567540@qq.com> can_put_echo_skb() consumes the skb on all paths except when the echo index is out of bounds. This leaves ownership with the caller on -EINVAL, unlike the other error paths, and can leak the skb if the caller expects consistent semantics. Free the skb before returning -EINVAL so that all return paths consume it. Fixes: 6411959c10fe ("can: dev: can_put_echo_skb(): don't crash kernel if can_priv::echo_skb is accessed out of bounds") Cc: stable@vger.kernel.org Reviewed-by: Vincent Mailhol Signed-off-by: Cunhao Lu <1579567540@qq.com> Link: https://patch.msgid.link/tencent_683AA16E643DE00211CD2FB62991264DC605@qq.com Signed-off-by: Marc Kleine-Budde --- FAQ: Q: `can_skb_init_valid()` directly modifies `skb->data` without checking if the SKB is cloned or shared. Is this violating SKB shared buffer rules and causing data corruption? A: The finding about can_skb_init_valid() modifying skb->data on a potentially shared buffer is not relevant for this (correct) patch. The FDF-flag write in can_skb_init_valid() only matters for PF_PACKET use: PF_CAN allocators already set CANFD_FDF. The write exists to normalize frames for PF_PACKET observers (tcpdump/Wireshark) so they can distinguish classic CAN from CAN FD at the netdev level, and to compensate for PF_PACKET senders that omit the bit. That normalization is functionally required. The shared-buffer concern only becomes realistic through a specific chain: a PF_PACKET sender injects a CANFD-sized frame, the CAN driver's loopback puts an echo skb back into can_rcv(), and cgw (without modfuncs) clones it for forwarding to another interface. Only on that second can_skb_init_valid() call is the skb cloned. By then the bit was already set on the exclusive skb during the initial xmit path, so the flags |= CANFD_FDF write is strictly idempotent for any parallel consumer of the shared buffer - no memory-safety or information-disclosure consequence. It's a formal violation of the skb sharing rules without an observable effect, so the current code can stay as-is. Link: https://lore.kernel.org/all/54a3cc01-abcf-4a33-b932-39bb1f68cdd5@hartkopp.net/ [mkl: convert Oliver's mail to FAQ section] --- drivers/net/can/dev/skb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c index d1f797d3778d..f414b75b5c0d 100644 --- a/drivers/net/can/dev/skb.c +++ b/drivers/net/can/dev/skb.c @@ -55,6 +55,7 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, if (idx >= priv->echo_skb_max) { netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n", __func__, idx, priv->echo_skb_max); + dev_kfree_skb_any(skb); return -EINVAL; } -- 2.53.0 From: zjamg Commit 9f10374bb024 ("can: remove private CAN skb headroom infrastructure") removed the skb_reset_mac_header()/skb_reset_network_header()/ skb_reset_transport_header() calls from init_can_skb(). As a result, RX skbs from alloc_can_skb() and friends again carry mac_header = 0xFFFF. When such an skb reaches packet_rcv_spkt() (SOCK_PACKET), the push length calculation overflows and triggers skb_under_panic -> kernel BUG -> full machine panic. The same issue was originally reported in 2014 on linux-can and fixed by commit 969439016d2c ("can: add missing initialisations in CAN related skbuffs"). packet_rcv_spkt() itself has never been hardened: only packet_rcv() and tpacket_rcv() gained dev_has_header() checks in commit d549699048b4 ("net/packet: fix packet receive on L3 devices without visible hard header"). Fixes: 9f10374bb024 ("can: remove private CAN skb headroom infrastructure") Cc: stable@vger.kernel.org Reviewed-by: Oliver Hartkopp Acked-by: Oliver Hartkopp Signed-off-by: zjamg Tested-by: Quchaosheng Link: https://patch.msgid.link/20260917123716.63116-1-ndaugoing@gmail.com Reported-by: Shaunak Datar Closes: https://lore.kernel.org/all/20260918151606.765490-1-shaunakkdatar@gmail.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/skb.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c index f414b75b5c0d..738d5968dfa6 100644 --- a/drivers/net/can/dev/skb.c +++ b/drivers/net/can/dev/skb.c @@ -212,6 +212,10 @@ static void init_can_skb(struct sk_buff *skb) { skb->pkt_type = PACKET_BROADCAST; skb->ip_summed = CHECKSUM_UNNECESSARY; + + skb_reset_mac_header(skb); + skb_reset_network_header(skb); + skb_reset_transport_header(skb); } struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf) -- 2.53.0 From: Tetsuo Handa syzbot is reporting "struct j1939_ecu" refcount leak, which occurs when netdev_hold() is called during ECU creation but the corresponding netdev_put() is never executed because the parent "struct j1939_ecu" object is leaked. unregister_netdevice: waiting for vxcan1 to become free. Usage count = 3 ref_tracker: netdev@ffff8880710f0700 has 1/2 users at __netdev_tracker_alloc include/linux/netdevice.h:4496 [inline] netdev_hold include/linux/netdevice.h:4525 [inline] j1939_ecu_create_locked+0x1c9/0x400 net/can/j1939/bus.c:159 j1939_local_ecu_get+0xeb/0x220 net/can/j1939/bus.c:293 j1939_sk_bind+0x70a/0xc60 net/can/j1939/socket.c:529 __sys_bind_socket net/socket.c:1920 [inline] __sys_bind+0x2e3/0x410 net/socket.c:1951 __do_sys_bind net/socket.c:1956 [inline] __se_sys_bind net/socket.c:1954 [inline] __x64_sys_bind+0x7a/0x90 net/socket.c:1954 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94 entry_SYSCALL_64_after_hwframe+0x77/0x7f ref_tracker: netdev@ffff8880710f0700 has 1/2 users at __netdev_tracker_alloc include/linux/netdevice.h:4496 [inline] netdev_hold include/linux/netdevice.h:4525 [inline] j1939_priv_create net/can/j1939/main.c:140 [inline] j1939_netdev_start+0x387/0xb20 net/can/j1939/main.c:268 j1939_sk_bind+0x946/0xc60 net/can/j1939/socket.c:506 __sys_bind_socket net/socket.c:1920 [inline] __sys_bind+0x2e3/0x410 net/socket.c:1951 __do_sys_bind net/socket.c:1956 [inline] __se_sys_bind net/socket.c:1954 [inline] __x64_sys_bind+0x7a/0x90 net/socket.c:1954 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94 entry_SYSCALL_64_after_hwframe+0x77/0x7f The root cause lies in the error handling of j1939_sk_bind() during a re-bind operation (binding an already bound socket to the same interface). Currently, the function prematurely drops the old ECU references by calling j1939_local_ecu_put() before verifying whether the new configuration can be successfully acquired via j1939_local_ecu_get(). If j1939_local_ecu_get() subsequently fails, the function unconditionally calls j1939_netdev_stop() and clears jsk->priv. This leaves the socket in a half-broken state where the old ECU's refcount has already been decremented incompletely, but the socket destruct pathway (j1939_sk_sock_destruct) can no longer perform proper cleanup because jsk->priv is NULL. As a result, the old "struct j1939_ecu" remains orphaned on the priv->ecus list, permanently leaking both the ECU object and the net_device reference held inside it. Fix this by deferring the removal and release of the old ECU references until after j1939_local_ecu_get() has successfully acquired the new resources. As a side effect of this change, the socket's state no longer changes when the re-bind operation failed. Reported-by: syzbot+e2af46126e0644cbebdd@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=e2af46126e0644cbebdd Assisted-by: Gemini-Pro Fixes: f214744c8a27 ("can: j1939: j1939_sk_bind(): call j1939_priv_put() immediately when j1939_local_ecu_get() failed") Signed-off-by: Tetsuo Handa Acked-by: Oleksij Rempel Link: https://patch.msgid.link/deb3ac27-5eaf-406a-9bf4-733cc43ecaba@I-love.SAKURA.ne.jp Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- net/can/j1939/socket.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c index 50a598ef5fd4..24efb25c58c3 100644 --- a/net/can/j1939/socket.c +++ b/net/can/j1939/socket.c @@ -450,6 +450,7 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, in struct sock *sk; struct net *net; int ret = 0; + bool was_bound; ret = j1939_sk_sanity_check(addr, len); if (ret) @@ -462,7 +463,8 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, in net = sock_net(sk); /* Already bound to an interface? */ - if (jsk->state & J1939_SOCK_BOUND) { + was_bound = (jsk->state & J1939_SOCK_BOUND); + if (was_bound) { /* A re-bind() to a different interface is not * supported. */ @@ -470,10 +472,6 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, in ret = -EINVAL; goto out_release_sock; } - - /* drop old references */ - j1939_jsk_del(priv, jsk); - j1939_local_ecu_put(priv, jsk->addr.src_name, jsk->addr.sa); } else { struct can_ml_priv *can_ml; struct net_device *ndev; @@ -519,22 +517,31 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, in jsk->priv = priv; } + /* get new references without dropping old references */ + ret = j1939_local_ecu_get(priv, addr->can_addr.j1939.name, addr->can_addr.j1939.addr); + if (ret) { + /* nothing to undo if re-bind() failed */ + if (!was_bound) { + j1939_netdev_stop(priv); + jsk->priv = NULL; + synchronize_rcu(); + j1939_priv_put(priv); + } + goto out_release_sock; + } + + /* drop old references after re-bind() succeeded */ + if (was_bound) { + j1939_jsk_del(priv, jsk); + j1939_local_ecu_put(priv, jsk->addr.src_name, jsk->addr.sa); + } + /* set default transmit pgn */ if (j1939_pgn_is_valid(addr->can_addr.j1939.pgn)) jsk->pgn_rx_filter = addr->can_addr.j1939.pgn; jsk->addr.src_name = addr->can_addr.j1939.name; jsk->addr.sa = addr->can_addr.j1939.addr; - /* get new references */ - ret = j1939_local_ecu_get(priv, jsk->addr.src_name, jsk->addr.sa); - if (ret) { - j1939_netdev_stop(priv); - jsk->priv = NULL; - synchronize_rcu(); - j1939_priv_put(priv); - goto out_release_sock; - } - j1939_jsk_add(priv, jsk); out_release_sock: /* fall through */ -- 2.53.0 From: Kaixuan Li isotp_rcv() separates Classic CAN from CAN FD by skb->len alone: if (skb->len != so->ll.mtu) return; cf = (struct canfd_frame *)skb->data; A CAN XL frame with cxl->len 4 is CAN_MTU bytes, so it passes, and is then read as a canfd_frame whose len comes out of canxl_frame.flags: at least 0x80. Of the paths that follow, only the flow control one uses that length without bounding it first, so check_pad() walks to 255 over a 16-byte frame and the caller reports EBADMSG on an unrelated socket. bcm_rx_handler(), j1939_can_recv(), can_can_gw_rcv() and raw_rcv() check the frame type here, and can_dropped_invalid_skb() switches on skb->protocol on the transmit side. isotp_rcv() is the gap. Fixes: fb08cba12b52 ("can: canxl: update CAN infrastructure for CAN XL frames") Signed-off-by: Kaixuan Li Reviewed-by: Oliver Hartkopp Acked-by: Oliver Hartkopp Reviewed-by: Quchaosheng Link: https://patch.msgid.link/20260920035626.2581040-1-kaixuanli0131@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- net/can/isotp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/net/can/isotp.c b/net/can/isotp.c index 155530aedce2..1e80eb2a6bae 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -755,6 +755,14 @@ static void isotp_rcv(struct sk_buff *skb, void *data) if (skb->len != so->ll.mtu) return; + /* check for correct CAN CC/FD frame content */ + if (so->ll.mtu == CAN_MTU) { + if (!can_is_can_skb(skb)) + return; + } else if (!can_is_canfd_skb(skb)) { + return; + } + cf = (struct canfd_frame *)skb->data; /* if enabled: check reception of my configured extended address */ -- 2.53.0 From: Quchaosheng cc770_platform_probe() turns every failure of platform_get_irq() into -ENODEV, including -EPROBE_DEFER. Return the error unchanged so that a deferred probe stays deferred, and keep -ENODEV for the case where no interrupt is described at all. The memory resource is checked separately for the same reason, so that a missing resource is still reported as -ENODEV rather than being confused with an interrupt lookup failure. The same pattern is used by other CAN platform drivers, such as c_can_platform.c and flexcan-core.c. Signed-off-by: Quchaosheng Link: https://patch.msgid.link/20260914055611.495105-1-quchaosheng000406@163.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/cc770/cc770_platform.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c index b6c4f02ffb97..ac218acedb10 100644 --- a/drivers/net/can/cc770/cc770_platform.c +++ b/drivers/net/can/cc770/cc770_platform.c @@ -156,8 +156,13 @@ static int cc770_platform_probe(struct platform_device *pdev) int err, irq; mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!mem) + return -ENODEV; + irq = platform_get_irq(pdev, 0); - if (!mem || irq <= 0) + if (irq < 0) + return irq; + if (!irq) return -ENODEV; mem_size = resource_size(mem); -- 2.53.0 From: Quchaosheng cc770_get_platform_data() tests priv->cpu_interface for CPUIF_DSC before assigning it from pdata->cir: priv->can.clock.freq = pdata->osc_freq; if (priv->cpu_interface & CPUIF_DSC) priv->can.clock.freq /= 2; priv->clkout = pdata->cor; priv->bus_config = pdata->bcr; priv->cpu_interface = pdata->cir; priv comes from alloc_cc770dev() -> alloc_candev() -> alloc_netdev_mqs(), which uses kvzalloc_flex(), so cpu_interface is still zero here and the test can never match. A board that sets CPUIF_DSC in pdata->cir therefore gets its system clock halved in the hardware but not in can.clock.freq, and the bit timing is computed from twice the real clock. The platform data example in the file header, .cir = 0x41, sets that bit. Assign pdata->cir before the test, as the ISA path in cc770_isa.c already does. Fixes: e285e44d91fe ("can: cc770: add platform bus driver for the CC770 and AN82527") Assisted-by: LLM Signed-off-by: Quchaosheng Link: https://patch.msgid.link/20260917114311.534863-1-quchaosheng000406@163.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/cc770/cc770_platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c index ac218acedb10..c7dfcff4e1ce 100644 --- a/drivers/net/can/cc770/cc770_platform.c +++ b/drivers/net/can/cc770/cc770_platform.c @@ -137,11 +137,11 @@ static int cc770_get_platform_data(struct platform_device *pdev, struct cc770_platform_data *pdata = dev_get_platdata(&pdev->dev); priv->can.clock.freq = pdata->osc_freq; + priv->cpu_interface = pdata->cir; if (priv->cpu_interface & CPUIF_DSC) priv->can.clock.freq /= 2; priv->clkout = pdata->cor; priv->bus_config = pdata->bcr; - priv->cpu_interface = pdata->cir; return 0; } -- 2.53.0 From: Fan Wu The bec poll timer is rearmed from the interrupt handler, so the timer_delete() call in kvaser_pciefd_remove() neither waits for a callback that is already running nor stops the handler from rearming the timer until the interrupt is freed later in the same function. The timer can therefore still be pending or running when free_candev() frees the CAN device, causing a use-after-free in kvaser_pciefd_bec_poll_timer(). Use timer_shutdown_sync() instead, which waits for a running callback and makes a later rearm a no-op. Also drain the timer in kvaser_pciefd_teardown_can_ctrls(), which frees the CAN devices on the probe error paths. This issue was found by an in-house static analysis tool. Fixes: 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices") Cc: stable@vger.kernel.org Assisted-by: Codex:gpt-5.6 Signed-off-by: Fan Wu Link: https://patch.msgid.link/20260818063832.383829-1-fanwu01@zju.edu.cn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c b/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c index d8c9bfb20230..a0597db72086 100644 --- a/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c +++ b/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c @@ -1739,6 +1739,7 @@ static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie) iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG); kvaser_pciefd_pwm_stop(can); kvaser_pciefd_devlink_port_unregister(can); + timer_shutdown_sync(&can->bec_poll_timer); free_candev(can->can.dev); } } @@ -1879,7 +1880,7 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev) struct kvaser_pciefd_can *can = pcie->can[i]; unregister_candev(can->can.dev); - timer_delete(&can->bec_poll_timer); + timer_shutdown_sync(&can->bec_poll_timer); kvaser_pciefd_pwm_stop(can); kvaser_pciefd_devlink_port_unregister(can); } -- 2.53.0 From: Joshua Crofts m_can_pci_remove() forgets to call pm_runtime_dont_use_autosuspend() on teardown, even though autosuspend is enabled in m_can_pci_probe(). Add the missing function call. Signed-off-by: Joshua Crofts Acked-by: Markus Schneider-Pargmann Fixes: cab7ffc0324f ("can: m_can: add PCI glue driver for Intel Elkhart Lake") Link: https://patch.msgid.link/20260916124236.1389-1-joshua.crofts1@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c index d11a7c88fc32..3c595749b5c7 100644 --- a/drivers/net/can/m_can/m_can_pci.c +++ b/drivers/net/can/m_can/m_can_pci.c @@ -159,6 +159,7 @@ static void m_can_pci_remove(struct pci_dev *pci) struct m_can_pci_priv *priv = cdev_to_priv(mcan_class); pm_runtime_forbid(&pci->dev); + pm_runtime_dont_use_autosuspend(&pci->dev); pm_runtime_get_noresume(&pci->dev); /* Disable interrupt control at CAN wrapper IP */ -- 2.53.0 From: "Markus Schneider-Pargmann (TI)" When suspending mcan, deinit is called and its return value is returned, but nothing is restored. Returning an error in the suspend function will stop suspending and resume the system immediately. So on error the device should be restored to its previous state. Fixes: ad1ddb3bfb0c ("can: m_can: call deinit/init callback when going into suspend/resume") Signed-off-by: Markus Schneider-Pargmann (TI) Reviewed-by: Kendall Willis Link: https://patch.msgid.link/20260918-v7-3-topic-mcan-suspend-fix-fix-v1-1-e24fa70c754e@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 16f80607e150..91a0c5eca260 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -2612,8 +2612,14 @@ int m_can_class_suspend(struct device *dev) hrtimer_cancel(&cdev->hrtimer); m_can_write(cdev, M_CAN_IE, IR_RF0N); - if (cdev->ops->deinit) + if (cdev->ops->deinit) { ret = cdev->ops->deinit(cdev); + if (ret) { + netdev_err(cdev->net, "failed to deinit device while suspending %pe\n", + ERR_PTR(ret)); + goto err_restore_interface; + } + } } else { m_can_stop(ndev); } @@ -2625,6 +2631,21 @@ int m_can_class_suspend(struct device *dev) if (!m_can_class_wakeup_pinctrl_enabled(cdev)) pinctrl_pm_select_sleep_state(dev); + return 0; + +err_restore_interface: + if (netif_running(ndev)) { + if (cdev->pm_wake_source) { + /* Enable interrupts that trigger immediately if + * something is there and keep the hrtimer off + */ + cdev->active_interrupts |= IR_RF0N | IR_TEFN; + m_can_write(cdev, M_CAN_IE, cdev->active_interrupts); + } + netif_device_attach(ndev); + netif_start_queue(ndev); + } + return ret; } EXPORT_SYMBOL_GPL(m_can_class_suspend); -- 2.53.0 From: Wentao Liang In sun4ican_probe(), the clock is obtained with of_clk_get(), which returns a reference that must be released with clk_put(). If platform_get_irq(), devm_platform_ioremap_resource(), alloc_candev() or register_candev() fails, the reference is never released, leaking the clock. Fix this by adding an exit_put_clk label that releases the clock and jumping to it from all error paths after the of_clk_get() call. Fixes: 0738eff14d81 ("can: Allwinner A10/A20 CAN Controller support - Kernel module") Cc: stable@vger.kernel.org Signed-off-by: Wentao Liang Link: https://patch.msgid.link/20260917104514.2147347-1-vulab@iscas.ac.cn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/sun4i_can.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c index af52285d5a4e..311526107ed8 100644 --- a/drivers/net/can/sun4i_can.c +++ b/drivers/net/can/sun4i_can.c @@ -854,13 +854,13 @@ static int sun4ican_probe(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq < 0) { err = -ENODEV; - goto exit; + goto exit_put_clk; } addr = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(addr)) { err = PTR_ERR(addr); - goto exit; + goto exit_put_clk; } dev = alloc_candev(sizeof(struct sun4ican_priv), 1); @@ -868,7 +868,7 @@ static int sun4ican_probe(struct platform_device *pdev) dev_err(&pdev->dev, "could not allocate memory for CAN device\n"); err = -ENOMEM; - goto exit; + goto exit_put_clk; } dev->netdev_ops = &sun4ican_netdev_ops; @@ -908,6 +908,8 @@ static int sun4ican_probe(struct platform_device *pdev) exit_free: free_candev(dev); +exit_put_clk: + clk_put(clk); exit: return err; } -- 2.53.0 From: Maximilian Zimmermann The Xilinx CAN FD controller reports the bit rate switch (BRS) and error state indicator (ESI) in the receive buffer DLC register. The receive path currently uses this register to determine frame format and payload length, but does not set BRS and ESI in struct canfd_frame::flags. This results in applications being unable to receive the BRS and ESI flags, even when the controller correctly received them. Add the ESI register mask and copy the controller flags to the corresponding SocketCAN canfd_frame struct. Fixes: c223da689324 ("can: xilinx_can: Add support for CANFD FD frames") Cc: stable@vger.kernel.org Signed-off-by: Maximilian Zimmermann Link: https://patch.msgid.link/20260921-fix-xilinx-canfd-flags-v1-1-a371c90b4e7c@spacecubics.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/xilinx_can.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c index 43d7f22820b8..52661e088205 100644 --- a/drivers/net/can/xilinx_can.c +++ b/drivers/net/can/xilinx_can.c @@ -157,6 +157,7 @@ enum xcan_reg { #define XCAN_2_FSR_RI_MASK 0x0000003F /* RX Read Index */ #define XCAN_DLCR_EDL_MASK 0x08000000 /* EDL Mask in DLC */ #define XCAN_DLCR_BRS_MASK 0x04000000 /* BRS Mask in DLC */ +#define XCAN_DLCR_ESI_MASK 0x02000000 /* ESI Mask in DLC */ #define XCAN_ECC_CFG_REECRX_MASK BIT(2) /* Reset RX FIFO ECC error counters */ #define XCAN_ECC_CFG_REECTXOL_MASK BIT(1) /* Reset TXOL FIFO ECC error counters */ #define XCAN_ECC_CFG_REECTXTL_MASK BIT(0) /* Reset TXTL FIFO ECC error counters */ @@ -959,6 +960,11 @@ static int xcanfd_rx(struct net_device *ndev, int frame_base) /* Check the frame received is FD or not*/ if (dlc & XCAN_DLCR_EDL_MASK) { + if (dlc & XCAN_DLCR_BRS_MASK) + cf->flags |= CANFD_BRS; + if (dlc & XCAN_DLCR_ESI_MASK) + cf->flags |= CANFD_ESI; + for (i = 0; i < cf->len; i += 4) { dw_offset = XCANFD_FRAME_DW_OFFSET(frame_base) + (dwindex * XCANFD_DW_BYTES); -- 2.53.0 From: Jiale Yao A device bound through driver_override need not match an entry in the driver tables. In that case spi_get_device_match_data() returns NULL, but mcp251xfd_probe() dereferences the result while copying the device type data. Reject devices without match data before continuing probe. Fixes: 9cdae370c4ec ("can: mcp251xfd: simplify with spi_get_device_match_data()") Signed-off-by: Jiale Yao Link: https://patch.msgid.link/20260925133716.2230252-1-yaojiale02@163.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index f441f2265299..8759bc05bd8f 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -2213,6 +2213,7 @@ MODULE_DEVICE_TABLE(spi, mcp251xfd_id_table); static int mcp251xfd_probe(struct spi_device *spi) { + const struct mcp251xfd_devtype_data *devtype_data; struct net_device *ndev; struct mcp251xfd_priv *priv; struct gpio_desc *rx_int; @@ -2222,6 +2223,10 @@ static int mcp251xfd_probe(struct spi_device *spi) u32 freq = 0; int err; + devtype_data = spi_get_device_match_data(spi); + if (!devtype_data) + return -ENODATA; + if (!spi->irq) return dev_err_probe(&spi->dev, -ENXIO, "No IRQ specified (maybe node \"interrupts-extended\" in DT missing)!\n"); @@ -2307,7 +2312,7 @@ static int mcp251xfd_probe(struct spi_device *spi) priv->reg_vdd = reg_vdd; priv->reg_xceiver = reg_xceiver; priv->xstbyen = device_property_present(&spi->dev, "microchip,xstbyen"); - priv->devtype_data = *(struct mcp251xfd_devtype_data *)spi_get_device_match_data(spi); + priv->devtype_data = *devtype_data; /* Errata Reference: * mcp2517fd: DS80000792C 5., mcp2518fd: DS80000789E 4., -- 2.53.0 From: Runyu Xiao hi3110_open() requests a threaded IRQ and then performs hardware setup while holding priv->hi3110_lock. If reset, setup, or normal-mode entry fails, the error path calls free_irq() while still holding that mutex. The threaded handler takes priv->hi3110_lock before checking force_quit, while free_irq() waits for the threaded handler to finish. That can deadlock the open() rollback path against a pending IRQ thread. Set force_quit, drop hi3110_lock before free_irq(), and take the mutex again for the remaining hardware cleanup. Fixes: 57e83fb9b746 ("can: hi311x: Add Holt HI-311x CAN driver") Cc: stable@vger.kernel.org Signed-off-by: Runyu Xiao Link: https://patch.msgid.link/20260820020631.316418-1-runyu.xiao@seu.edu.cn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/hi311x.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c index ae90e6716de5..2be851e8907d 100644 --- a/drivers/net/can/spi/hi311x.c +++ b/drivers/net/can/spi/hi311x.c @@ -787,7 +787,10 @@ static int hi3110_open(struct net_device *net) return 0; out_free_irq: + priv->force_quit = 1; + mutex_unlock(&priv->hi3110_lock); free_irq(spi->irq, priv); + mutex_lock(&priv->hi3110_lock); hi3110_hw_sleep(spi); out_close: hi3110_power_enable(priv->transceiver, 0); -- 2.53.0 From: Fan Wu The intr URB submitted in ems_usb_start() is not anchored, and its completion handler ems_usb_read_interrupt_callback() resubmits it, so it stays in flight as long as the interface is up. But unlink_all_urbs() stops this URB with usb_unlink_urb(), which only initiates an asynchronous unlink and returns without waiting for the handler. The handler can therefore still be running while ems_usb_disconnect() frees its data: it reads the transfer buffer dev->intr_in_buffer, which is kfree()d there, and dereferences the private context, which is released via free_candev() together with the network device. Fix this by stopping the intr URB with usb_kill_urb(), which waits until the handler has returned, so the frees in ems_usb_disconnect() happen strictly after the last callback. The handler treats the -ENOENT completion of a killed URB as terminal and does not take RTNL or any sleeping lock, so the resubmit loop is cut and no RTNL deadlock occurs. This issue was found by an in-house static analysis tool. Fixes: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface") Cc: stable@vger.kernel.org Co-developed-by: Song Li Signed-off-by: Song Li Signed-off-by: Fan Wu Link: https://patch.msgid.link/20260923030522.409344-1-fanwu01@zju.edu.cn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/ems_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c index 24cf8f651f8f..2d31f0b86859 100644 --- a/drivers/net/can/usb/ems_usb.c +++ b/drivers/net/can/usb/ems_usb.c @@ -748,7 +748,7 @@ static void unlink_all_urbs(struct ems_usb *dev) { int i; - usb_unlink_urb(dev->intr_urb); + usb_kill_urb(dev->intr_urb); usb_kill_anchored_urbs(&dev->rx_submitted); -- 2.53.0 From: "Ji-Ze Hong (Peter Hong)" The struct f81604_int_data defines 9 bytes of interrupt data: - Byte 0: Status register (sr) - Byte 1: Interrupt register (isrc) - Byte 2: Interrupt enable register (ier) - Byte 3: Arbitration lost capture (alc) - Byte 4: Error code capture (ecc) - Byte 5: Error warning limit register (ewlr) - Byte 6: RX error counter (rxerr) - Byte 7: TX error counter (txerr) - Byte 8: Reserved (val) The hardware sends exactly 9 bytes for the interrupt endpoint. However, the struct was defined with __aligned(4) attribute which caused the compiler to pad the struct to 12 bytes. This causes a problem in f81604_read_int_callback() where the short URB check compares urb->actual_length against sizeof(*data). When sizeof(struct f81604_int_data) is 12 but the hardware only sends 9 bytes, the check fails and valid interrupt messages are discarded. This results in the driver only being able to transmit once because the TX complete interrupt is never processed. Fix this by removing the __aligned(4) attribute so the struct size matches the actual hardware data size of 9 bytes. Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly") Cc: stable@vger.kernel.org Reported-by: Dynetrex, Admin Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/ Signed-off-by: Ji-Ze Hong (Peter Hong) Acked-by: Greg Kroah-Hartman Tested-by: Drew Willey Link: https://patch.msgid.link/20260824-f81604-fix-v2-1-fc9be5581394@fintek.com.tw Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/f81604.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/f81604.c b/drivers/net/can/usb/f81604.c index f12318268e46..4c147b9d6d69 100644 --- a/drivers/net/can/usb/f81604.c +++ b/drivers/net/can/usb/f81604.c @@ -169,7 +169,7 @@ struct f81604_int_data { u8 rxerr; u8 txerr; u8 val; -} __packed __aligned(4); +} __packed; struct f81604_sff { __be16 id; -- 2.53.0 From: Fan Wu gs_usb_disconnect() destroys the channels one by one via gs_destroy_candev()/free_candev(). gs_can_close() disposes the RX bulk URBs on the shared parent->rx_submitted anchor only when the last active channel is closed. With two or more channels up, the earlier channels are freed while their RX URBs are still submitted, and a completion in gs_usb_receive_bulk_callback() accesses the freed struct gs_can and struct net_device. Fix this by killing the anchored RX URBs in gs_usb_disconnect() before the first netdev is destroyed, and in the error path of gs_usb_probe() before the previously created netdevs are destroyed. usb_kill_anchored_urbs() waits for running completions and a killed URB completes with -ENOENT, so the completion handler returns without resubmitting the URB. The kill in gs_can_close() of the last active channel then operates on an already empty anchor. This issue was found by an in-house static analysis tool. Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices") Cc: stable@vger.kernel.org Co-developed-by: Song Li Signed-off-by: Song Li Signed-off-by: Fan Wu Link: https://patch.msgid.link/20260923070352.487595-1-fanwu01@zju.edu.cn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 3b9b2f104d86..f604358c8259 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -1595,10 +1595,10 @@ static int gs_usb_probe(struct usb_interface *intf, /* on failure destroy previously created candevs */ icount = i; + usb_kill_anchored_urbs(&parent->rx_submitted); for (i = 0; i < icount; i++) gs_destroy_candev(parent->canch[i]); - usb_kill_anchored_urbs(&parent->rx_submitted); kfree(parent); return rc; } @@ -1636,6 +1636,8 @@ static void gs_usb_disconnect(struct usb_interface *intf) return; } + usb_kill_anchored_urbs(&parent->rx_submitted); + for (i = 0; i < parent->channel_cnt; i++) if (parent->canch[i]) gs_destroy_candev(parent->canch[i]); -- 2.53.0 The HScanT [1] is a RISC-V based USB to 4 channels CAN-FD adapter, which is compatible with the gs_usb protocol. The device is shipped with firmware version 0x00010007 and needs several quirks to work properly. The HScanT FW announces 5 channels, but the hardware has only 4. Workaround the problem by changing the struct gs_device_config::icount to 3, which corresponds to 4 channels. The HScanT FW requires a USB High Speed Hub, bail out if device is connected to slower USB Hub. The HScanT FW always sends USB In URB with length 512 bytes and seems to have broken Zero Packet Length handling. Add quirk GS_CAN_FEATURE_QUIRK_HSCANT_URB_SIZE to allocate URBs of 513 bytes to work around these issues. This driver supports up to 256 channels per USB Interface. The HScanT device has 4 channels, but the FW requires each channels to be bound to a USB interface using the GS_USB_BREQ_HSCANT_SET_INTERFACENUMBER_ENDPOINT USB request. Add quirk to GS_CAN_FEATURE_QUIRK_HSCANT_BIND_CHANNEL to bind the CAN channel to the USB Interface 0 during gs_can_open() Link: https://github.com/cherry-embedded/HSCanT-hardware Link: https://patch.msgid.link/20260928-gs_usb-hscant-v2-1-a7c1c02460e9@pengutronix.de Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 125 +++++++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index f604358c8259..f7a349902c42 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -72,6 +72,7 @@ enum gs_usb_breq { GS_USB_BREQ_SET_TERMINATION, GS_USB_BREQ_GET_TERMINATION, GS_USB_BREQ_GET_STATE, + GS_USB_BREQ_HSCANT_SET_INTERFACENUMBER_ENDPOINT = 17, }; enum gs_can_mode { @@ -188,6 +189,21 @@ struct gs_device_termination_state { /* internal quirks - keep in GS_CAN_FEATURE space for now */ +/* HScanT firmware version 0x00010007: + * - FW requires the binding of CAN channels to USB Interfaces. + * - Route all CAN channels to USB Interface 0. + */ +#define GS_CAN_FEATURE_QUIRK_HSCANT_BIND_CHANNEL BIT(29) + +/* HScanT firmware version 0x00010007: + * - FW sends bulk In URBs with length of 512 bytes. + * - When using In URBs with 512 bytes it will send a second in URB with length 0 + * It seems the ZLP handling is broken. + * - Use In URBs of length GS_USB_QUIRK_HSCANT_IN_URB_SIZE as a workaround. + */ +#define GS_CAN_FEATURE_QUIRK_HSCANT_URB_SIZE BIT(30) +#define GS_USB_QUIRK_HSCANT_IN_URB_SIZE (513) + /* CANtact Pro original firmware: * BREQ DATA_BITTIMING overlaps with GET_USER_ID */ @@ -812,6 +828,18 @@ static int gs_usb_set_data_bittiming(struct gs_can *dev) GFP_KERNEL); } +static int gs_usb_hscant_bind_channel_to_interface(const struct gs_can *dev) +{ + const u16 interface_number = 0; + + /* Bind dev->channel to interface_number */ + return usb_control_msg_send(dev->udev, 0, GS_USB_BREQ_HSCANT_SET_INTERFACENUMBER_ENDPOINT, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, interface_number, + NULL, 0, 1000, + GFP_KERNEL); +} + static void gs_usb_xmit_callback(struct urb *urb) { struct gs_tx_context *txc = urb->context; @@ -1069,6 +1097,16 @@ static int gs_can_open(struct net_device *netdev) } } + if (dev->feature & GS_CAN_FEATURE_QUIRK_HSCANT_BIND_CHANNEL) { + rc = gs_usb_hscant_bind_channel_to_interface(dev); + if (rc) { + netdev_err(netdev, + "failed to bind Channel to Interface: %pe\n", + ERR_PTR(rc)); + goto out_usb_kill_anchored_urbs; + } + } + /* finally start device */ dev->can.state = CAN_STATE_ERROR_ACTIVE; dm.flags = cpu_to_le32(flags); @@ -1314,6 +1352,49 @@ static const u16 gs_usb_termination_const[] = { GS_USB_TERMINATION_ENABLED }; +static bool gs_usb_is_hscant(const struct usb_device *udev, + const struct gs_device_config *dconf, + const u32 sw_version) +{ + if (udev->descriptor.idVendor != cpu_to_le16(USB_GS_USB_1_VENDOR_ID) || + udev->descriptor.idProduct != cpu_to_le16(USB_GS_USB_1_PRODUCT_ID)) + return false; + + if (strcmp(udev->manufacturer, "HScanT") || + strcmp(udev->product, "HScanT USB to CAN adapter")) + return false; + + if (dconf->sw_version != cpu_to_le32(sw_version)) + return false; + + return true; +} + +static void +gs_usb_make_candev_get_feature(struct gs_can *dev, const struct gs_device_config *dconf, + const struct gs_device_bt_const *bt_const) +{ + const struct usb_device *udev = dev->udev; + const u32 feature = le32_to_cpu(bt_const->feature); + + dev->feature = FIELD_GET(GS_CAN_FEATURE_MASK, feature); + + if (!udev->manufacturer || !udev->product) + return; + + /* HScanT firmware version 0x00010007: + * - FW doesn't advertise GS_CAN_FEATURE_BT_CONST_EXT, + * but implements GS_USB_BREQ_BT_CONST_EXT, fixup. + * - FW requires binding of CAN channel to USB Interface, add quirk. + * - FW requires bulk In URBs with >= 512 bytes, add quirk. + */ + if (gs_usb_is_hscant(udev, dconf, 0x00010007)) { + dev->feature |= GS_CAN_FEATURE_BT_CONST_EXT | + GS_CAN_FEATURE_QUIRK_HSCANT_BIND_CHANNEL | + GS_CAN_FEATURE_QUIRK_HSCANT_URB_SIZE; + } +} + static struct gs_can *gs_make_candev(unsigned int channel, struct usb_interface *intf, struct gs_device_config *dconf) @@ -1385,8 +1466,9 @@ static struct gs_can *gs_make_candev(unsigned int channel, dev->can.ctrlmode_supported = CAN_CTRLMODE_CC_LEN8_DLC; - feature = le32_to_cpu(bt_const.feature); - dev->feature = FIELD_GET(GS_CAN_FEATURE_MASK, feature); + gs_usb_make_candev_get_feature(dev, dconf, &bt_const); + feature = dev->feature; + if (feature & GS_CAN_FEATURE_LISTEN_ONLY) dev->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY; @@ -1514,6 +1596,34 @@ static void gs_destroy_candev(struct gs_can *dev) free_candev(dev->netdev); } +static int gs_usb_probe_quirks(const struct usb_interface *intf, struct gs_device_config *dconf) +{ + const struct usb_device *udev = interface_to_usbdev(intf); + + if (!udev->manufacturer || !udev->product) + return 0; + + /* HScanT firmware version 0x00010007: + * - FW has an icount of 4, which corresponds to 5 CAN interfaces. + * The hardware has only 4 interfaces, fixup. + * - FW provides broken Endpoint Descriptors on USB Full Speed Hubs: + * config 1 interface 0 altsetting 0 endpoint 0x4 has invalid maxpacket 512, setting to 64 + * Probably related to GS_CAN_FEATURE_QUIRK_HSCANT_URB_SIZE, + * FW only works on USB High Speed Hubs, detect and bail out. + */ + if (gs_usb_is_hscant(udev, dconf, 0x00010007)) { + if (dconf->icount == 4) + dconf->icount = 3; + + if (udev->speed < USB_SPEED_HIGH) { + dev_err(&intf->dev, "Device only works with USB High Speed Hubs\n"); + return -ENODEV; + } + } + + return 0; +} + static int gs_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { @@ -1560,6 +1670,10 @@ static int gs_usb_probe(struct usb_interface *intf, return rc; } + rc = gs_usb_probe_quirks(intf, &dconf); + if (rc) + return rc; + icount = dconf.icount + 1; dev_info(&intf->dev, "Configuring for %u interfaces\n", icount); @@ -1604,10 +1718,13 @@ static int gs_usb_probe(struct usb_interface *intf, } parent->canch[i]->parent = parent; - /* set RX packet size based on FD and if hardware + /* set RX packet size based on quirks, FD and if hardware * timestamps are supported. */ - if (parent->canch[i]->can.ctrlmode_supported & CAN_CTRLMODE_FD) { + if (parent->canch[i]->feature & GS_CAN_FEATURE_QUIRK_HSCANT_URB_SIZE) { + hf_size_rx = GS_USB_QUIRK_HSCANT_IN_URB_SIZE; + BUILD_BUG_ON(struct_size(hf, canfd, 1) > GS_USB_QUIRK_HSCANT_IN_URB_SIZE); + } else if (parent->canch[i]->can.ctrlmode_supported & CAN_CTRLMODE_FD) { if (parent->canch[i]->feature & GS_CAN_FEATURE_HW_TIMESTAMP) hf_size_rx = struct_size(hf, canfd_ts, 1); else -- 2.53.0 From: "Cen Zhang (Microsoft Security FORGE Labs)" The receive-path command parsers (kvaser_usb_hydra_wait_cmd and kvaser_usb_hydra_read_bulk_callback) call kvaser_usb_hydra_cmd_size() without verifying that enough buffer remains. For CMD_EXTENDED, kvaser_usb_hydra_cmd_size() unconditionally reads a 2-byte len field at offset 4. A malicious USB device can place a CMD_EXTENDED header at the end of a 3072-byte bulk transfer such that only 4 bytes remain, causing a 2-byte slab-out-of-bounds read. BUG: KASAN: slab-out-of-bounds in kvaser_usb_hydra_wait_cmd+0x3f1/0x480 [kvaser_usb_hydra.c:678] Read of size 2 at addr ffff888013f7ec00 by task kworker/0:0/9 kvaser_usb_hydra_wait_cmd+0x3f1/0x480 kvaser_usb_hydra_get_software_details+0x1c7/0x5d0 kvaser_usb_probe+0x36a/0x1240 Additionally, if the device sends CMD_EXTENDED with len=0, kvaser_usb_hydra_cmd_size() returns 0 and the parser loops forever (pos += 0), permanently burning one CPU core. A positive but undersized extended length can also pass the buffer extent check and reach a handler. For example, an 8-byte CMD_RX_MESSAGE_FD at the end of an RX URB causes kvaser_usb_hydra_rx_msg_ext() to read fixed fields and payload past the buffer. Add receive-side length validation which preserves incomplete headers for reassembly, rejects extended lengths outside 8..128 bytes, and checks each known extended command against the minimum length its handler consumes. For CMD_RX_MESSAGE_FD, derive the required length from its flags and DLC so valid variable-length commands remain accepted. Apply the checks to both receive paths and clear malformed leftover state before returning. Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family") Reported-by: AutonomousCodeSecurity@microsoft.com Reported-by: Xiang Mei (Microsoft) Suggested-by: Jakub Kicinski Closes: https://lore.kernel.org/all/20260819145658.29872-1-blbllhy@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) Link: https://patch.msgid.link/20260910135000.34796-1-cenzhang@linux.microsoft.com [mkl: reduce scope of err in kvaser_usb_hydra_read_bulk_callback()] [mkl: increase readability, reformat to make use of ~100 columns] Signed-off-by: Marc Kleine-Budde --- .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 135 +++++++++++++++++- 1 file changed, 128 insertions(+), 7 deletions(-) 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 efbb7bed34c9..43405b1b6a3c 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c @@ -536,6 +536,82 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd) return ret; } +/* -EAGAIN means incomplete; -EINVAL rejects an invalid command length. */ +static int kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd, + size_t remaining, size_t *cmd_len) +{ + if (remaining < sizeof(cmd->header.cmd_no)) + return -EAGAIN; + + if (cmd->header.cmd_no == CMD_EXTENDED && + remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext)) + return -EAGAIN; + + *cmd_len = kvaser_usb_hydra_cmd_size(cmd); + if (cmd->header.cmd_no != CMD_EXTENDED) + return 0; + + if (*cmd_len < offsetof(struct kvaser_cmd_ext, rx_can) || + *cmd_len > KVASER_USB_HYDRA_MAX_CMD_LEN) + return -EINVAL; + + return 0; +} + +static int kvaser_usb_hydra_verify_cmd_size(const struct kvaser_cmd *cmd, + size_t cmd_len) +{ + const struct kvaser_cmd_ext *cmd_ext; + size_t min_len; + + if (cmd->header.cmd_no != CMD_EXTENDED) + return 0; + + cmd_ext = (const struct kvaser_cmd_ext *)cmd; + + /* Keep this switch in sync with kvaser_usb_hydra_handle_cmd_ext(). */ + switch (cmd_ext->cmd_no_ext) { + case CMD_TX_ACKNOWLEDGE_FD: + min_len = offsetof(struct kvaser_cmd_ext, tx_ack.timestamp) + + sizeof(cmd_ext->tx_ack.timestamp); + break; + + case CMD_RX_MESSAGE_FD: { + u32 flags; + + min_len = offsetof(struct kvaser_cmd_ext, rx_can.kcan_payload); + if (cmd_len < min_len) + return -EINVAL; + + flags = le32_to_cpu(cmd_ext->rx_can.flags); + if (flags & KVASER_USB_HYDRA_CF_FLAG_ERROR_FRAME) { + min_len += sizeof(cmd_ext->rx_can.err_frame_data); + } else if (!(flags & KVASER_USB_HYDRA_CF_FLAG_REMOTE_FRAME)) { + u32 kcan_header; + u8 dlc; + + kcan_header = le32_to_cpu(cmd_ext->rx_can.kcan_header); + dlc = (kcan_header & KVASER_USB_KCAN_DATA_DLC_MASK) >> + KVASER_USB_KCAN_DATA_DLC_SHIFT; + + if (flags & KVASER_USB_HYDRA_CF_FLAG_FDF) + min_len += can_fd_dlc2len(dlc); + else + min_len += can_cc_dlc2len(dlc); + } + break; + } + + default: + return 0; + } + + if (cmd_len < min_len) + return -EINVAL; + + return 0; +} + static struct kvaser_usb_net_priv * kvaser_usb_hydra_net_priv_from_cmd(const struct kvaser_usb *dev, const struct kvaser_cmd *cmd) @@ -675,8 +751,9 @@ static int kvaser_usb_hydra_wait_cmd(const struct kvaser_usb *dev, u8 cmd_no, size_t cmd_len; tmp_cmd = buf + pos; - cmd_len = kvaser_usb_hydra_cmd_size(tmp_cmd); - if (pos + cmd_len > actual_len) { + err = kvaser_usb_hydra_cmd_size_rx(tmp_cmd, actual_len - pos,&cmd_len); + if (err || pos + cmd_len > actual_len || + kvaser_usb_hydra_verify_cmd_size(tmp_cmd, cmd_len)) { dev_err_ratelimited(&dev->intf->dev, "Format error\n"); break; @@ -2120,27 +2197,60 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, spin_lock_irqsave(usb_rx_leftover_lock, irq_flags); usb_rx_leftover_len = card_data->usb_rx_leftover_len; if (usb_rx_leftover_len) { + const size_t cmd_size_field_end = offsetof(struct kvaser_cmd_ext, cmd_no_ext); int remaining_bytes; + int err; cmd = (struct kvaser_cmd *)card_data->usb_rx_leftover; - cmd_len = kvaser_usb_hydra_cmd_size(cmd); + if (cmd->header.cmd_no == CMD_EXTENDED && + usb_rx_leftover_len < cmd_size_field_end) { + remaining_bytes = min_t(int, len, cmd_size_field_end - usb_rx_leftover_len); - remaining_bytes = min_t(unsigned int, len, + memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len, buf, remaining_bytes); + usb_rx_leftover_len += remaining_bytes; + card_data->usb_rx_leftover_len = usb_rx_leftover_len; + pos += remaining_bytes; + + if (usb_rx_leftover_len < cmd_size_field_end) { + spin_unlock_irqrestore(usb_rx_leftover_lock, + irq_flags); + return; + } + } + + err = kvaser_usb_hydra_cmd_size_rx(cmd, usb_rx_leftover_len, + &cmd_len); + if (err || cmd_len < usb_rx_leftover_len) { + dev_err(&dev->intf->dev, "Format error\n"); + card_data->usb_rx_leftover_len = 0; + spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags); + return; + } + + remaining_bytes = min_t(unsigned int, len - pos, cmd_len - usb_rx_leftover_len); /* Make sure we do not overflow usb_rx_leftover */ if (remaining_bytes + usb_rx_leftover_len > KVASER_USB_HYDRA_MAX_CMD_LEN) { dev_err(&dev->intf->dev, "Format error\n"); + card_data->usb_rx_leftover_len = 0; spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags); return; } - memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len, buf, + memcpy(card_data->usb_rx_leftover + usb_rx_leftover_len, buf + pos, remaining_bytes); pos += remaining_bytes; if (remaining_bytes + usb_rx_leftover_len == cmd_len) { + if (kvaser_usb_hydra_verify_cmd_size(cmd, cmd_len)) { + dev_err(&dev->intf->dev, "Format error\n"); + card_data->usb_rx_leftover_len = 0; + spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags); + return; + } + kvaser_usb_hydra_handle_cmd(dev, cmd); usb_rx_leftover_len = 0; } else { @@ -2152,11 +2262,17 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags); while (pos < len) { + int err; + cmd = buf + pos; - cmd_len = kvaser_usb_hydra_cmd_size(cmd); + err = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos, &cmd_len); + if (err && err != -EAGAIN) { + dev_err(&dev->intf->dev, "Format error\n"); + return; + } - if (pos + cmd_len > len) { + if (err == -EAGAIN || pos + cmd_len > len) { /* We got first part of a command */ int leftover_bytes; @@ -2174,6 +2290,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev, break; } + if (kvaser_usb_hydra_verify_cmd_size(cmd, cmd_len)) { + dev_err(&dev->intf->dev, "Format error\n"); + return; + } + kvaser_usb_hydra_handle_cmd(dev, cmd); pos += cmd_len; } -- 2.53.0 From: Stefan Günther When the device reports error counters, the driver incorrectly uses an assignment (=) instead of a bitwise OR (|=) for CAN_ERR_CNT. This wipes out the CAN_ERR_FLAG and CAN_ERR_CRTL flags, causing the error frame to be sent to userspace as a regular CAN frame with ID 0x200. Fix this by using a bitwise OR to preserve the flags. Signed-off-by: Stefan Günther Link: https://patch.msgid.link/20260915134551.54038-1-stefangun@pm.me Cc: stable@vger.kernel.org Fixes: 3e5c291c7942 ("can: add CAN_ERR_CNT flag to notify availability of error counter") Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/peak_usb/pcan_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c index 8fd058c32856..785224e00c4e 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c @@ -526,7 +526,7 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n, /* Supply TX/RX error counters in case of * controller error. */ - cf->can_id = CAN_ERR_CNT; + cf->can_id |= CAN_ERR_CNT; cf->data[6] = mc->pdev->bec.txerr; cf->data[7] = mc->pdev->bec.rxerr; } -- 2.53.0