linux-kernel - Re: [PATCH 2/2] chipidea: Use 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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 31 Jul 2013 12:18:53 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Tejun Heo <htejun@...il.com>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Peter Chen <peter.chen@...escale.com>,
	Fabio Estevam <festevam@...il.com>,
	alexander.shishkin@...ux.intel.com, kernel@...gutronix.de,
	linux-usb@...r.kernel.org,
	Fabio Estevam <fabio.estevam@...escale.com>,
	Jeff Garzik <jeff@...zik.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org,
	Mike Turquette <mturquette@...aro.org>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] chipidea: Use devm_request_irq()

On Wed, Jul 31, 2013 at 05:54:11AM -0400, Tejun Heo wrote:
> On Wed, Jul 31, 2013 at 11:44:34AM +0200, Uwe Kleine-König wrote:

> > > > OK, so the possible problem is that remove is called while the irq is
> > > > still active. That means you have to assert that all resources the irq

> If your driver destruction path is running while your irq handler is
> still running, it's a crappy / broken driver.  You need a deactivation
> step whether you're using devm or not.  IRQs can be shared and the
> device should be in a quiesced state before the driver detaches
> itself.  Note that you can queue deactivation routine using devm.  For
> an example, please take a look at
> drivers/ata/libata-core.c::ata_host_start().

I'm not sure I understand how this relates the problem.  The main issue
here is that for the shared IRQ case quiescing the device doesn't make
any difference since one of the other users of the interrupt could cause
the interrupt handler to be called regardless of what the hardware is
doing.  This means that we need to guarantee that anything the interrupt
handler relies on has not been deallocated before the interrupt handler
is unregistered.

> irq deregistration.  Add an explicit deactivation step using
> devres_alloc().

> devm guarantees that the destruction callbacks are called in the
> reverse order of registration.

OK, that's helpful.  It'd be good to document this if it's something
the API is intending to guarantee, though - devres.txt doesn't mention
this and it's not something I'd intuitively expect to be the case.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)