Re: [PATCH] can: gs_usb: kill RX URBs before destroying the netdevs

[Marc Kleine-Budde]

On 23.09.2026 07:03:52, Fan Wu wrote:
> 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@xxxxxxxxxxxxxxx
> Co-developed-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
>  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 ec9a7cb..663746d 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);

I've fixed the root cause of this problem. Do not register an interface,
before all interfaces are setup:

https://lore.kernel.org/all/20261026-gs_usb-fixes-v4-2-59e3b6360053@xxxxxxxxxxxxxx/

>  			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);
> +

The USB core disables all endpoints before calling gs_usb_disconnect(),
so this isn't needed.

>  	for (i = 0; i < parent->channel_cnt; i++)
>  		if (parent->canch[i])
>  			gs_destroy_candev(parent->canch[i]);
>
>
>

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: signature.asc
Description: PGP signature