ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: ksummit-discuss@lists.linuxfoundation.org
Cc: Tejun Heo <tj@kernel.org>, Russell King <linux@arm.linux.org.uk>,
	Shuah Khan <shuah.kh@samsung.com>
Subject: [Ksummit-discuss] [TECH TOPIC] Fix devm_kzalloc, its users, or both
Date: Fri, 31 Jul 2015 18:14:14 +0300	[thread overview]
Message-ID: <2111196.TG1k3f53YQ@avalon> (raw)

Hello,

It recently came to my attention that the way devm_kzalloc() is used by most 
drivers is broken. I've raised the topic on LKML (see 
http://lkml.org/lkml/2015/7/14/741) in the hope that my findings were simply 
wrong, but it turned out I was unfortunately right. As the topic spans lots of 
subsystems I believe it would be a good technical topic for the Kernel Summit.

The issue occurs when drivers use devm_kzalloc() to allocate data structures 
that can be accessed through file operations on a device node. The following 
sequence of events will then lead to a crash.

1. Get a device bound to its driver
2. Open the corresponding device node in userspace and keep it open
3. Unbind the device from its driver through sysfs using for instance

echo <device-name> > /sys/bus/platform/drivers/<driver-name>/unbind

(or for hotpluggable devices just unplug the device)

4. Close the device node
5. Enjoy the fireworks

While having a device node open prevents modules from being unloaded, it 
doesn't prevent devices from being unbound from drivers. If the driver uses 
devm_* helpers to allocate memory the memory will be freed when the device is 
unbound from the driver, but that memory will still be used by any operation 
touching an open device node.

Tejun Heo commented that "this really is a general lifetime management 
problem. [...] If a piece of memory isn't attached to the harware side but the 
userland interface side [...], that shouldn't be allocated via devm - it has 
"dev" in its name for a reason."

While I agree that this is how devres operates today, I'm not sure we should 
throw the baby out with the bath water. Using devm_kzalloc() as currently done  
has value, and reverting drivers to the pre-devm memory allocation code would 
make error handling and cleanup code paths more complex again.

Should we introduce a managed allocator for that purpose that would have a 
lifespan explicitly handled by drivers (or, even better, handled automatically 
in a way that would suit our use cases) ? Tejun commented that "a better 
approach would be implementing generic revoke feature and sever open files on 
driver detach so that everything can be shutdown then".

People who might be interested:

- Tejun Heo <tj@kernel.org>
- Shuah Khan <shuah.kh@samsung.com> (for the media controller devres WIP)
- Russell King <linux@arm.linux.org.uk> (for the component framework)
- Anyone who believes that managed memory allocation for driver private 
structures is useful

-- 
Regards,

Laurent Pinchart

             reply	other threads:[~2015-07-31 15:13 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-31 15:14 Laurent Pinchart [this message]
2015-07-31 15:56 ` Russell King - ARM Linux
2015-07-31 16:34 ` Julia Lawall
2015-07-31 16:51   ` Dmitry Torokhov
2015-07-31 16:57     ` Julia Lawall
2015-07-31 17:03       ` Dmitry Torokhov
2015-07-31 16:53   ` Christoph Hellwig
2015-07-31 17:02     ` James Bottomley
2015-07-31 17:05       ` Dmitry Torokhov
2015-07-31 17:13         ` James Bottomley
2015-07-31 17:33           ` Dmitry Torokhov
2015-07-31 17:36             ` James Bottomley
2015-07-31 18:28               ` Dmitry Torokhov
2015-07-31 18:40                 ` James Bottomley
2015-07-31 19:41                   ` Dmitry Torokhov
2015-08-01 10:57                     ` Mark Brown
2015-08-02 14:05                     ` Russell King - ARM Linux
2015-08-02 14:21                       ` Julia Lawall
2015-08-01 11:04     ` Laurent Pinchart
2015-08-01 11:21       ` Julia Lawall
2015-08-04 12:55         ` Dan Carpenter
2015-08-04 14:01           ` Geert Uytterhoeven
2015-08-04 17:55           ` Dmitry Torokhov
2015-08-04 18:03             ` Julia Lawall
2015-08-04 18:07               ` Dmitry Torokhov
2015-08-04 19:49             ` Russell King - ARM Linux
2015-07-31 17:04 ` Mark Brown
2015-07-31 17:27   ` Russell King - ARM Linux
2015-08-01 10:55     ` Laurent Pinchart
2015-08-01 16:30       ` Russell King - ARM Linux
2015-08-02 23:33         ` Laurent Pinchart
2015-08-01 10:47   ` Laurent Pinchart
2015-08-01 10:55     ` Julia Lawall
2015-08-01 11:01       ` Laurent Pinchart
2015-08-01 15:18         ` Tejun Heo
2015-08-02  0:48           ` Guenter Roeck
2015-08-02 14:30             ` Russell King - ARM Linux
2015-08-02 16:04               ` Guenter Roeck
2015-08-04 10:40               ` Daniel Vetter
2015-08-04 11:18                 ` Russell King - ARM Linux
2015-08-04 11:56                   ` Daniel Vetter
2015-08-04 11:59                     ` Daniel Vetter
2015-08-04 14:48                     ` Tejun Heo
2015-08-04 22:44                     ` Laurent Pinchart
2015-08-05  9:41                       ` Daniel Vetter
2015-08-04 10:49               ` Takashi Iwai
2015-08-10  7:58 ` Linus Walleij
2015-08-10 10:23   ` Russell King - ARM Linux
2015-08-11 11:35     ` Takashi Iwai
2015-08-11 15:19       ` Daniel Vetter
2015-08-21  2:19 ` Dmitry Torokhov
2015-08-21 15:07   ` Julia Lawall
2015-08-21 16:14     ` Dmitry Torokhov
2015-08-21 16:58       ` Mark Brown
2015-08-21 17:30         ` Dmitry Torokhov
2015-08-21 17:41           ` Mark Brown
2015-08-21 17:52             ` Mark Brown
2015-08-21 18:05               ` Dmitry Torokhov
2015-08-21 18:18                 ` Mark Brown
2015-10-12 18:36                   ` Theodore Ts'o
2015-10-12 18:44                     ` Mark Brown
2015-10-14 15:58                       ` Theodore Ts'o

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=2111196.TG1k3f53YQ@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=linux@arm.linux.org.uk \
    --cc=shuah.kh@samsung.com \
    --cc=tj@kernel.org \
    /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