[PATCH net] can: peak_usb: fix double free of transfer buffer on URB submit error

[PATCH net] can: peak_usb: fix double free of transfer buffer on URB submit error

[Maoyi Xie]

In peak_usb_start(), each RX URB transfer buffer is allocated with kmalloc()
and the URB is flagged URB_FREE_BUFFER so that the final usb_free_urb() also
frees the transfer buffer.

If usb_submit_urb() fails, the error path frees the buffer explicitly with
kfree(buf) and then calls usb_free_urb(urb). Because URB_FREE_BUFFER is set,
usb_free_urb() -> urb_destroy() frees the same buffer a second time, a double
free of the transfer buffer.

  BUG: KASAN: double-free in usb_free_urb.part.0+0x91/0xb0
  Free of addr ffff8881069ccb80 by task trigger.sh/285

  Call Trace:
   kfree+0x113/0x3c0
   usb_free_urb.part.0+0x91/0xb0

Drop the redundant kfree(buf); usb_free_urb() already releases the transfer
buffer. This mirrors commit 03819abbeb11 ("net: usb: lan78xx: Fix double free
issue with interrupt buffer allocation").

Fixes: bb4785551f64 ("can: usb: PEAK-System Technik USB adapters driver core")
Closes: https://lore.kernel.org/linux-can/178159320216.2154888.16953451793788581739@maoyixie.com/T/#u
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index cf48bb26d46d..079b23dd74f0 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -470,7 +470,6 @@ static int peak_usb_start(struct peak_usb_device *dev)
 				netif_device_detach(dev->netdev);
 
 			usb_unanchor_urb(urb);
-			kfree(buf);
 			usb_free_urb(urb);
 			break;
 		}
-- 
2.34.1

Re: [PATCH net] can: peak_usb: fix double free of transfer buffer on URB submit error

[sashiko-bot]

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

Pre-existing issues:
- [High] In-flight TX URBs and their transfer buffers are permanently leaked when the CAN interface is brought down.
- [High] If `peak_usb_start()` fails to allocate the full set of TX URBs, uninitialized contexts cause a NULL pointer dereference in `peak_usb_ndo_start_xmit()`.
--

commit 940b179b096bd59f1b3c9ac845bb1de2207ae9fa
Author: Maoyi Xie <maoyixie.tju@gmail.com>

can: peak_usb: fix double free of transfer buffer on URB submit error

This commit removes a redundant explicit free of the transfer buffer in
the error path of peak_usb_start() because usb_free_urb() already handles
it when URB_FREE_BUFFER is set.

Closes: https://lore.kernel.org/linux-can/178159320216.2154888.16953451793788581739@maoyixie.com/T/#u
Cc: stable@vger.kernel.org

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> index cf48bb26d46d2..079b23dd74f04 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -470,7 +470,6 @@ static int peak_usb_start(struct peak_usb_device *dev)
>  				netif_device_detach(dev->netdev);
>  
>  			usb_unanchor_urb(urb);
> -			kfree(buf);
>  			usb_free_urb(urb);
>  			break;
>  		}

[Severity: High]
This is a pre-existing issue, but does this driver leak in-flight TX URBs
and their transfer buffers when the CAN interface is brought down?

When the interface is stopped, peak_usb_ndo_stop() calls
peak_usb_unlink_all_urbs() to clean up. This function iterates over the
pre-allocated TX URBs but explicitly skips freeing any URB that is
currently in-flight:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_unlink_all_urbs() {
        if (!urb ||
            dev->tx_contexts[i].echo_index != PCAN_USB_MAX_TX_URBS) {
                /*
                 * this urb is already released or always submitted,
                 * let usb core free by itself
                 */
                continue;
        }

        usb_free_urb(urb);
        dev->tx_contexts[i].urb = NULL;
}

While usb_kill_anchored_urbs() will abort the submitted URBs and cause the
USB core to drop its submission reference, the driver's original allocation
reference is never dropped because of the continue statement above.

When the interface is subsequently restarted, peak_usb_start()
unconditionally allocates new URBs, overwriting the context pointers
and permanently leaking the old URBs.


[Severity: High]
This isn't a bug introduced by this patch, but could this code path cause
a NULL pointer dereference in peak_usb_ndo_start_xmit() if the driver fails
to allocate the full set of TX URBs?

Later in peak_usb_start(), if memory allocation fails for a TX context after
at least one context is successfully allocated, the driver issues a warning
and proceeds:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_start() {
        /* warn if we were not able to allocate enough tx contexts */
        if (i < PCAN_USB_MAX_TX_URBS) {
                if (i == 0) {
                        netdev_err(netdev, "couldn't setup any tx URB\n");
                        goto err_tx;
                }

                netdev_warn(netdev, "tx performance may be slow\n");
        }
        ...
        return 0;
}

The unallocated contexts retain a NULL urb pointer and their echo_index 
is set to PCAN_USB_MAX_TX_URBS, marking them as available.

However, peak_usb_ndo_start_xmit() only stops the netdev TX queue when the
total number of active URBs reaches PCAN_USB_MAX_TX_URBS. If the number
of concurrent transmissions exceeds the number of successfully allocated
contexts, it can select an unallocated context and blindly dereference it:

drivers/net/can/usb/peak_usb/pcan_usb_core.c:peak_usb_ndo_start_xmit() {
        if (!context) {
                /* should not occur except during restart */
                return NETDEV_TX_BUSY;
        }

        urb = context->urb;
        obuf = urb->transfer_buffer;
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178163373110.2507866.216458825145756798@maoyixie.com?part=1

Re: [PATCH net] can: peak_usb: fix double free of transfer buffer on URB submit error

[Vincent Mailhol]

On 16/06/2026 at 20:15, Maoyi Xie wrote:
> In peak_usb_start(), each RX URB transfer buffer is allocated with kmalloc()
> and the URB is flagged URB_FREE_BUFFER so that the final usb_free_urb() also
> frees the transfer buffer.
> 
> If usb_submit_urb() fails, the error path frees the buffer explicitly with
> kfree(buf) and then calls usb_free_urb(urb). Because URB_FREE_BUFFER is set,
> usb_free_urb() -> urb_destroy() frees the same buffer a second time, a double
> free of the transfer buffer.
> 
>   BUG: KASAN: double-free in usb_free_urb.part.0+0x91/0xb0
>   Free of addr ffff8881069ccb80 by task trigger.sh/285
> 
>   Call Trace:
>    kfree+0x113/0x3c0
>    usb_free_urb.part.0+0x91/0xb0
> 
> Drop the redundant kfree(buf); usb_free_urb() already releases the transfer
> buffer. This mirrors commit 03819abbeb11 ("net: usb: lan78xx: Fix double free
> issue with interrupt buffer allocation").
> 
> Fixes: bb4785551f64 ("can: usb: PEAK-System Technik USB adapters driver core")
> Closes: https://lore.kernel.org/linux-can/178159320216.2154888.16953451793788581739@maoyixie.com/T/#u
> Cc: stable@vger.kernel.org
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>

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


Yours sincerely,
Vincent Mailhol