Skip to content

Commit 91c0255

Browse files
pskrgagmarckleinebudde
authored andcommitted
can: mcba_usb: fix memory leak in mcba_usb
Syzbot reported memory leak in SocketCAN driver for Microchip CAN BUS Analyzer Tool. The problem was in unfreed usb_coherent. In mcba_usb_start() 20 coherent buffers are allocated and there is nothing, that frees them: 1) In callback function the urb is resubmitted and that's all 2) In disconnect function urbs are simply killed, but URB_FREE_BUFFER is not set (see mcba_usb_start) and this flag cannot be used with coherent buffers. Fail log: | [ 1354.053291][ T8413] mcba_usb 1-1:0.0 can0: device disconnected | [ 1367.059384][ T8420] kmemleak: 20 new suspected memory leaks (see /sys/kernel/debug/kmem) So, all allocated buffers should be freed with usb_free_coherent() explicitly NOTE: The same pattern for allocating and freeing coherent buffers is used in drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c Fixes: 51f3baa ("can: mcba_usb: Add support for Microchip CAN BUS Analyzer") Link: https://lore.kernel.org/r/[email protected] Cc: linux-stable <[email protected]> Reported-and-tested-by: [email protected] Signed-off-by: Pavel Skripkin <[email protected]> Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 5e87ddb commit 91c0255

File tree

1 file changed

+15
-2
lines changed

1 file changed

+15
-2
lines changed

drivers/net/can/usb/mcba_usb.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ struct mcba_priv {
8282
bool can_ka_first_pass;
8383
bool can_speed_check;
8484
atomic_t free_ctx_cnt;
85+
void *rxbuf[MCBA_MAX_RX_URBS];
86+
dma_addr_t rxbuf_dma[MCBA_MAX_RX_URBS];
8587
};
8688

8789
/* CAN frame */
@@ -633,6 +635,7 @@ static int mcba_usb_start(struct mcba_priv *priv)
633635
for (i = 0; i < MCBA_MAX_RX_URBS; i++) {
634636
struct urb *urb = NULL;
635637
u8 *buf;
638+
dma_addr_t buf_dma;
636639

637640
/* create a URB, and a buffer for it */
638641
urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -642,7 +645,7 @@ static int mcba_usb_start(struct mcba_priv *priv)
642645
}
643646

644647
buf = usb_alloc_coherent(priv->udev, MCBA_USB_RX_BUFF_SIZE,
645-
GFP_KERNEL, &urb->transfer_dma);
648+
GFP_KERNEL, &buf_dma);
646649
if (!buf) {
647650
netdev_err(netdev, "No memory left for USB buffer\n");
648651
usb_free_urb(urb);
@@ -661,11 +664,14 @@ static int mcba_usb_start(struct mcba_priv *priv)
661664
if (err) {
662665
usb_unanchor_urb(urb);
663666
usb_free_coherent(priv->udev, MCBA_USB_RX_BUFF_SIZE,
664-
buf, urb->transfer_dma);
667+
buf, buf_dma);
665668
usb_free_urb(urb);
666669
break;
667670
}
668671

672+
priv->rxbuf[i] = buf;
673+
priv->rxbuf_dma[i] = buf_dma;
674+
669675
/* Drop reference, USB core will take care of freeing it */
670676
usb_free_urb(urb);
671677
}
@@ -708,7 +714,14 @@ static int mcba_usb_open(struct net_device *netdev)
708714

709715
static void mcba_urb_unlink(struct mcba_priv *priv)
710716
{
717+
int i;
718+
711719
usb_kill_anchored_urbs(&priv->rx_submitted);
720+
721+
for (i = 0; i < MCBA_MAX_RX_URBS; ++i)
722+
usb_free_coherent(priv->udev, MCBA_USB_RX_BUFF_SIZE,
723+
priv->rxbuf[i], priv->rxbuf_dma[i]);
724+
712725
usb_kill_anchored_urbs(&priv->tx_submitted);
713726
}
714727

0 commit comments

Comments
 (0)