linux-kernel - Re: [GIT PULL] irq/core changes for v3.5

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, 13 Jun 2012 08:30:53 +0300
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Guennadi Liakhovetski <g.liakhovetski@....de>,
	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] irq/core changes for v3.5

On Tue, Jun 12, 2012 at 7:47 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
>
> So now we have the choice of:
>
>   - Leaving the current check and regress 90+ drivers
>
>   - Leaving the current check and fix 90+ broken drivers
>
>   - Reverting it and end up with no protection at all
>
>   - Forcing the flag and risking the wreckage of two oddball drivers
>     _IF_ they ever show up on a PC platform.

So I'd really prefer #2.

I'd rather have a nice sane generic irq layer that really makes it an
*error* to do stupid things that make no sense, and then fix the
drivers that do them.

I would assume that fixing the drivers should be simple, and could
even be done largely by simply just grepping for the pattern of "NULL
irq-time handler without the proper markers to show that the driver
author knew what the hell they were doing".

Because I really do think that your suggested "let's work around it"
approach breaks the *wrong* driver, and does so very subtly. If you
have a buggy driver that first registers a threaded interrupt and that
silently gets turned into a IRQF_ONESHOT, and then you have the case
of a driver later coming in that would like to share the irq (but
doesn't use threading, and wants the sane shared level semantics), you
now disallow that second (non-buggy) request.

And clearly that kind of sharing cannot work, but the driver author
that looks at his (non-buggy) driver and then maybe even understands
what the problem is, and starts looking at the *buggy* driver, doesn't
actually *see* that the buggy drver uses IRQF_ONESHOT.

See what I'm trying to say? The "implicit IRQF_ONESHOT" model
basically hides a failure point in a subtle way. If we just make it a
hard requirement that drivers have to use sane models, the driver that
wants the one-shot behavior (and *depends* on it, because it doesn't
have an interrupt-time routine at all), will have to explicitly mark
itself that way, so that *other* drivers - that might be impacted by
that choice - can see it, and can see why they can't share the irq.

I relaize that the current drivers that rely on our old fast-and-loose
behavior actually *work*, and I realize that apparently there aren't
any shared irq issues in existence today, but I'd like our generic irq
code to do the RightThing(tm), and I think that implies that it is the
drivers that should be fixed, not the core irq layer that should work
around the problem.

Driver authors that see the error should be able to easily fix their
drivers, no?

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