can: gs_usb: gs_usb_open/close(): fix memory leak
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: d08e973a77
("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>
This commit is contained in:

committed by
Greg Kroah-Hartman

parent
b6f4b347a1
commit
d0b8e22399
@@ -184,6 +184,8 @@ struct gs_can {
|
|||||||
|
|
||||||
struct usb_anchor tx_submitted;
|
struct usb_anchor tx_submitted;
|
||||||
atomic_t active_tx_urbs;
|
atomic_t active_tx_urbs;
|
||||||
|
void *rxbuf[GS_MAX_RX_URBS];
|
||||||
|
dma_addr_t rxbuf_dma[GS_MAX_RX_URBS];
|
||||||
};
|
};
|
||||||
|
|
||||||
/* usb interface struct */
|
/* usb interface struct */
|
||||||
@@ -592,6 +594,7 @@ static int gs_can_open(struct net_device *netdev)
|
|||||||
for (i = 0; i < GS_MAX_RX_URBS; i++) {
|
for (i = 0; i < GS_MAX_RX_URBS; i++) {
|
||||||
struct urb *urb;
|
struct urb *urb;
|
||||||
u8 *buf;
|
u8 *buf;
|
||||||
|
dma_addr_t buf_dma;
|
||||||
|
|
||||||
/* alloc rx urb */
|
/* alloc rx urb */
|
||||||
urb = usb_alloc_urb(0, GFP_KERNEL);
|
urb = usb_alloc_urb(0, GFP_KERNEL);
|
||||||
@@ -602,7 +605,7 @@ static int gs_can_open(struct net_device *netdev)
|
|||||||
buf = usb_alloc_coherent(dev->udev,
|
buf = usb_alloc_coherent(dev->udev,
|
||||||
sizeof(struct gs_host_frame),
|
sizeof(struct gs_host_frame),
|
||||||
GFP_KERNEL,
|
GFP_KERNEL,
|
||||||
&urb->transfer_dma);
|
&buf_dma);
|
||||||
if (!buf) {
|
if (!buf) {
|
||||||
netdev_err(netdev,
|
netdev_err(netdev,
|
||||||
"No memory left for USB buffer\n");
|
"No memory left for USB buffer\n");
|
||||||
@@ -610,6 +613,8 @@ static int gs_can_open(struct net_device *netdev)
|
|||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
urb->transfer_dma = buf_dma;
|
||||||
|
|
||||||
/* fill, anchor, and submit rx urb */
|
/* fill, anchor, and submit rx urb */
|
||||||
usb_fill_bulk_urb(urb,
|
usb_fill_bulk_urb(urb,
|
||||||
dev->udev,
|
dev->udev,
|
||||||
@@ -633,10 +638,17 @@ static int gs_can_open(struct net_device *netdev)
|
|||||||
rc);
|
rc);
|
||||||
|
|
||||||
usb_unanchor_urb(urb);
|
usb_unanchor_urb(urb);
|
||||||
|
usb_free_coherent(dev->udev,
|
||||||
|
sizeof(struct gs_host_frame),
|
||||||
|
buf,
|
||||||
|
buf_dma);
|
||||||
usb_free_urb(urb);
|
usb_free_urb(urb);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
dev->rxbuf[i] = buf;
|
||||||
|
dev->rxbuf_dma[i] = buf_dma;
|
||||||
|
|
||||||
/* Drop reference,
|
/* Drop reference,
|
||||||
* USB core will take care of freeing it
|
* USB core will take care of freeing it
|
||||||
*/
|
*/
|
||||||
@@ -701,13 +713,20 @@ static int gs_can_close(struct net_device *netdev)
|
|||||||
int rc;
|
int rc;
|
||||||
struct gs_can *dev = netdev_priv(netdev);
|
struct gs_can *dev = netdev_priv(netdev);
|
||||||
struct gs_usb *parent = dev->parent;
|
struct gs_usb *parent = dev->parent;
|
||||||
|
unsigned int i;
|
||||||
|
|
||||||
netif_stop_queue(netdev);
|
netif_stop_queue(netdev);
|
||||||
|
|
||||||
/* Stop polling */
|
/* Stop polling */
|
||||||
parent->active_channels--;
|
parent->active_channels--;
|
||||||
if (!parent->active_channels)
|
if (!parent->active_channels) {
|
||||||
usb_kill_anchored_urbs(&parent->rx_submitted);
|
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 */
|
/* Stop sending URBs */
|
||||||
usb_kill_anchored_urbs(&dev->tx_submitted);
|
usb_kill_anchored_urbs(&dev->tx_submitted);
|
||||||
|
Reference in New Issue
Block a user