free_irq() can remove an irqaction while the corresponding interrupt is in progress, but free_irq() sets action->thread to NULL unconditionally, which might lead to a NULL pointer dereference in handle_IRQ_event() when the hard interrupt context tries to wake up the handler thread. Prevent this by moving the thread stop after synchronize_irq(). No need to set action->thread to NULL either as action is going to be freed anyway. This fixes a boot crash reported against preempt-rt which uses the mainline irq threads code to implement full irq threading. [ tglx: removed local irqthread variable ]
Unified · Split
| @@ -761,7 +761,6 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) · 760 unmodified lines | |||
| 761 | { | 761 | { |
| 762 | struct irq_desc *desc = irq_to_desc(irq); | 762 | struct irq_desc *desc = irq_to_desc(irq); |
| 763 | struct irqaction *action, **action_ptr; | 763 | struct irqaction *action, **action_ptr; |
| 764 | struct task_struct *irqthread; | ||
| 765 | unsigned long flags; | 764 | unsigned long flags; |
| 766 | 765 | ||
| 767 | WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq); | 766 | WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq); |
| @@ -809,9 +808,6 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) · 41 unmodified lines | |||
| 809 | desc->chip->disable(irq); | 808 | desc->chip->disable(irq); |
| 810 | } | 809 | } |
| 811 | 810 | ||
| 812 | irqthread = action->thread; | ||
| 813 | action->thread = NULL; | ||
| 814 | |||
| 815 | spin_unlock_irqrestore(&desc->lock, flags); | 811 | spin_unlock_irqrestore(&desc->lock, flags); |
| 816 | 812 | ||
| 817 | unregister_handler_proc(irq, action); | 813 | unregister_handler_proc(irq, action); |
| @@ -819,12 +815,6 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) · 1 unmodified line | |||
| 819 | /* Make sure it's not being used on another CPU: */ | 815 | /* Make sure it's not being used on another CPU: */ |
| 820 | synchronize_irq(irq); | 816 | synchronize_irq(irq); |
| 821 | 817 | ||
| 822 | if (irqthread) { | ||
| 823 | if (!test_bit(IRQTF_DIED, &action->thread_flags)) | ||
| 824 | kthread_stop(irqthread); | ||
| 825 | put_task_struct(irqthread); | ||
| 826 | } | ||
| 827 | |||
| 828 | #ifdef CONFIG_DEBUG_SHIRQ | 818 | #ifdef CONFIG_DEBUG_SHIRQ |
| 829 | /* | 819 | /* |
| 830 | * It's a shared IRQ -- the driver ought to be prepared for an IRQ | 820 | * It's a shared IRQ -- the driver ought to be prepared for an IRQ |
| @@ -840,6 +830,13 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) · 9 unmodified lines | |||
| 840 | local_irq_restore(flags); | 830 | local_irq_restore(flags); |
| 841 | } | 831 | } |
| 842 | #endif | 832 | #endif |
| 833 | |||
| 834 | if (action->thread) { | ||
| 835 | if (!test_bit(IRQTF_DIED, &action->thread_flags)) | ||
| 836 | kthread_stop(action->thread); | ||
| 837 | put_task_struct(action->thread); | ||
| 838 | } | ||
| 839 | |||
| 843 | return action; | 840 | return action; |
| 844 | } | 841 | } |
| 845 | 842 | ||