Skip to content

Commit 9969e3c

Browse files
pskrgagmarckleinebudde
authored andcommitted
can: ems_usb: fix memory leak
In ems_usb_start() MAX_RX_URBS 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 ems_usb_start) and this flag cannot be used with coherent buffers. So, all allocated buffers should be freed with usb_free_coherent() explicitly. Side note: This code looks like a copy-paste of other can drivers. The same patch was applied to mcba_usb driver and it works nice with real hardware. There is no change in functionality, only clean-up code for coherent buffers. Fixes: 702171a ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface") Link: https://lore.kernel.org/r/59aa9fbc9a8cbf9af2bbd2f61a659c480b415800.1627404470.git.paskripkin@gmail.com Cc: linux-stable <[email protected]> Signed-off-by: Pavel Skripkin <[email protected]> Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 0e865f0 commit 9969e3c

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

drivers/net/can/usb/ems_usb.c

+13-1
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,8 @@ struct ems_usb {
255255
unsigned int free_slots; /* remember number of available slots */
256256

257257
struct ems_cpc_msg active_params; /* active controller parameters */
258+
void *rxbuf[MAX_RX_URBS];
259+
dma_addr_t rxbuf_dma[MAX_RX_URBS];
258260
};
259261

260262
static void ems_usb_read_interrupt_callback(struct urb *urb)
@@ -587,6 +589,7 @@ static int ems_usb_start(struct ems_usb *dev)
587589
for (i = 0; i < MAX_RX_URBS; i++) {
588590
struct urb *urb = NULL;
589591
u8 *buf = NULL;
592+
dma_addr_t buf_dma;
590593

591594
/* create a URB, and a buffer for it */
592595
urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -596,14 +599,16 @@ static int ems_usb_start(struct ems_usb *dev)
596599
}
597600

598601
buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
599-
&urb->transfer_dma);
602+
&buf_dma);
600603
if (!buf) {
601604
netdev_err(netdev, "No memory left for USB buffer\n");
602605
usb_free_urb(urb);
603606
err = -ENOMEM;
604607
break;
605608
}
606609

610+
urb->transfer_dma = buf_dma;
611+
607612
usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 2),
608613
buf, RX_BUFFER_SIZE,
609614
ems_usb_read_bulk_callback, dev);
@@ -619,6 +624,9 @@ static int ems_usb_start(struct ems_usb *dev)
619624
break;
620625
}
621626

627+
dev->rxbuf[i] = buf;
628+
dev->rxbuf_dma[i] = buf_dma;
629+
622630
/* Drop reference, USB core will take care of freeing it */
623631
usb_free_urb(urb);
624632
}
@@ -684,6 +692,10 @@ static void unlink_all_urbs(struct ems_usb *dev)
684692

685693
usb_kill_anchored_urbs(&dev->rx_submitted);
686694

695+
for (i = 0; i < MAX_RX_URBS; ++i)
696+
usb_free_coherent(dev->udev, RX_BUFFER_SIZE,
697+
dev->rxbuf[i], dev->rxbuf_dma[i]);
698+
687699
usb_kill_anchored_urbs(&dev->tx_submitted);
688700
atomic_set(&dev->active_tx_urbs, 0);
689701

0 commit comments

Comments
 (0)