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@p... 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 */
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@p...
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) {
[ ... ]
linux-stable-mirror@lists.linaro.org
-
Jakub Kicinski -
Marc Kleine-Budde