Skip to content

Commit 2bda24e

Browse files
roadriverrailmarckleinebudde
authored andcommitted
can: gs_usb: gs_usb_open/close(): fix memory leak
The gs_usb driver appears to suffer from a malady common to many USB CAN adapter drivers in that it performs usb_alloc_coherent() to allocate a number of USB request blocks (URBs) for RX, and then later relies on usb_kill_anchored_urbs() to free them, but this doesn't actually free them. As a result, this may be leaking DMA memory that's been used by the driver. This commit is an adaptation of the techniques found in the esd_usb2 driver where a similar design pattern led to a memory leak. It explicitly frees the RX URBs and their DMA memory via a call to usb_free_coherent(). Since the RX URBs were allocated in the gs_can_open(), we remove them in gs_can_close() rather than in the disconnect function as was done in esd_usb2. For more information, see the 928150f ("can: esd_usb2: fix memory leak"). Link: https://lore.kernel.org/all/alpine.DEB.2.22.394.2206031547001.1630869@thelappy Fixes: d08e973 ("can: gs_usb: Added support for the GS_USB CAN devices") Cc: [email protected] Signed-off-by: Rhett Aultman <[email protected]> Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 374e11f commit 2bda24e

File tree

1 file changed

+21
-2
lines changed

1 file changed

+21
-2
lines changed

drivers/net/can/usb/gs_usb.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ struct gs_can {
268268

269269
struct usb_anchor tx_submitted;
270270
atomic_t active_tx_urbs;
271+
void *rxbuf[GS_MAX_RX_URBS];
272+
dma_addr_t rxbuf_dma[GS_MAX_RX_URBS];
271273
};
272274

273275
/* usb interface struct */
@@ -742,6 +744,7 @@ static int gs_can_open(struct net_device *netdev)
742744
for (i = 0; i < GS_MAX_RX_URBS; i++) {
743745
struct urb *urb;
744746
u8 *buf;
747+
dma_addr_t buf_dma;
745748

746749
/* alloc rx urb */
747750
urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -752,14 +755,16 @@ static int gs_can_open(struct net_device *netdev)
752755
buf = usb_alloc_coherent(dev->udev,
753756
dev->parent->hf_size_rx,
754757
GFP_KERNEL,
755-
&urb->transfer_dma);
758+
&buf_dma);
756759
if (!buf) {
757760
netdev_err(netdev,
758761
"No memory left for USB buffer\n");
759762
usb_free_urb(urb);
760763
return -ENOMEM;
761764
}
762765

766+
urb->transfer_dma = buf_dma;
767+
763768
/* fill, anchor, and submit rx urb */
764769
usb_fill_bulk_urb(urb,
765770
dev->udev,
@@ -781,10 +786,17 @@ static int gs_can_open(struct net_device *netdev)
781786
"usb_submit failed (err=%d)\n", rc);
782787

783788
usb_unanchor_urb(urb);
789+
usb_free_coherent(dev->udev,
790+
sizeof(struct gs_host_frame),
791+
buf,
792+
buf_dma);
784793
usb_free_urb(urb);
785794
break;
786795
}
787796

797+
dev->rxbuf[i] = buf;
798+
dev->rxbuf_dma[i] = buf_dma;
799+
788800
/* Drop reference,
789801
* USB core will take care of freeing it
790802
*/
@@ -842,13 +854,20 @@ static int gs_can_close(struct net_device *netdev)
842854
int rc;
843855
struct gs_can *dev = netdev_priv(netdev);
844856
struct gs_usb *parent = dev->parent;
857+
unsigned int i;
845858

846859
netif_stop_queue(netdev);
847860

848861
/* Stop polling */
849862
parent->active_channels--;
850-
if (!parent->active_channels)
863+
if (!parent->active_channels) {
851864
usb_kill_anchored_urbs(&parent->rx_submitted);
865+
for (i = 0; i < GS_MAX_RX_URBS; i++)
866+
usb_free_coherent(dev->udev,
867+
sizeof(struct gs_host_frame),
868+
dev->rxbuf[i],
869+
dev->rxbuf_dma[i]);
870+
}
852871

853872
/* Stop sending URBs */
854873
usb_kill_anchored_urbs(&dev->tx_submitted);

0 commit comments

Comments
 (0)