lkml.org 
[lkml]   [2022]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 05/55] can: gs_usb: gs_usb_open/close(): fix memory leak
    Date
    From: Rhett Aultman <rhett.aultman@samsara.com>

    commit 2bda24ef95c0311ab93bda00db40486acf30bd0a upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/can/usb/gs_usb.c | 23 +++++++++++++++++++++--
    1 file changed, 21 insertions(+), 2 deletions(-)

    --- a/drivers/net/can/usb/gs_usb.c
    +++ b/drivers/net/can/usb/gs_usb.c
    @@ -184,6 +184,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 */
    @@ -592,6 +594,7 @@ static int gs_can_open(struct net_device
    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);
    @@ -602,7 +605,7 @@ static int gs_can_open(struct net_device
    buf = usb_alloc_coherent(dev->udev,
    sizeof(struct gs_host_frame),
    GFP_KERNEL,
    - &urb->transfer_dma);
    + &buf_dma);
    if (!buf) {
    netdev_err(netdev,
    "No memory left for USB buffer\n");
    @@ -610,6 +613,8 @@ static int gs_can_open(struct net_device
    return -ENOMEM;
    }

    + urb->transfer_dma = buf_dma;
    +
    /* fill, anchor, and submit rx urb */
    usb_fill_bulk_urb(urb,
    dev->udev,
    @@ -633,10 +638,17 @@ static int gs_can_open(struct net_device
    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
    */
    @@ -701,13 +713,20 @@ static int gs_can_close(struct net_devic
    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);

    \
     
     \ /
      Last update: 2022-07-11 11:20    [W:4.398 / U:0.196 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site