[PATCH net 2/3] can: gs_usb: gs_usb_receive_bulk_callback(): fix URB memory leak

[PATCH net 2/3] can: gs_usb: gs_usb_receive_bulk_callback(): fix URB memory leak

[Marc Kleine-Budde]

In gs_can_open(), the URBs for USB-in transfers are allocated, added to the
parent->rx_submitted anchor and submitted. In the complete callback
gs_usb_receive_bulk_callback(), the URB is processed and resubmitted. In
gs_can_close() the URBs are freed by calling
usb_kill_anchored_urbs(parent->rx_submitted).

However, this does not take into account that the USB framework unanchors
the URB before the complete function is called. This means that once an
in-URB has been completed, it is no longer anchored and is ultimately not
released in gs_can_close().

Fix the memory leak by anchoring the URB in the
gs_usb_receive_bulk_callback() to the parent->rx_submitted anchor.

Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260105-gs_usb-fix-memory-leak-v2-1-cc6ed6438034@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index a0233e550a5a..d093babbc320 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -751,6 +751,8 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 			  hf, parent->hf_size_rx,
 			  gs_usb_receive_bulk_callback, parent);
 
+	usb_anchor_urb(urb, &parent->rx_submitted);
+
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
 
 	/* USB failure take down all interfaces */
-- 
2.51.0

[PATCH net 3/3] can: ctucanfd: fix SSP_SRC in cases when bit-rate is higher than 1 MBit.

[Marc Kleine-Budde]

From: Ondrej Ille <ondrej.ille@gmail.com>

The Secondary Sample Point Source field has been
set to an incorrect value by some mistake in the
past

  0b01 - SSP_SRC_NO_SSP - SSP is not used.

for data bitrates above 1 MBit/s. The correct/default
value already used for lower bitrates is

  0b00 - SSP_SRC_MEAS_N_OFFSET - SSP position = TRV_DELAY
         (Measured Transmitter delay) + SSP_OFFSET.

The related configuration register structure is described
in section 3.1.46 SSP_CFG of the CTU CAN FD
IP CORE Datasheet.

The analysis leading to the proper configuration
is described in section 2.8.3 Secondary sampling point
of the datasheet.

The change has been tested on AMD/Xilinx Zynq
with the next CTU CN FD IP core versions:

 - 2.6 aka master in the "integration with Zynq-7000 system" test
   6.12.43-rt12+ #1 SMP PREEMPT_RT kernel with CTU CAN FD git
   driver (change already included in the driver repo)
 - older 2.5 snapshot with mainline kernels with this patch
   applied locally in the multiple CAN latency tester nightly runs
   6.18.0-rc4-rt3-dut #1 SMP PREEMPT_RT
   6.19.0-rc3-dut

The logs, the datasheet and sources are available at

 https://canbus.pages.fel.cvut.cz/

Signed-off-by: Ondrej Ille <ondrej.ille@gmail.com>
Signed-off-by: Pavel Pisa <pisa@fel.cvut.cz>
Link: https://patch.msgid.link/20260105111620.16580-1-pisa@fel.cvut.cz
Fixes: 2dcb8e8782d8 ("can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.")
Cc: stable@vger.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 1e6b9e3dc2fe..0ea1ff28dfce 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -310,7 +310,7 @@ static int ctucan_set_secondary_sample_point(struct net_device *ndev)
 		}
 
 		ssp_cfg = FIELD_PREP(REG_TRV_DELAY_SSP_OFFSET, ssp_offset);
-		ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x1);
+		ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x0);
 	}
 
 	ctucan_write32(priv, CTUCANFD_TRV_DELAY, ssp_cfg);
-- 
2.51.0

Re: [net,2/3] can: gs_usb: gs_usb_receive_bulk_callback(): fix URB memory leak

[Jakub Kicinski]

This is an AI-generated review of your patch.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
can: gs_usb: gs_usb_receive_bulk_callback(): fix URB memory leak

This patch fixes a memory leak by re-anchoring the URB in
gs_usb_receive_bulk_callback() before resubmission. The USB framework
unanchors URBs before calling the completion callback, so the URB needs to
be re-anchored to be tracked by usb_kill_anchored_urbs() in gs_can_close().

Link: https://patch.msgid.link/20260105-gs_usb-fix-memory-leak-v2-1-cc6ed6438034@pengutronix.de

> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index a0233e550a5a..d093babbc320 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -751,6 +751,8 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
>  			  hf, parent->hf_size_rx,
>  			  gs_usb_receive_bulk_callback, parent);
>
> +	usb_anchor_urb(urb, &parent->rx_submitted);
> +
>  	rc = usb_submit_urb(urb, GFP_ATOMIC);

If usb_submit_urb() fails here, the URB remains anchored but is never
submitted. When gs_can_close() later calls usb_kill_anchored_urbs(), it
iterates over anchored URBs and calls usb_kill_urb() on each one. For URBs
that were never successfully submitted, use_count is 0, so usb_kill_urb()
returns without unanchoring them. This causes usb_kill_anchored_urbs() to
loop infinitely on the same URB since the anchor list never becomes empty.

Other CAN USB drivers handle this by calling usb_unanchor_urb() when
submission fails. For example, peak_usb_read_bulk_callback() does:

    usb_anchor_urb(urb, &dev->rx_submitted);
    err = usb_submit_urb(urb, GFP_ATOMIC);
    if (!err)
        return;

    usb_unanchor_urb(urb);

Should similar error handling be added here to avoid potential hangs when
closing the device after a URB submission failure?

>
>  	/* USB failure take down all interfaces */
>  	if (rc == -ENODEV) {

[ ... ]

Re: [net,2/3] can: gs_usb: gs_usb_receive_bulk_callback(): fix URB memory leak

[Marc Kleine-Budde]

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

On 10.01.2026 14:38:36, Jakub Kicinski wrote:
> This is an AI-generated review of your patch.
> > diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> > index a0233e550a5a..d093babbc320 100644
> > --- a/drivers/net/can/usb/gs_usb.c
> > +++ b/drivers/net/can/usb/gs_usb.c
> > @@ -751,6 +751,8 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
> >  			  hf, parent->hf_size_rx,
> >  			  gs_usb_receive_bulk_callback, parent);
> >
> > +	usb_anchor_urb(urb, &parent->rx_submitted);
> > +
> >  	rc = usb_submit_urb(urb, GFP_ATOMIC);
>
> If usb_submit_urb() fails here, the URB remains anchored but is never
> submitted. When gs_can_close() later calls usb_kill_anchored_urbs(), it
> iterates over anchored URBs and calls usb_kill_urb() on each one. For URBs
> that were never successfully submitted, use_count is 0, so usb_kill_urb()
> returns without unanchoring them. This causes usb_kill_anchored_urbs() to
> loop infinitely on the same URB since the anchor list never becomes empty.

Good AI!

Here's the patch to fix the problem. I'll include this in my next PR.

| https://lore.kernel.org/all/20260116-can_usb-fix-reanchor-v1-1-9d74e7289225@pengutronix.de/

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: 488 bytes --]