| 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
| ||
|
Date: Tue, 12 Jun 2012 12:24:00 +0200 (CEST)
From: Guennadi Liakhovetski <g.liakhovetski@....de>
To: Ingo Molnar <mingo@...nel.org>
cc: Linus Torvalds <torvalds@...ux-foundation.org>,
linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] irq/core changes for v3.5
Hi Ingo
On Tue, 22 May 2012, Ingo Molnar wrote:
> Linus,
>
> Please pull the latest irq-core-for-linus git tree from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-core-for-linus
>
> HEAD: e0d8ffd1df44518cb9ac9b1807d1f13cc100fc2f hexagon: Remove select of not longer existing Kconfig switches
>
> A collection of small fixes.
>
> Thanks,
>
> Ingo
>
> ------------------>
> Thomas Gleixner (7):
> genirq: Streamline irq_action
> genirq: Reject bogus threaded irq requests
Correct me if I'm wrong, but the above patch, however good and correct in
principle, breaks all existing drivers, that currently use
request_threaded_irq() in this "bogus" way, namely, with NULL handler and
without the IRQF_ONESHOT flag? Of those drivers I counted 73 in today's
Linus' tree with a total of 92 call occurrences. Isn't this a bit too
brutal? How about either doing it more gently (first just issue a warning)
or first fixing them all? The current approach seems to just cause a bunch
of regressions to me? I can provide a list of affected files, if someone
volunteers to fix them all;-)
Thanks
Guennadi
> genirq: Be more informative on irq type mismatch
> genirq: Allow check_wakeup_irqs to notice level-triggered interrupts
> genirq: Do not consider disabled wakeup irqs
> arm: Select core options instead of redefining them
> hexagon: Remove select of not longer existing Kconfig switches
>
>
> arch/arm/Kconfig | 10 +-------
> arch/hexagon/Kconfig | 2 -
> include/linux/interrupt.h | 8 +++---
> kernel/irq/chip.c | 4 ++-
> kernel/irq/manage.c | 46 ++++++++++++++++++++++++++++++--------------
> kernel/irq/pm.c | 7 +++++-
> kernel/irq/resend.c | 7 ++++-
> 7 files changed, 51 insertions(+), 33 deletions(-)
[snip]
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 89a3ea8..585f638 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -565,8 +565,8 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq,
> * IRQF_TRIGGER_* but the PIC does not support multiple
> * flow-types?
> */
> - pr_debug("No set_type function for IRQ %d (%s)\n", irq,
> - chip ? (chip->name ? : "unknown") : "unknown");
> + pr_debug("genirq: No set_type function for IRQ %d (%s)\n", irq,
> + chip ? (chip->name ? : "unknown") : "unknown");
> return 0;
> }
>
> @@ -600,7 +600,7 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq,
> ret = 0;
> break;
> default:
> - pr_err("setting trigger mode %lu for irq %u failed (%pF)\n",
> + pr_err("genirq: Setting trigger mode %lu for irq %u failed (%pF)\n",
> flags, irq, chip->irq_set_type);
> }
> if (unmask)
> @@ -837,8 +837,7 @@ void exit_irq_thread(void)
>
> action = kthread_data(tsk);
>
> - printk(KERN_ERR
> - "exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n",
> + pr_err("genirq: exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n",
> tsk->comm ? tsk->comm : "", tsk->pid, action->irq);
>
> desc = irq_to_desc(action->irq);
> @@ -878,7 +877,6 @@ static int
> __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> {
> struct irqaction *old, **old_ptr;
> - const char *old_name = NULL;
> unsigned long flags, thread_mask = 0;
> int ret, nested, shared = 0;
> cpumask_var_t mask;
> @@ -972,10 +970,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> */
> if (!((old->flags & new->flags) & IRQF_SHARED) ||
> ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
> - ((old->flags ^ new->flags) & IRQF_ONESHOT)) {
> - old_name = old->name;
> + ((old->flags ^ new->flags) & IRQF_ONESHOT))
> goto mismatch;
> - }
>
> /* All handlers must agree on per-cpuness */
> if ((old->flags & IRQF_PERCPU) !=
> @@ -1031,6 +1027,27 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> * all existing action->thread_mask bits.
> */
> new->thread_mask = 1 << ffz(thread_mask);
> +
> + } else if (new->handler == irq_default_primary_handler) {
> + /*
> + * The interrupt was requested with handler = NULL, so
> + * we use the default primary handler for it. But it
> + * does not have the oneshot flag set. In combination
> + * with level interrupts this is deadly, because the
> + * default primary handler just wakes the thread, then
> + * the irq lines is reenabled, but the device still
> + * has the level irq asserted. Rinse and repeat....
> + *
> + * While this works for edge type interrupts, we play
> + * it safe and reject unconditionally because we can't
> + * say for sure which type this interrupt really
> + * has. The type flags are unreliable as the
> + * underlying chip implementation can override them.
> + */
> + pr_err("genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq %d\n",
> + irq);
> + ret = -EINVAL;
> + goto out_mask;
> }
>
> if (!shared) {
> @@ -1078,7 +1095,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
>
> if (nmsk != omsk)
> /* hope the handler works with current trigger mode */
> - pr_warning("IRQ %d uses trigger mode %u; requested %u\n",
> + pr_warning("genirq: irq %d uses trigger mode %u; requested %u\n",
> irq, nmsk, omsk);
> }
>
> @@ -1115,14 +1132,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> return 0;
>
> mismatch:
> -#ifdef CONFIG_DEBUG_SHIRQ
> if (!(new->flags & IRQF_PROBE_SHARED)) {
> - printk(KERN_ERR "IRQ handler type mismatch for IRQ %d\n", irq);
> - if (old_name)
> - printk(KERN_ERR "current handler: %s\n", old_name);
> + pr_err("genirq: Flags mismatch irq %d. %08x (%s) vs. %08x (%s)\n",
> + irq, new->flags, new->name, old->flags, old->name);
> +#ifdef CONFIG_DEBUG_SHIRQ
> dump_stack();
> - }
> #endif
> + }
> ret = -EBUSY;
>
> out_mask:
[snip]
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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/
Powered by blists - more mailing lists