linux-kernel - Re: Dangerous devm_request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20130222182432.GP3570@htj.dyndns.org>
Date:	Fri, 22 Feb 2013 10:24:32 -0800
From:	'Tejun Heo' <tj@...nel.org>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Jingoo Han <jg1.han@...sung.com>,
	'Linus Torvalds' <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	'Andrew Morton' <akpm@...ux-foundation.org>,
	'Al Viro' <viro@...iv.linux.org.uk>
Subject: Re: Dangerous devm_request_irq() conversions

Hey, guys.

On Fri, Feb 22, 2013 at 12:05:48AM -0800, Dmitry Torokhov wrote:
> > Please, argue that it would be better to deprecate devm_request_irq()
> > or devm_request_threaded_irq().
> 
> No, they do have their use. But probably people doing the conversion
> should be required to put $100 in escrow ;)

So, what devm does is simple - it records resources being allocated
using devm and releases them in the reverse order when the driver
detaches from the device.  If all or most resources are covered by
devm, this is the right thing to do most of the time and there's
nothing much to worry about.

However, singling out one resource and converting only that one to
devm is likely to break something if the resource in question has
release ordering dependency and irqs do definitely have ordering
requirements while tearing down, so, no, one can't single out
request_irq() and change only that one to devm_request_irq().  It's
like mixing managed objects with manual new/delete objects.  You can't
do that willy-nilly and it generaly isn't a good idea as evidenced by
only c++ doing it.

The recommended way to do devm conversion is doing it all the way.
Look at the whole subsystem, identify resources used by various
drivers (they usually aren't that diverse in a single subsystem),
identify the pattern and implement helpers in the subsystem as
appropriate and convert the whole thing, preferably to the point where
->remove() almost doesn't have to worry about freeing resources other
than shutting down devices or doing one-off cleanups not covered by
devm.

If there are resources which are frequently used by a group of drivers
but devm interface doesn't currently exist, add a devm interface
either as a generic or subsystem-specific helper.  devm is
specifically designed to allow such extensions.

So, in summary, no, please don't single out irq requests and convert
only those to devm.  That's bound to break stuff.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/