[PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback

[PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback

[Greg Kroah-Hartman]

When submitting an urb, that is using the anchor pattern, it needs to be
anchored before submitting it otherwise it could be leaked if
usb_kill_anchored_urbs() is called.  This logic is correctly done
elsewhere in the driver, except in the read bulk callback so do that
here also.

Cc: Vincent Mailhol <mailhol@kernel.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/can/usb/etas_es58x/es58x_core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.c b/drivers/net/can/usb/etas_es58x/es58x_core.c
index 2d248deb69dc..b259f6109808 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_core.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
@@ -1461,12 +1461,18 @@ static void es58x_read_bulk_callback(struct urb *urb)
 	}
 
  resubmit_urb:
+	usb_anchor_urb(urb, &es58x_dev->rx_urbs);
 	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (!ret)
+		return;
+
+	usb_unanchor_urb(urb);
+
 	if (ret == -ENODEV) {
 		for (i = 0; i < es58x_dev->num_can_ch; i++)
 			if (es58x_dev->netdev[i])
 				netif_device_detach(es58x_dev->netdev[i]);
-	} else if (ret)
+	} else
 		dev_err_ratelimited(dev,
 				    "Failed resubmitting read bulk urb: %pe\n",
 				    ERR_PTR(ret));
-- 
2.53.0

Re: [PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback

[Vincent Mailhol]

On 23/02/2026 at 17:39, Greg Kroah-Hartman wrote:
> When submitting an urb, that is using the anchor pattern, it needs to be
> anchored before submitting it otherwise it could be leaked if
> usb_kill_anchored_urbs() is called.  This logic is correctly done
> elsewhere in the driver, except in the read bulk callback so do that
> here also.

I can explain why this bug went through the cracks. When I wrote this
driver, I was actually wondering whether I had to manually re-anchor the
urbs. I checked the documentation of usb_submit_urb() and
usb_fill_bulk_urb() but there was no mention that the urb will be
unachored by the framework before calling urb->complete(). And so, I
assumed that it would stay anchored. And I ended up just anchoring them
once at creation and then reuse them without paying attention to the
anchor any more.

Now that I see your patch, I had a look at the code source and can see
that indeed __usb_hcd_giveback_urb() will unanchor the urbs.

Anyway, thanks for the fix!

> Cc: Vincent Mailhol <mailhol@kernel.org>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Tested-by: Vincent Mailhol <mailhol@kernel.org>


Yours sincerely,
Vincent Mailhol

Re: [PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback

[Marc Kleine-Budde]

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

On 23.02.2026 17:39:20, Greg Kroah-Hartman wrote:
> When submitting an urb, that is using the anchor pattern, it needs to be
> anchored before submitting it otherwise it could be leaked if
> usb_kill_anchored_urbs() is called.  This logic is correctly done
> elsewhere in the driver, except in the read bulk callback so do that
> here also.
>
> Cc: Vincent Mailhol <mailhol@kernel.org>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: stable <stable@kernel.org>

FYI: checkpatch suggests:
Invalid email format for stable: 'stable <stable@kernel.org>', prefer 'stable@kernel.org'

> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied to linux-can, with preferred stable format.

Thanks,
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: 228 bytes --]

Re: [PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback

[Greg Kroah-Hartman]

On Mon, Mar 02, 2026 at 11:01:48AM +0100, Marc Kleine-Budde wrote:
> On 23.02.2026 17:39:20, Greg Kroah-Hartman wrote:
> > When submitting an urb, that is using the anchor pattern, it needs to be
> > anchored before submitting it otherwise it could be leaked if
> > usb_kill_anchored_urbs() is called.  This logic is correctly done
> > elsewhere in the driver, except in the read bulk callback so do that
> > here also.
> >
> > Cc: Vincent Mailhol <mailhol@kernel.org>
> > Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> > Cc: stable <stable@kernel.org>
> 
> FYI: checkpatch suggests:
> Invalid email format for stable: 'stable <stable@kernel.org>', prefer 'stable@kernel.org'

Checkpatch is wrong here, either is just fine.

thanks,

greg k-h

checkpatch: format of stable@k.o address (was: Re: [PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback)

[Marc Kleine-Budde]

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

On 02.03.2026 07:50:58, Greg Kroah-Hartman wrote:
> On Mon, Mar 02, 2026 at 11:01:48AM +0100, Marc Kleine-Budde wrote:
> > On 23.02.2026 17:39:20, Greg Kroah-Hartman wrote:
> > > When submitting an urb, that is using the anchor pattern, it needs to be
> > > anchored before submitting it otherwise it could be leaked if
> > > usb_kill_anchored_urbs() is called.  This logic is correctly done
> > > elsewhere in the driver, except in the read bulk callback so do that
> > > here also.
> > >
> > > Cc: Vincent Mailhol <mailhol@kernel.org>
> > > Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> > > Cc: stable <stable@kernel.org>
> >
> > FYI: checkpatch suggests:
> > Invalid email format for stable: 'stable <stable@kernel.org>', prefer 'stable@kernel.org'
>
> Checkpatch is wrong here, either is just fine.

That check was introduced by Dwaipayan Ray (Cc'ed) in commit
fccaebf00e60 ("checkpatch: improve email parsing"), a.k.a.
v5.11-rc1~133^2~46.

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

Re: checkpatch: format of stable@k.o address (was: Re: [PATCH] can: usb: etas_es58x: correctly anchor the urb in the read bulk callback)

[Greg Kroah-Hartman]

On Mon, Mar 02, 2026 at 02:11:37PM +0100, Marc Kleine-Budde wrote:
> On 02.03.2026 07:50:58, Greg Kroah-Hartman wrote:
> > On Mon, Mar 02, 2026 at 11:01:48AM +0100, Marc Kleine-Budde wrote:
> > > On 23.02.2026 17:39:20, Greg Kroah-Hartman wrote:
> > > > When submitting an urb, that is using the anchor pattern, it needs to be
> > > > anchored before submitting it otherwise it could be leaked if
> > > > usb_kill_anchored_urbs() is called.  This logic is correctly done
> > > > elsewhere in the driver, except in the read bulk callback so do that
> > > > here also.
> > > >
> > > > Cc: Vincent Mailhol <mailhol@kernel.org>
> > > > Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> > > > Cc: stable <stable@kernel.org>
> > >
> > > FYI: checkpatch suggests:
> > > Invalid email format for stable: 'stable <stable@kernel.org>', prefer 'stable@kernel.org'
> >
> > Checkpatch is wrong here, either is just fine.
> 
> That check was introduced by Dwaipayan Ray (Cc'ed) in commit
> fccaebf00e60 ("checkpatch: improve email parsing"), a.k.a.
> v5.11-rc1~133^2~46.

That's some regex hell that I don't want to attempt to fix up, but
really, what I have here is just fine, and "should" be ok by checkpatch
as well.

thanks,

greg k-h