linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rhett Aultman <rhett.aultman@samsara.com>
To: linux-usb@vger.kernel.org, linux-can <linux-can@vger.kernel.org>
Cc: --cc=Oliver Neukum <oneukum@suse.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rhett Aultman <rhett.aultman@samsara.com>
Subject: [PATCH v3 0/2] URB_FREE_COHERENT gs_usb memory leak fix
Date: Fri, 10 Jun 2022 17:33:33 -0400	[thread overview]
Message-ID: <20220610213335.3077375-1-rhett.aultman@samsara.com> (raw)
In-Reply-To: <20220609204714.2715188-1-rhett.aultman@samsara.com>

This patchset resolves a to memory leak which can occur with successive
iterations of calling gs_can_open() and gs_can_close().  The central
cause of this memory leak, which is an issue common to many of the USB
CAN drivers, is that memory allocated for RX buffers using
usb_alloc_coherent() and then kept in the URB will be properly freed
when the URB is killed.  This assumption is incorrect, as memory
allocated with usb_alloc_coherent() must be freed with
usb_free_coherent(), and there is no provision for this in the existing
URB code.

The common solution to this, found in v1 of my patches as well as in
already merged patches for other CAN USB drivers (see the patch for the
esd CAN-USB/2 driver here:
https://www.spinics.net/lists/linux-can/msg08203.html) is for the driver
itself to maintain an array of addresses of the buffers allocated with
usb_alloc_coherent() and to then iteratively call usb_free_coherent() on
them in their close function.  This solution requires a driver developer
to understand this unclear nuance, and it has historically been solved
in a piecemeal way one driver at a time (note: the gs_usb driver has had
this issue since the 3.x.x kernel series).

Rather than continue to place the burden of complexity on the drivers,
this patchset adds a new URB flag which allows the DMA buffer to be
correctly freed with the URB is killed.  This results in a much simpler
solution at the driver level and with minimal additional code in the USB
core.

** Changelog **

v2 -> v3:

  * Resolve style nits in gs_usb.c
  * Squash commits in urb.c and correct authorship

v1 -> v2:

  * Add URB_FREE_COHERENT flag to urb.c
  * Use URB_FREE_COHERENT flag rather than arrays of buffers in gs_usb.c

Rhett Aultman (1):
  can: gs_usb: fix DMA memory leak on close

Vincent Mailhol (1):
  drivers: usb/core/urb: Add URB_FREE_COHERENT

 drivers/net/can/usb/gs_usb.c | 2 +-
 drivers/usb/core/urb.c       | 5 ++++-
 include/linux/usb.h          | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.30.2


  parent reply	other threads:[~2022-06-10 21:33 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   ` Rhett Aultman [this message]
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       ` [PATCH v4 1/1] can: gs_usb: fix DMA memory leak on close Rhett Aultman

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=20220610213335.3077375-1-rhett.aultman@samsara.com \
    --to=rhett.aultman@samsara.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=oneukum@suse.com \
    --cc=stern@rowland.harvard.edu \
    /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).