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 928150fad41b ("can: esd_usb2: fix memory
leak").

Link: https://lore.kernel.org/all/alpine.DEB.2.22.394.2206031547001.1630869@thelappy
Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
Cc: stable@vger.kernel.org
Signed-off-by: Rhett Aultman <rhett.aultman@samsara.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b29ba9138866b1d926b660095ec61211c6a818b2..d3a658b444b5f645fb7a579190fe7379bae1279c 100644 (file)
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -268,6 +268,8 @@ struct gs_can {
 
        struct usb_anchor tx_submitted;
        atomic_t active_tx_urbs;
+       void *rxbuf[GS_MAX_RX_URBS];
+       dma_addr_t rxbuf_dma[GS_MAX_RX_URBS];
 };
 
 /* usb interface struct */
@@ -742,6 +744,7 @@ static int gs_can_open(struct net_device *netdev)
                for (i = 0; i < GS_MAX_RX_URBS; i++) {
                        struct urb *urb;
                        u8 *buf;
+                       dma_addr_t buf_dma;
 
                        /* alloc rx urb */
                        urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -752,7 +755,7 @@ static int gs_can_open(struct net_device *netdev)
                        buf = usb_alloc_coherent(dev->udev,
                                                 dev->parent->hf_size_rx,
                                                 GFP_KERNEL,
-                                                &urb->transfer_dma);
+                                                &buf_dma);
                        if (!buf) {
                                netdev_err(netdev,
                                           "No memory left for USB buffer\n");
@@ -760,6 +763,8 @@ static int gs_can_open(struct net_device *netdev)
                                return -ENOMEM;
                        }
 
+                       urb->transfer_dma = buf_dma;
+
                        /* fill, anchor, and submit rx urb */
                        usb_fill_bulk_urb(urb,
                                          dev->udev,
@@ -781,10 +786,17 @@ static int gs_can_open(struct net_device *netdev)
                                           "usb_submit failed (err=%d)\n", rc);
 
                                usb_unanchor_urb(urb);
+                               usb_free_coherent(dev->udev,
+                                                 sizeof(struct gs_host_frame),
+                                                 buf,
+                                                 buf_dma);
                                usb_free_urb(urb);
                                break;
                        }
 
+                       dev->rxbuf[i] = buf;
+                       dev->rxbuf_dma[i] = buf_dma;
+
                        /* Drop reference,
                         * USB core will take care of freeing it
                         */
@@ -842,13 +854,20 @@ static int gs_can_close(struct net_device *netdev)
        int rc;
        struct gs_can *dev = netdev_priv(netdev);
        struct gs_usb *parent = dev->parent;
+       unsigned int i;
 
        netif_stop_queue(netdev);
 
        /* Stop polling */
        parent->active_channels--;
-       if (!parent->active_channels)
+       if (!parent->active_channels) {
                usb_kill_anchored_urbs(&parent->rx_submitted);
+               for (i = 0; i < GS_MAX_RX_URBS; i++)
+                       usb_free_coherent(dev->udev,
+                                         sizeof(struct gs_host_frame),
+                                         dev->rxbuf[i],
+                                         dev->rxbuf_dma[i]);
+       }
 
        /* Stop sending URBs */
        usb_kill_anchored_urbs(&dev->tx_submitted);