linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rhett Aultman <rhett.aultman@samsara.com>
To: linux-can <linux-can@vger.kernel.org>
Cc: --cc=Marc Kleine-Budde <mkl@pengutronix.de>,
	Vincent MAILHOL <mailhol.vincent@wanadoo.fr>,
	Rhett Aultman <rhett.aultman@samsara.com>
Subject: [PATCH v4 1/1] can: gs_usb: fix DMA memory leak on close
Date: Tue, 14 Jun 2022 11:26:31 -0400	[thread overview]
Message-ID: <20220614152630.2255641-2-rhett.aultman@samsara.com> (raw)
In-Reply-To: <20220614152630.2255641-1-rhett.aultman@samsara.com>

The gs_usb driver allocates DMA memory with usb_alloc_coherent() in
gs_can_open() and then keeps this memory in an URB, with the expectation
that the memory will be freed when the URB is killed in gs_can_close().
Memory allocated with usb_alloc_coherent() cannot be freed in this way
and must be freed using usb_free_coherent() instead.  This means that
repeated cycles of calling gs_can_open() and gs_can_close() will lead to
a memory leak.

Historically, drivers have handled this by keeping an array of pointers
to their DMA rx buffers and explicitly freeing them.  For an example of
this technique used in the esd_usb2 driver, see here:
928150fad41b ("can: esd_usb2: fix memory leak")

While the above method works, the conditions that cause this leak are
not apparent to driver writers and the method for solving it at the
driver level has been piecemeal.  This patch makes use of a new
URB_FREE_COHERENT flag on the URB, reducing the solution of this memory
leak down to a single line of code.

Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
Signed-off-by: Rhett Aultman <rhett.aultman@samsara.com>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 drivers/net/can/usb/gs_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b29ba9138866..4e7e6a7b961a 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -768,7 +768,7 @@ static int gs_can_open(struct net_device *netdev)
 					  buf,
 					  dev->parent->hf_size_rx,
 					  gs_usb_receive_bulk_callback, parent);
-			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+			urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP | URB_FREE_COHERENT;
 
 			usb_anchor_urb(urb, &parent->rx_submitted);
 
-- 
2.30.2


      reply	other threads:[~2022-06-14 15:27 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-03 19:52 [PATCH 001/001] can: gs_usb: gs_usb_open/close( ) fix memory leak Rhett Aultman
2022-06-04  2:11 ` [PATCH] " Vincent Mailhol
2022-06-04  2:26   ` Vincent MAILHOL
2022-06-04 14:08     ` Rhett Aultman
2022-06-04 14:41       ` [RFC PATCH] USB: core: urb: add new transfer flag URB_FREE_COHERENT Vincent Mailhol
2022-06-04 16:40         ` Alan Stern
2022-06-05  2:04           ` Vincent MAILHOL
2022-06-05  6:00           ` Oliver Neukum
2022-06-05 13:45             ` Vincent MAILHOL
2022-06-07  9:49               ` Oliver Neukum
2022-06-07 10:18                 ` Vincent MAILHOL
2022-06-07 11:46                   ` Oliver Neukum
2022-06-07 12:12                     ` Vincent MAILHOL
2022-06-05  2:15         ` [RFC PATCH v2] usb: " Vincent Mailhol
2022-06-04 14:53       ` [PATCH] can: gs_usb: gs_usb_open/close( ) fix memory leak Vincent MAILHOL
2022-06-09 20:47 ` [PATCH v2 0/3] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-09 20:47   ` [PATCH v2 1/3] drivers: usb/core/urb: Add URB_FREE_COHERENT Rhett Aultman
2022-06-10  0:18     ` Vincent MAILHOL
2022-06-10 10:46       ` Marc Kleine-Budde
2022-06-09 20:47   ` [PATCH v2 2/3] drivers: usb/core/urb: allow URB_FREE_COHERENT Rhett Aultman
2022-06-09 23:18     ` Vincent Mailhol
2022-06-09 20:47   ` [PATCH v2 3/3] can: gs_usb: fix DMA memory leak on close Rhett Aultman
2022-06-10  0:05     ` Vincent Mailhol
2022-06-10  1:28       ` Vincent Mailhol
2022-06-10 21:33   ` [PATCH v3 0/2] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-10 21:33     ` [PATCH v3 1/2] drivers: usb/core/urb: Add URB_FREE_COHERENT Rhett Aultman
2022-06-11 15:31       ` Marc Kleine-Budde
2022-06-11 16:06         ` Vincent MAILHOL
2022-06-21 13:51           ` Greg Kroah-Hartman
2022-06-21 14:59             ` Vincent MAILHOL
2022-06-21 15:03               ` Greg Kroah-Hartman
2022-06-21 15:54                 ` Vincent MAILHOL
2022-06-21 15:11               ` Alan Stern
2022-06-21 15:55                 ` Vincent MAILHOL
2022-06-21 16:14                   ` Alan Stern
2022-06-21 16:40                     ` Vincent MAILHOL
2022-06-21 17:00                       ` Greg Kroah-Hartman
2022-06-21 17:14                         ` Vincent MAILHOL
2022-06-21 17:46                           ` Greg Kroah-Hartman
2022-06-22  9:22                   ` David Laight
2022-06-22  9:41                     ` Greg Kroah-Hartman
2022-06-22 10:03                       ` David Laight
2022-06-22 11:11                         ` Oliver Neukum
2022-06-22 10:34                       ` Vincent MAILHOL
2022-06-22 12:23                         ` Greg Kroah-Hartman
2022-06-22 15:59                           ` Vincent MAILHOL
2022-06-22 18:11                             ` Rhett Aultman
2022-06-26  8:21                               ` Vincent MAILHOL
2022-06-27 19:24                                 ` Rhett Aultman
2022-06-28  1:09                                   ` Vincent MAILHOL
2022-07-04 13:02                                     ` Marc Kleine-Budde
2022-07-04 15:35                                       ` Rhett Aultman
2022-07-05  7:50                                         ` Marc Kleine-Budde
2022-06-23 17:30       ` Hongren Zenithal Zheng
2022-06-23 17:45         ` Shuah Khan
2022-06-24 14:43           ` Alan Stern
2022-06-24 16:01             ` Hongren Zenithal Zheng
2022-06-24 16:31             ` Shuah Khan
2022-06-24 18:07               ` Alan Stern
2022-06-27 22:54                 ` Shuah Khan
2022-06-28  1:35                   ` Alan Stern
2022-07-01  2:10                     ` Shuah Khan
2022-08-01 17:42                       ` Shuah Khan
2022-08-01 18:28                         ` Vincent MAILHOL
2022-08-03 23:44                           ` Shuah Khan
2022-06-10 21:33     ` [PATCH v3 2/2] can: gs_usb: fix DMA memory leak on close Rhett Aultman
2022-06-11 15:35       ` Marc Kleine-Budde
2022-06-11 16:03         ` Vincent MAILHOL
2022-06-12 21:28       ` David Laight
2022-06-12 21:33         ` Marc Kleine-Budde
2022-06-14 15:26     ` [PATCH v4 0/1] URB_FREE_COHERENT gs_usb memory leak fix Rhett Aultman
2022-06-14 15:26       ` Rhett Aultman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220614152630.2255641-2-rhett.aultman@samsara.com \
    --to=rhett.aultman@samsara.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=mkl@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).