linux-kernel - Re: [BUG] irq_dispose_mapping after irq request failure

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]

Message-Id: <20130211205255.72B513E3530@localhost>
Date:	Mon, 11 Feb 2013 20:52:55 +0000
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Michael Ellerman <michael@...erman.id.au>,
	Baruch Siach <baruch@...s.co.il>
Cc:	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [BUG] irq_dispose_mapping after irq request failure

On Mon, 11 Feb 2013 17:19:49 +1100, Michael Ellerman <michael@...erman.id.au> wrote:
> On Mon, Feb 11, 2013 at 07:31:00AM +0200, Baruch Siach wrote:
> > Hi lkml,
> 
> Hi Baruch,
> 
> > The drivers/edac/mpc85xx_edac.c driver contains the following (abbreviated)
> > code snippet it its .probe:
> 
> You dropped an important detail which is the preceeding line:
> 
> 	pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
> 
> > 		res = devm_request_irq(&op->dev, pdata->irq,
> > 				       mpc85xx_pci_isr, IRQF_DISABLED,
> > 				       "[EDAC] PCI err", pci);
> > 		if (res < 0) {
> > 			irq_dispose_mapping(pdata->irq);
> > 			goto err2;
> > 		}
> > 
> > Now, since the requested irq is already in use, and IRQF_SHARED is not set,
> > devm_request_irq errors() out, which is OK. Less OK is the
> > irq_dispose_mapping() call, which gives me this:
> > 
> > EDAC PCI1: Giving out device to module 'MPC85xx_edac' controller 'mpc85xx_pci_err': DEV 'ffe0a000.pcie' (INTERRUPT)
> > genirq: Flags mismatch irq 16. 00000020 ([EDAC] PCI err) vs. 00000020 ([EDAC] PCI err)
> 
> The hint here is to notice which other irq you're clashing with          ^^
> ie. yourself. Which is odd, that is the root of the problem.
> 
> The badness you're getting from irq_dispose_mapping() is caused because you're
> disposing of that mapping which is currently still in use, by the same interrupt.
> 
> That is caused by a "feature" in the irq mapping code, where if you ask to map an
> already mapped hwirq, it will give you back the same virq. So in your case when
> you called irq_of_parse_and_map() it noticed that someone had already mapped
> that hwirq, and gave you back an existing (in use) virq.

Really the irq mappings should be using reference counting. The existing
code is naive on this count and just releases the irq on the first call
to irq_dispose_mapping().  I've not gotten around to fixing that. Anyone
want to take that task on?

g.

--
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/