[PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Deepanshu Kartikey]

lock_release() performs a raw_local_irq_save/restore dance around its
validation work. While safe in process and softirq context, this is
dangerous in hardirq context where IRQs must remain disabled for the
entire duration of the handler.

When lock_release() calls raw_local_irq_restore() inside a hardirq
handler, it briefly re-enables IRQs, creating a window where a new
interrupt can fire before the handler returns. This was observed with
taprio's advance_sched() hrtimer callback - the temporary IRQ
re-enablement inside lock_release() prevented CPU 0 from acknowledging
a pending TLB flush IPI sent by CPU 1 via smp_call_function_many().
CPU 1 then spun indefinitely in csd_lock_wait(), starving the RCU
grace-period kthread and triggering an RCU stall with eventual OOM.

lock_acquire() already handles the NMI case specially via lockdep_nmi()
to avoid this class of problem. Mirror that pattern for hardirq context
in lock_release() by introducing lockdep_hardirq() and skipping the
irq save/restore dance when called from hardirq context.

Reported-by: syzbot+0635dc2e2c3c21a6aa04@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0635dc2e2c3c21a6aa04
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 kernel/locking/lockdep.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 2d4c5bab5af8..17eb9590e751 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -5872,6 +5872,15 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 }
 EXPORT_SYMBOL_GPL(lock_acquire);
 
+static bool lockdep_hardirq(void)
+{
+	if (raw_cpu_read(lockdep_recursion))
+		return false;
+	if (!in_hardirq())
+		return false;
+	return true;
+}
+
 void lock_release(struct lockdep_map *lock, unsigned long ip)
 {
 	unsigned long flags;
@@ -5882,6 +5891,14 @@ void lock_release(struct lockdep_map *lock, unsigned long ip)
 		     lock->key == &__lockdep_no_track__))
 		return;
 
+	if (lockdep_hardirq()) {
+		lockdep_recursion_inc();
+		if (__lock_release(lock, ip))
+			check_chain_key(current);
+		lockdep_recursion_finish();
+		return;
+	}
+
 	raw_local_irq_save(flags);
 	check_flags(flags);
 
-- 
2.43.0

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Waiman Long]

On 6/29/26 12:11 AM, Deepanshu Kartikey wrote:
> lock_release() performs a raw_local_irq_save/restore dance around its
> validation work. While safe in process and softirq context, this is
> dangerous in hardirq context where IRQs must remain disabled for the
> entire duration of the handler.
>
> When lock_release() calls raw_local_irq_restore() inside a hardirq
> handler, it briefly re-enables IRQs, creating a window where a new
> interrupt can fire before the handler returns. This was observed with
> taprio's advance_sched() hrtimer callback - the temporary IRQ
> re-enablement inside lock_release() prevented CPU 0 from acknowledging
> a pending TLB flush IPI sent by CPU 1 via smp_call_function_many().
> CPU 1 then spun indefinitely in csd_lock_wait(), starving the RCU
> grace-period kthread and triggering an RCU stall with eventual OOM.
Where exactly is the temporary window when interrupt is enabled during 
the raw_local_irq_restore() call? Interrupt handling is arch specific. 
Is it specific to certain architectures?
>
> lock_acquire() already handles the NMI case specially via lockdep_nmi()
> to avoid this class of problem. Mirror that pattern for hardirq context
> in lock_release() by introducing lockdep_hardirq() and skipping the
> irq save/restore dance when called from hardirq context.
>
> Reported-by: syzbot+0635dc2e2c3c21a6aa04@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0635dc2e2c3c21a6aa04
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>   kernel/locking/lockdep.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 2d4c5bab5af8..17eb9590e751 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -5872,6 +5872,15 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
>   }
>   EXPORT_SYMBOL_GPL(lock_acquire);
>   
> +static bool lockdep_hardirq(void)
> +{
> +	if (raw_cpu_read(lockdep_recursion))
> +		return false;
> +	if (!in_hardirq())
> +		return false;
> +	return true;
> +}
> +
The lockdep_nmi() is for a different use case where we want to save some 
lockdep information in nmi context and so checking lockdep recursion 
makes sense. If you want to always disable the irq_save/restore 
operation in hardirq context, I doubt you want to check for lockdep 
recursion.
>   void lock_release(struct lockdep_map *lock, unsigned long ip)
>   {
>   	unsigned long flags;
> @@ -5882,6 +5891,14 @@ void lock_release(struct lockdep_map *lock, unsigned long ip)
>   		     lock->key == &__lockdep_no_track__))
>   		return;
>   
> +	if (lockdep_hardirq()) {
> +		lockdep_recursion_inc();
> +		if (__lock_release(lock, ip))
> +			check_chain_key(current);
> +		lockdep_recursion_finish();
> +		return;
> +	}
> +
>   	raw_local_irq_save(flags);
>   	check_flags(flags);
>   

I believe the code change will be easier to understand if you just 
conditionally disable the irq_save/restore when in hardirq context 
instead of duplicating the remaining code without irq_save/restore.

Cheers,
Longman

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Deepanshu Kartikey]

On Mon, Jun 29, 2026 at 10:57 AM Waiman Long <longman@redhat.com> wrote:
>
>
> On 6/29/26 12:11 AM, Deepanshu Kartikey wrote:
> > lock_release() performs a raw_local_irq_save/restore dance around its
> > validation work. While safe in process and softirq context, this is
> > dangerous in hardirq context where IRQs must remain disabled for the
> > entire duration of the handler.
> >
> > When lock_release() calls raw_local_irq_restore() inside a hardirq
> > handler, it briefly re-enables IRQs, creating a window where a new
> > interrupt can fire before the handler returns. This was observed with
> > taprio's advance_sched() hrtimer callback - the temporary IRQ
> > re-enablement inside lock_release() prevented CPU 0 from acknowledging
> > a pending TLB flush IPI sent by CPU 1 via smp_call_function_many().
> > CPU 1 then spun indefinitely in csd_lock_wait(), starving the RCU
> > grace-period kthread and triggering an RCU stall with eventual OOM.

> Where exactly is the temporary window when interrupt is enabled during
> the raw_local_irq_restore() call? Interrupt handling is arch specific.
> Is it specific to certain architectures?

On x86, raw_local_irq_restore() executes the 'sti' instruction which
immediately re-enables IRQs. Inside lock_release(), after the validation
work completes, calling raw_local_irq_restore() with the saved flags
will execute 'sti' even when called from hardirq context.

The window is between the 'sti' instruction (which re-enables IRQs) and
the return from lock_release(). During this window, a new interrupt can
fire and hijack the CPU before the hardirq handler can return and
acknowledge pending IPIs.

In the syzkaller trace, this window allowed a new IRQ to fire on CPU 0
after lock_release()'s sti, preventing CPU 0 from ever acknowledging
the TLB flush IPI sent by CPU 1, causing CPU 1 to spin indefinitely in
csd_lock_wait(), which starved the RCU grace-period kthread.

The fix is correct on all architectures - hardirq context must never
restore IRQs mid-handler since the hardware manages IRQ state for
interrupt entry/exit. This is why we conditionally skip the
irq_save/restore dance when in_hardirq() is true.


> I believe the code change will be easier to understand if you just
> conditionally disable the irq_save/restore when in hardirq context
> instead of duplicating the remaining code without irq_save/restore.
>
> Cheers,
> Longman
>

I have sent patch v2.

Deepanshu

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Waiman Long]

On 6/29/26 7:44 PM, Deepanshu Kartikey wrote:
> On Mon, Jun 29, 2026 at 10:57 AM Waiman Long <longman@redhat.com> wrote:
>>
>> On 6/29/26 12:11 AM, Deepanshu Kartikey wrote:
>>> lock_release() performs a raw_local_irq_save/restore dance around its
>>> validation work. While safe in process and softirq context, this is
>>> dangerous in hardirq context where IRQs must remain disabled for the
>>> entire duration of the handler.
>>>
>>> When lock_release() calls raw_local_irq_restore() inside a hardirq
>>> handler, it briefly re-enables IRQs, creating a window where a new
>>> interrupt can fire before the handler returns. This was observed with
>>> taprio's advance_sched() hrtimer callback - the temporary IRQ
>>> re-enablement inside lock_release() prevented CPU 0 from acknowledging
>>> a pending TLB flush IPI sent by CPU 1 via smp_call_function_many().
>>> CPU 1 then spun indefinitely in csd_lock_wait(), starving the RCU
>>> grace-period kthread and triggering an RCU stall with eventual OOM.
>> Where exactly is the temporary window when interrupt is enabled during
>> the raw_local_irq_restore() call? Interrupt handling is arch specific.
>> Is it specific to certain architectures?
> On x86, raw_local_irq_restore() executes the 'sti' instruction which
> immediately re-enables IRQs. Inside lock_release(), after the validation
> work completes, calling raw_local_irq_restore() with the saved flags
> will execute 'sti' even when called from hardirq context.
>
> The window is between the 'sti' instruction (which re-enables IRQs) and
> the return from lock_release(). During this window, a new interrupt can
> fire and hijack the CPU before the hardirq handler can return and
> acknowledge pending IPIs.
>
> In the syzkaller trace, this window allowed a new IRQ to fire on CPU 0
> after lock_release()'s sti, preventing CPU 0 from ever acknowledging
> the TLB flush IPI sent by CPU 1, causing CPU 1 to spin indefinitely in
> csd_lock_wait(), which starved the RCU grace-period kthread.
>
> The fix is correct on all architectures - hardirq context must never
> restore IRQs mid-handler since the hardware manages IRQ state for
> interrupt entry/exit. This is why we conditionally skip the
> irq_save/restore dance when in_hardirq() is true.

I looked at the generated code of raw_local_irq_restore():

./arch/x86/include/asm/irqflags.h:
146        return !(flags & X86_EFLAGS_IF);
    0x00000000000082b9 <+9>:    test   $0x200,%edi
    0x00000000000082bf <+15>:    je     0x82c2 <cpuset_test+18>

42        asm volatile("sti": : :"memory");
    0x00000000000082c1 <+17>:    sti

kernel/cgroup/cpuset.c:
4553    }
    0x00000000000082c2 <+18>:    jmp    0x82c7

sti should only be called if the saved flags has the IF bit set. In 
hardirq context, the IF bit shouldn't be set. Is my interpretation correct?

Regards,
Longman

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Waiman Long]

On 6/30/26 12:57 AM, Waiman Long wrote:
> On 6/29/26 7:44 PM, Deepanshu Kartikey wrote:
>> On Mon, Jun 29, 2026 at 10:57 AM Waiman Long <longman@redhat.com> wrote:
>>>
>>> On 6/29/26 12:11 AM, Deepanshu Kartikey wrote:
>>>> lock_release() performs a raw_local_irq_save/restore dance around its
>>>> validation work. While safe in process and softirq context, this is
>>>> dangerous in hardirq context where IRQs must remain disabled for the
>>>> entire duration of the handler.
>>>>
>>>> When lock_release() calls raw_local_irq_restore() inside a hardirq
>>>> handler, it briefly re-enables IRQs, creating a window where a new
>>>> interrupt can fire before the handler returns. This was observed with
>>>> taprio's advance_sched() hrtimer callback - the temporary IRQ
>>>> re-enablement inside lock_release() prevented CPU 0 from acknowledging
>>>> a pending TLB flush IPI sent by CPU 1 via smp_call_function_many().
>>>> CPU 1 then spun indefinitely in csd_lock_wait(), starving the RCU
>>>> grace-period kthread and triggering an RCU stall with eventual OOM.
>>> Where exactly is the temporary window when interrupt is enabled during
>>> the raw_local_irq_restore() call? Interrupt handling is arch specific.
>>> Is it specific to certain architectures?
>> On x86, raw_local_irq_restore() executes the 'sti' instruction which
>> immediately re-enables IRQs. Inside lock_release(), after the validation
>> work completes, calling raw_local_irq_restore() with the saved flags
>> will execute 'sti' even when called from hardirq context.
>>
>> The window is between the 'sti' instruction (which re-enables IRQs) and
>> the return from lock_release(). During this window, a new interrupt can
>> fire and hijack the CPU before the hardirq handler can return and
>> acknowledge pending IPIs.
>>
>> In the syzkaller trace, this window allowed a new IRQ to fire on CPU 0
>> after lock_release()'s sti, preventing CPU 0 from ever acknowledging
>> the TLB flush IPI sent by CPU 1, causing CPU 1 to spin indefinitely in
>> csd_lock_wait(), which starved the RCU grace-period kthread.
>>
>> The fix is correct on all architectures - hardirq context must never
>> restore IRQs mid-handler since the hardware manages IRQ state for
>> interrupt entry/exit. This is why we conditionally skip the
>> irq_save/restore dance when in_hardirq() is true.
>
> I looked at the generated code of raw_local_irq_restore():
>
> ./arch/x86/include/asm/irqflags.h:
> 146        return !(flags & X86_EFLAGS_IF);
>    0x00000000000082b9 <+9>:    test   $0x200,%edi
>    0x00000000000082bf <+15>:    je     0x82c2 <cpuset_test+18>
>
> 42        asm volatile("sti": : :"memory");
>    0x00000000000082c1 <+17>:    sti
>
> kernel/cgroup/cpuset.c:
> 4553    }
>    0x00000000000082c2 <+18>:    jmp    0x82c7
>
> sti should only be called if the saved flags has the IF bit set. In 
> hardirq context, the IF bit shouldn't be set. Is my interpretation 
> correct? 

Or is it because x86 can speculatively execute an sti instruction even 
if it is the wrong branch and there is a small window that interrupt is 
enabled before it is rolled back?

Cheers,
Longman

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Deepanshu Kartikey]

On Tue, Jun 30, 2026 at 10:28 AM Waiman Long <longman@redhat.com> wrote:
>
>
> I looked at the generated code of raw_local_irq_restore():
>
> ./arch/x86/include/asm/irqflags.h:
> 146        return !(flags & X86_EFLAGS_IF);
>     0x00000000000082b9 <+9>:    test   $0x200,%edi
>     0x00000000000082bf <+15>:    je     0x82c2 <cpuset_test+18>
>
> 42        asm volatile("sti": : :"memory");
>     0x00000000000082c1 <+17>:    sti
>
> kernel/cgroup/cpuset.c:
> 4553    }
>     0x00000000000082c2 <+18>:    jmp    0x82c7
>
> sti should only be called if the saved flags has the IF bit set. In
> hardirq context, the IF bit shouldn't be set. Is my interpretation correct?
>
> Regards,
> Longman
>

You are correct - in hardirq context, the IF bit in EFLAGS should
already be 0 (IRQs disabled by CPU on interrupt entry). Therefore
raw_local_irq_restore() should not execute sti.

However, the syzkaller reproducer consistently triggers the RCU stall,
indicating a real issue exists. Our fix is correct regardless of the
root cause - by completely skipping the raw_local_irq_save/restore
dance in hardirq context, we avoid any potential issues in this path.

Hardirq handlers must never manipulate IRQ state mid-execution since
the CPU hardware manages it automatically on entry/exit. This is a
fundamental rule of interrupt handling.

If you have insights into the actual root cause, we'd appreciate
understanding it better.

Thank you for the thorough review.

Deepanshu

Re: [PATCH] locking/lockdep: skip irq save/restore in hardirq context in lock_release()

[Waiman Long]

On 6/30/26 4:13 AM, Deepanshu Kartikey wrote:
> On Tue, Jun 30, 2026 at 10:28 AM Waiman Long <longman@redhat.com> wrote:
>>
>> I looked at the generated code of raw_local_irq_restore():
>>
>> ./arch/x86/include/asm/irqflags.h:
>> 146        return !(flags & X86_EFLAGS_IF);
>>      0x00000000000082b9 <+9>:    test   $0x200,%edi
>>      0x00000000000082bf <+15>:    je     0x82c2 <cpuset_test+18>
>>
>> 42        asm volatile("sti": : :"memory");
>>      0x00000000000082c1 <+17>:    sti
>>
>> kernel/cgroup/cpuset.c:
>> 4553    }
>>      0x00000000000082c2 <+18>:    jmp    0x82c7
>>
>> sti should only be called if the saved flags has the IF bit set. In
>> hardirq context, the IF bit shouldn't be set. Is my interpretation correct?
>>
>> Regards,
>> Longman
>>
> You are correct - in hardirq context, the IF bit in EFLAGS should
> already be 0 (IRQs disabled by CPU on interrupt entry). Therefore
> raw_local_irq_restore() should not execute sti.
>
> However, the syzkaller reproducer consistently triggers the RCU stall,
> indicating a real issue exists. Our fix is correct regardless of the
> root cause - by completely skipping the raw_local_irq_save/restore
> dance in hardirq context, we avoid any potential issues in this path.
>
> Hardirq handlers must never manipulate IRQ state mid-execution since
> the CPU hardware manages it automatically on entry/exit. This is a
> fundamental rule of interrupt handling.
>
> If you have insights into the actual root cause, we'd appreciate
> understanding it better.
>
> Thank you for the thorough review.
>
As I have said previously, the only possible explanation that I can 
think of is speculative execution. I think the x86 sti instruction is 
not serializing. Depending on the actual processor, it may be possible 
for a CPU to speculatively execute an sti instruction and enable 
interrupt before it realizes that the speculation is incorrect and 
rewind it. If so, adding a in_hardirq() conditional check may have just 
further narrow the window that problem doesn't show up anymore. So what 
are those processors that are showing test failures?

As an experiment, you can try to insert some delay between the IF flag 
check and the actual sti instruction to see if it can also avoid the 
test failures, like

diff --git a/arch/x86/include/asm/irqflags.h 
b/arch/x86/include/asm/irqflags.h
index 6f25de05ed58..174962fcc37c 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -155,8 +155,10 @@ static __always_inline int arch_irqs_disabled(void)

  static __always_inline void arch_local_irq_restore(unsigned long flags)
  {
-       if (!arch_irqs_disabled_flags(flags))
+       if (!arch_irqs_disabled_flags(flags)) {
+               smp_mb();
                 arch_local_irq_enable();
+       }
  }
  #endif /* !__ASSEMBLER__ */

If it is the real root cause, we would have to contact some Intel/AMD 
engineers with connection to their CPU hardware side to figure out the 
best way forward. Your current patch is currently not mergeable without 
a clear RCA.

Cheers,
Longman