diff options
author | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-05-24 08:37:14 -0700 |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-05-24 08:37:14 -0700 |
commit | 92ea77275b5345c1300433f28689493dc4163f24 (patch) | |
tree | 8813e2453b081d700ae32b7dc6f056f2eba8ebe7 /kernel/irq/spurious.c | |
parent | db2668fdbeb2e3c95ebadf95856c9e31a8a8d569 (diff) |
Fix crash with irqpoll due to the IRQF_IRQPOLL flag testing
With irqpoll enabled, trying to test the IRQF_IRQPOLL flag in the
actions would cause a NULL pointer dereference if no action was
installed (for example, the driver might have been unloaded with
interrupts still pending).
So be a bit more careful about testing the flag by making sure to test
for that case.
(The actual _change_ is trivial, the patch is more than a one-liner
because I rewrote the testing to also be much more readable.
Original (discarded) bugfix by Bernhard Walle.
Cc: Bernhard Walle <bwalle@suse.de>
Tested-by: Vivek Goyal <vgoyal@in.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'kernel/irq/spurious.c')
-rw-r--r-- | kernel/irq/spurious.c | 46 |
1 files changed, 37 insertions, 9 deletions
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c index b0d81aae472..bd9e272d55e 100644 --- a/kernel/irq/spurious.c +++ b/kernel/irq/spurious.c @@ -135,6 +135,39 @@ report_bad_irq(unsigned int irq, struct irq_desc *desc, irqreturn_t action_ret) } } +static inline int try_misrouted_irq(unsigned int irq, struct irq_desc *desc, irqreturn_t action_ret) +{ + struct irqaction *action; + + if (!irqfixup) + return 0; + + /* We didn't actually handle the IRQ - see if it was misrouted? */ + if (action_ret == IRQ_NONE) + return 1; + + /* + * But for 'irqfixup == 2' we also do it for handled interrupts if + * they are marked as IRQF_IRQPOLL (or for irq zero, which is the + * traditional PC timer interrupt.. Legacy) + */ + if (irqfixup < 2) + return 0; + + if (!irq) + return 1; + + /* + * Since we don't get the descriptor lock, "action" can + * change under us. We don't really care, but we don't + * want to follow a NULL pointer. So tell the compiler to + * just load it once by using a barrier. + */ + action = desc->action; + barrier(); + return action && (action->flags & IRQF_IRQPOLL); +} + void note_interrupt(unsigned int irq, struct irq_desc *desc, irqreturn_t action_ret) { @@ -144,15 +177,10 @@ void note_interrupt(unsigned int irq, struct irq_desc *desc, report_bad_irq(irq, desc, action_ret); } - if (unlikely(irqfixup)) { - /* Don't punish working computers */ - if ((irqfixup == 2 && ((irq == 0) || - (desc->action->flags & IRQF_IRQPOLL))) || - action_ret == IRQ_NONE) { - int ok = misrouted_irq(irq); - if (action_ret == IRQ_NONE) - desc->irqs_unhandled -= ok; - } + if (unlikely(try_misrouted_irq(irq, desc, action_ret))) { + int ok = misrouted_irq(irq); + if (action_ret == IRQ_NONE) + desc->irqs_unhandled -= ok; } desc->irq_count++; |