linux-kernel - Re: [PATCH 05/15] drivers/char: minor irq handler cleanups

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: <1208861168.9640.109.camel@pasglop>
Date:	Tue, 22 Apr 2008 20:46:08 +1000
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Rogier Wolff <R.E.Wolff@...Wizard.nl>
Cc:	Jeff Garzik <jeff@...zik.org>, alan@...hat.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/15] drivers/char: minor irq handler cleanups


On Tue, 2008-04-22 at 12:13 +0200, Rogier Wolff wrote:
> > You should let the kernel generic code deal with the runaway
> interrupt,
> > it should be capable of doing so nowadays pretty reliably.
> > 
> > free_irq() is definitely not going to be happy when it start messing
> > with /proc from an interrupt... It will at least give you a WARN_ON.
> 
> The situation is NOT normal operation. It is an emergency measure in
> an attempt to prevent a full hang. It is great that other parts of the
> kernel also "shout" that something is wrong.

But free_irq at interrupt time has great chances of crashing the machine
or currupting data structures in subtle ways. You should -not- do that.

Either let the kernel handle the runaway interrupt, or eventually if you
want to be brutal, call disable_irq_nosync yourself, but not free_irq.

> Consider it similar to a "kernel null pointer dereference". Once that
> happens, all bets are off. In practise you've probably seen one, and
> you were able to continue to work. It is advisable to save everything
> you can, and reboot. This is similar. 

It would be if you used something like disable_irq_nosync. Not free_irq.
The later is just bogus.

> The "generic code for runaway interrupts" didn't exist when this was
> written. If it exists, and works for the case that this was written
> for, then all is fine, and we can remove my code. As you can see, I
> copied over the code from one driver to the next after I got bitten
> again with the second driver. So having something generic is of course
> preferable. :-)

Well, if you know how to trigger the bug, it would be useful to verify
that the kernel generic code properly detects and masks the runaway
interrupt. If that works, then remove your code completely. If not, it
would be useful to figure out why :-) But either way, just replace it
with disable_irq_nosync.

Cheers,
Ben.


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