In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to
resubmit the urb, we need to deallocate the transfer buffer that is
allocated in mcba_usb_start().
On some architectures, usb_free_coherent() cannot be called in interrupt
context, create a usb_anchor to keep track of these urbs and free them
later.
Reported-by: syzbot+57281c762a3922e14dfe@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Bui Quang Minh <minhquangbui99@xxxxxxxxx>
---
v1: add memory leak fix when not resubmitting urb
v2: add memory leak fix when failing to resubmit urb
v3: remove usb_free_coherent() calls in interrupt context
drivers/net/can/usb/mcba_usb.c | 48 +++++++++++++++++++++++++++-------
1 file changed, 39 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index df54eb7d4b36..8025db484a22 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -77,6 +77,7 @@ struct mcba_priv {
struct net_device *netdev;
struct usb_anchor tx_submitted;
struct usb_anchor rx_submitted;
+ struct usb_anchor urbs_cleanup;
struct can_berr_counter bec;
bool usb_ka_first_pass;
bool can_ka_first_pass;
@@ -220,14 +221,17 @@ static void mcba_usb_write_bulk_callback(struct urb *urb)
{
struct mcba_usb_ctx *ctx = urb->context;
struct net_device *netdev;
+ struct mcba_priv *priv;
WARN_ON(!ctx);
netdev = ctx->priv->netdev;
+ priv = netdev_priv(netdev);
- /* free up our allocated buffer */
- usb_free_coherent(urb->dev, urb->transfer_buffer_length,
- urb->transfer_buffer, urb->transfer_dma);
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);
if (ctx->can) {
if (!netif_device_present(netdev))
@@ -291,8 +295,11 @@ static netdev_tx_t mcba_usb_xmit(struct mcba_priv *priv,
failed:
usb_unanchor_urb(urb);
- usb_free_coherent(priv->udev, MCBA_USB_TX_BUFF_SIZE, buf,
- urb->transfer_dma);
+
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);
if (err == -ENODEV)
netif_device_detach(priv->netdev);
@@ -584,7 +591,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
case -EPIPE:
case -EPROTO:
case -ESHUTDOWN:
- return;
+ goto free;
default:
netdev_info(netdev, "Rx URB aborted (%d)\n", urb->status);
@@ -615,11 +622,20 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
retval = usb_submit_urb(urb, GFP_ATOMIC);
- if (retval == -ENODEV)
- netif_device_detach(netdev);
- else if (retval)
+ if (retval < 0) {
netdev_err(netdev, "failed resubmitting read bulk urb: %d\n",
retval);
+ if (retval == -ENODEV)
+ netif_device_detach(netdev);
+ goto free;
+ }
+
+ return;
+free:
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);
}
/* Start USB device */
@@ -706,6 +722,17 @@ static int mcba_usb_open(struct net_device *netdev)
return 0;
}
+static void mcba_urb_cleanup(struct mcba_priv *priv)
+{
+ struct urb *urb;
+
+ while ((urb = usb_get_from_anchor(&priv->urbs_cleanup))) {
+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);
+ usb_free_urb(urb);
+ }
+}
+
static void mcba_urb_unlink(struct mcba_priv *priv)
{
usb_kill_anchored_urbs(&priv->rx_submitted);
@@ -723,6 +750,7 @@ static int mcba_usb_close(struct net_device *netdev)
/* Stop polling */
mcba_urb_unlink(priv);
+ mcba_urb_cleanup(priv);
close_candev(netdev);
can_led_event(netdev, CAN_LED_EVENT_STOP);
@@ -812,6 +840,7 @@ static int mcba_usb_probe(struct usb_interface *intf,
init_usb_anchor(&priv->rx_submitted);
init_usb_anchor(&priv->tx_submitted);
+ init_usb_anchor(&priv->urbs_cleanup);
usb_set_intfdata(intf, priv);
@@ -877,6 +906,7 @@ static void mcba_usb_disconnect(struct usb_interface *intf)
unregister_candev(priv->netdev);
mcba_urb_unlink(priv);
+ mcba_urb_cleanup(priv);
free_candev(priv->netdev);
}
--
2.17.1
