[PATCH 1/3 v3] genirq: Prevent from early irq thread spurious wake-ups

Frederic Weisbecker posted 3 patches 1 week, 3 days ago
[PATCH 1/3 v3] genirq: Prevent from early irq thread spurious wake-ups
Posted by Frederic Weisbecker 1 week, 3 days ago
From: Thomas Gleixner <tglx@linutronix.de>

During initialization, the IRQ thread is created before the IRQ get a
chance to be enabled. But the IRQ enablement may happen before the first
official kthread wake up point. As a result, the firing IRQ can perform
an early wake-up of the IRQ thread before the first official kthread
wake up point.

Although this has happened to be harmless so far, this uncontrolled
behaviour is a bug waiting to happen at some point in the future with
the threaded handler accessing halfway initialized states.

Prevent from such surprise with performing a wake-up only if the target
is in TASK_INTERRUPTIBLE state. Since the IRQ thread waits in this state
for interrupts to handle only after proper initialization, it is then
guaranteed not to be spuriously woken up while waiting in
TASK_UNINTERRUPTIBLE, right after creation in the kthread code, before
the official first wake up point to be reached.

Not-yet-Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/irq/handle.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index e103451243a0..786f5570a640 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -133,7 +133,15 @@ void __irq_wake_thread(struct irq_desc *desc, struct irqaction *action)
 	 */
 	atomic_inc(&desc->threads_active);
 
-	wake_up_process(action->thread);
+	/*
+	 * This might be a premature wakeup before the thread reached the
+	 * thread function and set the IRQTF_READY bit. It's waiting in
+	 * kthread code with state UNINTERRUPTIBLE. Once it reaches the
+	 * thread function it waits with INTERRUPTIBLE. The wakeup is not
+	 * lost in that case because the thread is guaranteed to observe
+	 * the RUN flag before it goes to sleep in wait_for_interrupt().
+	 */
+	wake_up_state(action->thread, TASK_INTERRUPTIBLE);
 }
 
 static DEFINE_STATIC_KEY_FALSE(irqhandler_duration_check_enabled);
-- 
2.51.1
Re: [PATCH 1/3 v3] genirq: Prevent from early irq thread spurious wake-ups
Posted by Thomas Gleixner 1 week, 3 days ago
On Fri, Nov 21 2025 at 15:34, Frederic Weisbecker wrote:
> During initialization, the IRQ thread is created before the IRQ get a
> chance to be enabled. But the IRQ enablement may happen before the first
> official kthread wake up point. As a result, the firing IRQ can perform
> an early wake-up of the IRQ thread before the first official kthread
> wake up point.
>
> Although this has happened to be harmless so far, this uncontrolled
> behaviour is a bug waiting to happen at some point in the future with
> the threaded handler accessing halfway initialized states.

No. At the point where the first wake up can happen, the state used by
the thread is completely initialized. That's right after setup_irq()
drops the descriptor lock. Even if the hardware raises it immediately on
starting the interrupt up, the handler is stuck on the descriptor lock,
which is not released before everything is ready.

That kthread_bind() issue is a special case as it makes the assumption
that the thread is still in that UNINTERRUPTIBLE state waiting for the
initial wake up. That assumption is only true, when the thread creator
guarantees that there is no wake up before kthread_bind() is invoked.

I'll rephrase that a bit. :)
Re: [PATCH 1/3 v3] genirq: Prevent from early irq thread spurious wake-ups
Posted by Frederic Weisbecker 1 week, 3 days ago
Le Fri, Nov 21, 2025 at 08:12:38PM +0100, Thomas Gleixner a écrit :
> On Fri, Nov 21 2025 at 15:34, Frederic Weisbecker wrote:
> > During initialization, the IRQ thread is created before the IRQ get a
> > chance to be enabled. But the IRQ enablement may happen before the first
> > official kthread wake up point. As a result, the firing IRQ can perform
> > an early wake-up of the IRQ thread before the first official kthread
> > wake up point.
> >
> > Although this has happened to be harmless so far, this uncontrolled
> > behaviour is a bug waiting to happen at some point in the future with
> > the threaded handler accessing halfway initialized states.
> 
> No. At the point where the first wake up can happen, the state used by
> the thread is completely initialized. That's right after setup_irq()
> drops the descriptor lock. Even if the hardware raises it immediately on
> starting the interrupt up, the handler is stuck on the descriptor lock,
> which is not released before everything is ready.
> 
> That kthread_bind() issue is a special case as it makes the assumption
> that the thread is still in that UNINTERRUPTIBLE state waiting for the
> initial wake up. That assumption is only true, when the thread creator
> guarantees that there is no wake up before kthread_bind() is invoked.
> 
> I'll rephrase that a bit. :)

Eh, thanks and sorry for the misinterpretation.

-- 
Frederic Weisbecker
SUSE Labs
[tip: irq/core] genirq: Prevent early spurious wake-ups of interrupt threads
Posted by tip-bot2 for Frederic Weisbecker 1 week, 2 days ago
The following commit has been merged into the irq/core branch of tip:

Commit-ID:     68775ca79af3b8d4c147598983ece012d7007bac
Gitweb:        https://git.kernel.org/tip/68775ca79af3b8d4c147598983ece012d7007bac
Author:        Frederic Weisbecker <frederic@kernel.org>
AuthorDate:    Fri, 21 Nov 2025 15:34:58 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 22 Nov 2025 09:26:18 +01:00

genirq: Prevent early spurious wake-ups of interrupt threads

During initialization, the interrupt thread is created before the interrupt
is enabled. The interrupt enablement happens before the actual kthread wake
up point. Once the interrupt is enabled the hardware can raise an interrupt
and once setup_irq() drops the descriptor lock a interrupt wake-up can
happen.

Even when such an interrupt can be considered premature, this is not a
problem in general because at the point where the descriptor lock is
dropped and the wakeup can happen, the data which is used by the thread is
fully initialized.

Though from the perspective of least surprise, the initial wakeup really
should be performed by the setup code and not randomly by a premature
interrupt.

Prevent this by performing a wake-up only if the target is in state
TASK_INTERRUPTIBLE, which the thread uses in wait_for_interrupt().

If the thread is still in state TASK_UNINTERRUPTIBLE, the wake-up is not
lost because after the setup code completed the initial wake-up the thread
will observe the IRQTF_RUNTHREAD and proceed with the handling.

[ tglx: Simplified the changes and extended the changelog. ]

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://patch.msgid.link/20251121143500.42111-2-frederic@kernel.org
---
 kernel/irq/handle.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index e103451..786f557 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -133,7 +133,15 @@ void __irq_wake_thread(struct irq_desc *desc, struct irqaction *action)
 	 */
 	atomic_inc(&desc->threads_active);
 
-	wake_up_process(action->thread);
+	/*
+	 * This might be a premature wakeup before the thread reached the
+	 * thread function and set the IRQTF_READY bit. It's waiting in
+	 * kthread code with state UNINTERRUPTIBLE. Once it reaches the
+	 * thread function it waits with INTERRUPTIBLE. The wakeup is not
+	 * lost in that case because the thread is guaranteed to observe
+	 * the RUN flag before it goes to sleep in wait_for_interrupt().
+	 */
+	wake_up_state(action->thread, TASK_INTERRUPTIBLE);
 }
 
 static DEFINE_STATIC_KEY_FALSE(irqhandler_duration_check_enabled);
[tip: irq/core] genirq: Prevent early spurious wake-ups of interrupt threads
Posted by tip-bot2 for Thomas Gleixner 1 week, 3 days ago
The following commit has been merged into the irq/core branch of tip:

Commit-ID:     9d5ca2edd74e479ad09bc7d02820395a9d46e2bd
Gitweb:        https://git.kernel.org/tip/9d5ca2edd74e479ad09bc7d02820395a9d46e2bd
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Fri, 21 Nov 2025 15:34:58 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 21 Nov 2025 20:50:30 +01:00

genirq: Prevent early spurious wake-ups of interrupt threads

During initialization, the interrupt thread is created before the interrupt
is enabled. The interrupt enablement happens before the actual kthread wake
up point. Once the interrupt is enabled the hardware can raise an interrupt
and once setup_irq() drops the descriptor lock a interrupt wake-up can
happen.

Even when such an interrupt can be considered premature, this is not a
problem in general because at the point where the descriptor lock is
dropped and the wakeup can happen, the data which is used by the thread is
fully initialized.

Though from the perspective of least surprise, the initial wakeup really
should be performed by the setup code and not randomly by a premature
interrupt.

Prevent this by performing a wake-up only if the target is in state
TASK_INTERRUPTIBLE, which the thread uses in wait_for_interrupt().

If the thread is still in state TASK_UNINTERRUPTIBLE, the wake-up is not
lost because after the setup code completed the initial wake-up the thread
will observe the IRQTF_RUNTHREAD and proceed with the handling.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://patch.msgid.link/20251121143500.42111-2-frederic@kernel.org
---
 kernel/irq/handle.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index e103451..786f557 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -133,7 +133,15 @@ void __irq_wake_thread(struct irq_desc *desc, struct irqaction *action)
 	 */
 	atomic_inc(&desc->threads_active);
 
-	wake_up_process(action->thread);
+	/*
+	 * This might be a premature wakeup before the thread reached the
+	 * thread function and set the IRQTF_READY bit. It's waiting in
+	 * kthread code with state UNINTERRUPTIBLE. Once it reaches the
+	 * thread function it waits with INTERRUPTIBLE. The wakeup is not
+	 * lost in that case because the thread is guaranteed to observe
+	 * the RUN flag before it goes to sleep in wait_for_interrupt().
+	 */
+	wake_up_state(action->thread, TASK_INTERRUPTIBLE);
 }
 
 static DEFINE_STATIC_KEY_FALSE(irqhandler_duration_check_enabled);