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

[Jakub Kicinski <kuba@kernel.org>]

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) {

[ ... ]