On Thu, 24 May 2018, Lukas Wunner wrote:
> static int irq_wait_for_interrupt(struct irqaction *action)
> {
> - set_current_state(TASK_INTERRUPTIBLE);
> + for (;;) {
> + set_current_state(TASK_INTERRUPTIBLE);
>
> - while (!kthread_should_stop()) {
> + if (kthread_should_stop()) {
> + /* may need to run one last time. */
> + if (test_and_clear_bit(IRQTF_RUNTHREAD,
> + &action->thread_flags)) {
> + __set_current_state(TASK_RUNNING);
> + return 0;
> + }
> + __set_current_state(TASK_RUNNING);
> + return -1;
> + }
>
> if (test_and_clear_bit(IRQTF_RUNTHREAD,
> &action->thread_flags)) {
> @@ -766,10 +776,7 @@ static int irq_wait_for_interrupt(struct irqaction
> *action)
> return 0;
> }
> schedule();
> - set_current_state(TASK_INTERRUPTIBLE);
> }
> - __set_current_state(TASK_RUNNING);
> - return -1;
> }
>
> /*
> @@ -990,7 +997,7 @@ static int irq_thread(void *data)
> /*
> * This is the regular exit path. __free_irq() is stopping the
> * thread via kthread_stop() after calling
> - * synchronize_irq(). So neither IRQTF_RUNTHREAD nor the
> + * synchronize_hardirq(). So neither IRQTF_RUNTHREAD nor the
> * oneshot mask bit can be set. We cannot verify that as we
> * cannot touch the oneshot mask at this point anymore as
> * __setup_irq() might have given out currents thread_mask
> @@ -1595,7 +1602,7 @@ static struct irqaction *__free_irq(struct irq_desc
> *desc, void *dev_id)
> unregister_handler_proc(irq, action);
>
> /* Make sure it's not being used on another CPU: */
> - synchronize_irq(irq);
> + synchronize_hardirq(irq);
So after that, action can be freed and when the thread above tries to
access it. Nice Use After Free. That needs more thought.
Thanks,
tglx
