netdev - Re: [net,2/7] can: gs_usb: gs_usb_receive_bulk_callback(): unanchor URL on usb_submit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260123-radiant-nice-gecko-6331ac-mkl@pengutronix.de>
Date: Fri, 23 Jan 2026 18:36:59 +0100
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Jakub Kicinski <kuba@...nel.org>
Cc: stable@...r.kernel.org, kernel@...gutronix.de, socketcan@...tkopp.net, 
	davem@...emloft.net, linux-can@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [net,2/7] can: gs_usb: gs_usb_receive_bulk_callback(): unanchor
 URL on usb_submit_urb() error

On 19.01.2026 10:19:03, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> can: gs_usb: gs_usb_receive_bulk_callback(): unanchor URL on usb_submit_urb() error
>
> This patch fixes an infinite loop in gs_can_close() by unanchoring the URB
> when usb_submit_urb() fails after re-anchoring. It also adds an error
> message for non-ENODEV/ESHUTDOWN failures.
>
> Link: https://patch.msgid.link/20260116-can_usb-fix-reanchor-v1-1-9d74e7289225@pengutronix.de
>
> > diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> > index d093babbc320..192338b481f2 100644
> > --- a/drivers/net/can/usb/gs_usb.c
> > +++ b/drivers/net/can/usb/gs_usb.c
> > @@ -754,6 +754,10 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
> >  	usb_anchor_urb(urb, &parent->rx_submitted);
> >
> >  	rc = usb_submit_urb(urb, GFP_ATOMIC);
> > +	if (!rc)
> > +		return;
> > +
> > +	usb_unanchor_urb(urb);
> >
> >  	/* USB failure take down all interfaces */
> >  	if (rc == -ENODEV) {
>
> [ ... ]
>
> > +	} else if (rc != -ESHUTDOWN && net_ratelimit()) {
> > +		netdev_info(netdev, "failed to re-submit IN URB: %pe\n",
> > +			    ERR_PTR(urb->status));
>
> Two issues with this error message:
>
> 1. The message prints urb->status, but shouldn't it print rc instead? The
>    variable rc holds the actual error from usb_submit_urb(), while urb->status
>    contains the completion status from the previous callback invocation (which
>    would typically be 0 for success since we only reach resubmit_urb on
>    successful URB completion). Other CAN USB drivers like usb_8dev print the
>    return value from usb_submit_urb() in similar error paths.
>
> 2. The local variable netdev is uninitialized when gs_usb_receive_bulk_callback()
>    reaches this code path via the short read case. The function declares netdev
>    at the top without initialization, and the short read check at the beginning
>    of the function jumps directly to resubmit_urb without ever assigning netdev.
>    If usb_submit_urb() then fails with an error other than -ENODEV or -ESHUTDOWN,
>    this code will pass the uninitialized netdev pointer to netdev_info().

Fixed with today's PR: https://lore.kernel.org/all/20260123173241.1026226-1-mkl@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   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)