[PATCH] can: usb: etas_es58x: Fix RX buffer leak on URB resubmit failure

[PATCH] can: usb: etas_es58x: Fix RX buffer leak on URB resubmit failure

[Guangshuo Li]

es58x_read_bulk_callback() resubmits the RX URB after processing a
received packet. If the resubmit succeeds, the URB remains anchored and
will be handled by the normal RX path or by teardown.

However, if usb_submit_urb() fails, the callback unanchors the URB and
then returns directly. This skips the existing free_urb path, so the
coherent transfer buffer allocated with usb_alloc_coherent() is not
released.

Reuse the existing free_urb path after a resubmit failure so that the RX
coherent buffer is freed before leaving the callback.

Fixes: 5eaad4f76826 ("can: usb: etas_es58x: correctly anchor the urb in the read bulk callback")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/can/usb/etas_es58x/es58x_core.c | 2 +-
 1 file changed, 1 insertion(+), 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 b259f6109808..da12a35dd2f9 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_core.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
@@ -1476,7 +1476,7 @@ static void es58x_read_bulk_callback(struct urb *urb)
 		dev_err_ratelimited(dev,
 				    "Failed resubmitting read bulk urb: %pe\n",
 				    ERR_PTR(ret));
-	return;
+	goto free_urb;
 
  free_urb:
 	usb_free_coherent(urb->dev, urb->transfer_buffer_length,
-- 
2.43.0

Re: [PATCH] can: usb: etas_es58x: Fix RX buffer leak on URB resubmit failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition during device disconnect leads to a Use-After-Free of the `netdev` structure in the URB completion handler.
--

commit 60846d2bc70af068d7027f0af965d52838e69d83
Author: Guangshuo Li <lgs201920130244@gmail.com>

can: usb: etas_es58x: Fix RX buffer leak on URB resubmit failure

This commit reuses the free_urb path after an RX URB resubmit failure
in es58x_read_bulk_callback(). This prevents a memory leak of the coherent
transfer buffer when usb_submit_urb() fails.

> diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.c b/drivers/net/can/usb/etas_es58x/es58x_core.c
> index b259f61098083..da12a35dd2f9a 100644
> --- a/drivers/net/can/usb/etas_es58x/es58x_core.c
> +++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
> @@ -1476,7 +1476,7 @@ static void es58x_read_bulk_callback(struct urb *urb)
>  		dev_err_ratelimited(dev,
>  				    "Failed resubmitting read bulk urb: %pe\n",
>  				    ERR_PTR(ret));
> -	return;
> +	goto free_urb;

[Severity: High]
This is a pre-existing issue, but does a race condition exist during device
disconnect that leads to a use-after-free of the netdev structure?

If a dual-channel ES58x device is abruptly disconnected, es58x_disconnect()
calls es58x_free_netdevs() to iteratively free the channels. When it
unregisters the first channel, opened_channel_cnt drops to 1, so
es58x_free_urbs() is not called yet, and URBs continue running concurrently.

Meanwhile, es58x_free_netdevs() frees the first channel's netdev:

drivers/net/can/usb/etas_es58x/es58x_core.c:es58x_free_netdevs() {
        ...
        es58x_dev->netdev[i] = NULL;
        free_candev(netdev);
        ...
}

If a concurrent es58x_read_bulk_callback() fails to resubmit its URB
and receives -ENODEV due to the disconnect, it iterates over all channels
just above this diff hunk:

drivers/net/can/usb/etas_es58x/es58x_core.c:es58x_read_bulk_callback() {
        ...
        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]);
        ...
}

Could the callback read the netdev pointer before it is set to NULL, but
execute netif_device_detach() on it after free_candev() has already freed it?

>  
>   free_urb:
>  	usb_free_coherent(urb->dev, urb->transfer_buffer_length,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704165717.248880-1-lgs201920130244@gmail.com?part=1

Re: [PATCH] can: usb: etas_es58x: Fix RX buffer leak on URB resubmit failure

[Vincent Mailhol]

On 04/07/2026 at 18:57, Guangshuo Li wrote:
> es58x_read_bulk_callback() resubmits the RX URB after processing a
> received packet. If the resubmit succeeds, the URB remains anchored and
> will be handled by the normal RX path or by teardown.
> 
> However, if usb_submit_urb() fails, the callback unanchors the URB and
> then returns directly. This skips the existing free_urb path, so the
> coherent transfer buffer allocated with usb_alloc_coherent() is not
> released.
> 
> Reuse the existing free_urb path after a resubmit failure so that the RX
> coherent buffer is freed before leaving the callback.
> 
> Fixes: 5eaad4f76826 ("can: usb: etas_es58x: correctly anchor the urb in the read bulk callback")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/net/can/usb/etas_es58x/es58x_core.c | 2 +-
>  1 file changed, 1 insertion(+), 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 b259f6109808..da12a35dd2f9 100644
> --- a/drivers/net/can/usb/etas_es58x/es58x_core.c
> +++ b/drivers/net/can/usb/etas_es58x/es58x_core.c
> @@ -1476,7 +1476,7 @@ static void es58x_read_bulk_callback(struct urb *urb)
>  		dev_err_ratelimited(dev,
>  				    "Failed resubmitting read bulk urb: %pe\n",
>  				    ERR_PTR(ret));
> -	return;
> +	goto free_urb;

Just delete the return statement. No need for the goto as the label is
just below.

With above comment addressed:

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

>   free_urb:
>  	usb_free_coherent(urb->dev, urb->transfer_buffer_length,


Yours sincerely,
Vincent Mailhol