[PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[Tiezhu Yang]

When testing the BPF selftest "sudo ./test_progs -t timer_lockup", there
is a kernel lockup and panic on LoongArch:

  watchdog: BUG: soft lockup - CPU#1 stuck for 8s! [test_progs:39601]
  Kernel panic - not syncing: softlockup: hung tasks
  ...
  Call Trace:
  ...
  [<9000000000c40e98>] panic+0x44/0x48
  [<9000000000e8b000>] watchdog_timer_fn+0x500/0x520
  [<9000000000dfd9a4>] __hrtimer_run_queues+0xc4/0x530
  [<9000000000dffe90>] hrtimer_interrupt+0x140/0x320
  ...
  [<9000000002ad9e4c>] _raw_spin_unlock_irqrestore+0x8c/0xc0
  [<9000000000dfe9e0>] hrtimer_try_to_cancel.part.0+0x70/0x350
  [<9000000000dfed58>] hrtimer_cancel+0x38/0x80
  [<9000000000f6d944>] bpf_timer_cancel+0x94/0x1e0
  [<ffff80000200fad0>] bpf_prog_108ab87b32f22e44_timer_cb1+0xb0/0xfc
  [<9000000000f6b838>] bpf_timer_cb+0x98/0x170
  [<9000000000dfdaac>] __hrtimer_run_queues+0x1cc/0x530
  [<9000000000dfde94>] hrtimer_run_softirq+0x84/0xd0
  ...
  [<900000000271d9e0>] bpf_test_run+0x1c0/0x5c0
  [<900000000271f548>] bpf_prog_test_run_skb+0x6e8/0xe20
  [<9000000000f39940>] __sys_bpf+0x1690/0x2c50
  [<9000000000f3af28>] sys_bpf+0x28/0x40
  [<9000000002ac2d68>] do_syscall+0x108/0x5e0
  [<9000000000c6a850>] handle_syscall+0xd0/0x170

In bpf_timer_cancel() of kernel/bpf/helpers.c, it explicitly notes that
"Need full barrier after relaxed atomic_inc" to ensure global visibility
of the cancelling state before performing the lockless dependency checks.

However, on weakly-ordered architectures such as LoongArch, the current
combination of a relaxed atomic_inc() followed by smp_mb__after_atomic()
fails to guarantee the physical store-load ordering because the latter
currently expands to an empty compiler barrier rather than a hardware
data barrier on LoongArch.

This allows a subsequent read to bypass the prior write due to store-load
reordering, enabling concurrent CPUs to simultaneously bypass the software
deadlock detection, enter hrtimer_cancel(), and then trigger a severe ABBA
deadlock in the hrtimer core.

Instead of relying on arch-specific macro implementations which may vary
in strictness, fix this issue directly in the BPF core helper by replacing
atomic_inc() and smp_mb__after_atomic() with a single atomic_fetch_inc()
to provide full ordering natively.

This ensures that the BPF core satisfies its strict store-load ordering
requirement in a self-contained manner to eliminate the deadlock under
weak memory models, and also hardens the defensive programming in the
BPF core, making the lockless deadlock detection logic immune to any
platform-level barrier interpretation variations.

With this patch, the BPF timer_lockup selftest was stressed for 10000
consecutive loops on a physical LoongArch machine without encountering
any further lockups or warnings on LoongArch:

  for i in {1..10000}; do sudo ./test_progs -t timer_lockup; done

Reported-by: Vincent Li <vincent.mc.li@gmail.com>
Closes: https://lore.kernel.org/loongarch/CAK3+h2xOSEZUHhou7N2cRL-aGrZCNSm45g+P7thObMe+fpgYCA@mail.gmail.com/
Fixes: d4523831f07a ("bpf: Fail bpf_timer_cancel when callback is being cancelled")
Cc: stable@vger.kernel.org
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
Resend due to
"Can not connect to recipient's server because of unstable network or firewall filter."

 kernel/bpf/helpers.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b3cc5c8fc875..7d89dc143883 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1591,9 +1591,7 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, async)
 	 */
 	if (!cur_t)
 		goto drop;
-	atomic_inc(&t->cancelling);
-	/* Need full barrier after relaxed atomic_inc */
-	smp_mb__after_atomic();
+	atomic_fetch_inc(&t->cancelling);
 	inc = true;
 	if (atomic_read(&cur_t->cancelling)) {
 		/* We're cancelling timer t, while some other timer callback is
-- 
2.42.0

Re: [PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[Kumar Kartikeya Dwivedi]

On Wed Sep 2, 2026 at 2:51 AM CEST, Tiezhu Yang wrote:
> When testing the BPF selftest "sudo ./test_progs -t timer_lockup", there
> is a kernel lockup and panic on LoongArch:
>
>   watchdog: BUG: soft lockup - CPU#1 stuck for 8s! [test_progs:39601]
>   Kernel panic - not syncing: softlockup: hung tasks
>   ...
>   Call Trace:
>   ...
>   [<9000000000c40e98>] panic+0x44/0x48
>   [<9000000000e8b000>] watchdog_timer_fn+0x500/0x520
>   [<9000000000dfd9a4>] __hrtimer_run_queues+0xc4/0x530
>   [<9000000000dffe90>] hrtimer_interrupt+0x140/0x320
>   ...
>   [<9000000002ad9e4c>] _raw_spin_unlock_irqrestore+0x8c/0xc0
>   [<9000000000dfe9e0>] hrtimer_try_to_cancel.part.0+0x70/0x350
>   [<9000000000dfed58>] hrtimer_cancel+0x38/0x80
>   [<9000000000f6d944>] bpf_timer_cancel+0x94/0x1e0
>   [<ffff80000200fad0>] bpf_prog_108ab87b32f22e44_timer_cb1+0xb0/0xfc
>   [<9000000000f6b838>] bpf_timer_cb+0x98/0x170
>   [<9000000000dfdaac>] __hrtimer_run_queues+0x1cc/0x530
>   [<9000000000dfde94>] hrtimer_run_softirq+0x84/0xd0
>   ...
>   [<900000000271d9e0>] bpf_test_run+0x1c0/0x5c0
>   [<900000000271f548>] bpf_prog_test_run_skb+0x6e8/0xe20
>   [<9000000000f39940>] __sys_bpf+0x1690/0x2c50
>   [<9000000000f3af28>] sys_bpf+0x28/0x40
>   [<9000000002ac2d68>] do_syscall+0x108/0x5e0
>   [<9000000000c6a850>] handle_syscall+0xd0/0x170
>
> In bpf_timer_cancel() of kernel/bpf/helpers.c, it explicitly notes that
> "Need full barrier after relaxed atomic_inc" to ensure global visibility
> of the cancelling state before performing the lockless dependency checks.
>
> However, on weakly-ordered architectures such as LoongArch, the current
> combination of a relaxed atomic_inc() followed by smp_mb__after_atomic()
> fails to guarantee the physical store-load ordering because the latter
> currently expands to an empty compiler barrier rather than a hardware
> data barrier on LoongArch.
>
> This allows a subsequent read to bypass the prior write due to store-load
> reordering, enabling concurrent CPUs to simultaneously bypass the software
> deadlock detection, enter hrtimer_cancel(), and then trigger a severe ABBA
> deadlock in the hrtimer core.
>
> Instead of relying on arch-specific macro implementations which may vary
> in strictness, fix this issue directly in the BPF core helper by replacing
> atomic_inc() and smp_mb__after_atomic() with a single atomic_fetch_inc()
> to provide full ordering natively.

This paragraph is completely bogus. LoongArch's smp_mb__after_atomic() had a
bug. The specification in LKMM for smp_mb__after_atomic() is that it should be a
full barrier, which can be relaxed if the prior atomic operation already
provides the necessary ordering.

As I already said, there is no point in changing or "optimizing" this atomic
inc. Just let it be. You will see no measurable difference for this function.

Just accept that it was a bug, and fix the lowering for your arch. Plenty of
other logic in the kernel uses this primitive, so I think you folks were just
lucky this wasn't hit before by something else.

> [...]

Re: [PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[Tiezhu Yang]

On 2026/9/2 上午9:13, Kumar Kartikeya Dwivedi wrote:
> On Wed Sep 2, 2026 at 2:51 AM CEST, Tiezhu Yang wrote:
>> When testing the BPF selftest "sudo ./test_progs -t timer_lockup", there
>> is a kernel lockup and panic on LoongArch:
>>
>>    watchdog: BUG: soft lockup - CPU#1 stuck for 8s! [test_progs:39601]
>>    Kernel panic - not syncing: softlockup: hung tasks
>>    ...
>>    Call Trace:
>>    ...
>>    [<9000000000c40e98>] panic+0x44/0x48
>>    [<9000000000e8b000>] watchdog_timer_fn+0x500/0x520
>>    [<9000000000dfd9a4>] __hrtimer_run_queues+0xc4/0x530
>>    [<9000000000dffe90>] hrtimer_interrupt+0x140/0x320
>>    ...
>>    [<9000000002ad9e4c>] _raw_spin_unlock_irqrestore+0x8c/0xc0
>>    [<9000000000dfe9e0>] hrtimer_try_to_cancel.part.0+0x70/0x350
>>    [<9000000000dfed58>] hrtimer_cancel+0x38/0x80
>>    [<9000000000f6d944>] bpf_timer_cancel+0x94/0x1e0
>>    [<ffff80000200fad0>] bpf_prog_108ab87b32f22e44_timer_cb1+0xb0/0xfc
>>    [<9000000000f6b838>] bpf_timer_cb+0x98/0x170
>>    [<9000000000dfdaac>] __hrtimer_run_queues+0x1cc/0x530
>>    [<9000000000dfde94>] hrtimer_run_softirq+0x84/0xd0
>>    ...
>>    [<900000000271d9e0>] bpf_test_run+0x1c0/0x5c0
>>    [<900000000271f548>] bpf_prog_test_run_skb+0x6e8/0xe20
>>    [<9000000000f39940>] __sys_bpf+0x1690/0x2c50
>>    [<9000000000f3af28>] sys_bpf+0x28/0x40
>>    [<9000000002ac2d68>] do_syscall+0x108/0x5e0
>>    [<9000000000c6a850>] handle_syscall+0xd0/0x170
>>
>> In bpf_timer_cancel() of kernel/bpf/helpers.c, it explicitly notes that
>> "Need full barrier after relaxed atomic_inc" to ensure global visibility
>> of the cancelling state before performing the lockless dependency checks.
>>
>> However, on weakly-ordered architectures such as LoongArch, the current
>> combination of a relaxed atomic_inc() followed by smp_mb__after_atomic()
>> fails to guarantee the physical store-load ordering because the latter
>> currently expands to an empty compiler barrier rather than a hardware
>> data barrier on LoongArch.
>>
>> This allows a subsequent read to bypass the prior write due to store-load
>> reordering, enabling concurrent CPUs to simultaneously bypass the software
>> deadlock detection, enter hrtimer_cancel(), and then trigger a severe ABBA
>> deadlock in the hrtimer core.
>>
>> Instead of relying on arch-specific macro implementations which may vary
>> in strictness, fix this issue directly in the BPF core helper by replacing
>> atomic_inc() and smp_mb__after_atomic() with a single atomic_fetch_inc()
>> to provide full ordering natively.
> 
> This paragraph is completely bogus. LoongArch's smp_mb__after_atomic() had a
> bug. The specification in LKMM for smp_mb__after_atomic() is that it should be a
> full barrier, which can be relaxed if the prior atomic operation already
> provides the necessary ordering.
> 
> As I already said, there is no point in changing or "optimizing" this atomic
> inc. Just let it be. You will see no measurable difference for this function.
> 
> Just accept that it was a bug, and fix the lowering for your arch. Plenty of
> other logic in the kernel uses this primitive, so I think you folks were just
> lucky this wasn't hit before by something else.

Thanks for your feedback.

I looked into tools/memory-model/Documentation/ordering.txt,
as it explicitly clarifies, atomic_inc() does not guarantee
full ordering on weakly-ordered architectures, therefore
smp_mb__after_atomic() must emit a hardware data barrier to
comply with the LKMM specification, rather than expanding to
a plain compiler barrier.

I also noticed there are hundreds of smp_mb__after_atomic()
call sites across the core kernel (kernel/, drivers/, mm/,
net/, fs/) that currently lack necessary hardware data barriers
on LoongArch, which have potential risks.

So please disregard this bpf patch, I will send a v2 patch to
remove the definition of __smp_mb__{before,after}_atomic() in
arch/loongarch/include/asm/barrier.h, so that these two macros
can automatically fall back to the generic definition in
include/asm-generic/barrier.h:

#ifndef __smp_mb__before_atomic
#define __smp_mb__before_atomic()	__smp_mb()
#endif

#ifndef __smp_mb__after_atomic
#define __smp_mb__after_atomic()	__smp_mb()
#endif

Thanks,
Tiezhu

Re: [PATCH bpf v1 RESEND] bpf: Fix timer_lockup deadlock using atomic_fetch_inc()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4448 bytes --]

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b3cc5c8fc875..7d89dc143883 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1591,9 +1591,7 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, async)
>  	 */
>  	if (!cur_t)
>  		goto drop;
> -	atomic_inc(&t->cancelling);
> -	/* Need full barrier after relaxed atomic_inc */
> -	smp_mb__after_atomic();
> +	atomic_fetch_inc(&t->cancelling);
>  	inc = true;
>  	if (atomic_read(&cur_t->cancelling)) {
>  		/* We're cancelling timer t, while some other timer callback is

The commit message states:

> Instead of relying on arch-specific macro implementations which may vary
> in strictness, fix this issue directly in the BPF core helper by replacing
> atomic_inc() and smp_mb__after_atomic() with a single atomic_fetch_inc()
> to provide full ordering natively.
> ...
> making the lockless deadlock detection logic immune to any
> platform-level barrier interpretation variations.

Is this rationale consistent with the atomic API documentation?
Documentation/atomic_t.txt:208-216 states that atomic_fetch_inc() is
equivalent to:

    smp_mb__before_atomic();
    atomic_fetch_add_relaxed();
    smp_mb__after_atomic();

The generic implementation in include/linux/atomic/atomic-arch-fallback.h
shows that atomic_fetch_inc() expands to __atomic_pre_full_fence() +
relaxed RMW + __atomic_post_full_fence(), where those fences are defined
as smp_mb__before_atomic() and smp_mb__after_atomic() in
include/linux/atomic.h:50-55.

On architectures that only supply the *_relaxed RMW variants, the new line
re-expands to exactly the sequence the patch deleted. The claim that this
makes the code "immune to any platform-level barrier interpretation
variations" and provides "full ordering natively" does not follow from the
API contract.

The patch does fix the reported LoongArch lockup, but only because
arch/loongarch/include/asm/atomic-amo.h emits the fully-ordered fetch
variant as amadd_db.w when CONFIG_CPU_HAS_AMO is set.

The actual contract violation appears to be in the architecture.
Documentation/atomic_t.txt:190-206 defines smp_mb__after_atomic() as a
barrier that augments the ordering inherent to the op, with
smp_mb__after_atomic() ordering all later accesses against the RMW op.
arch/loongarch/include/asm/barrier.h:60-61 defines both as plain
barrier(), while arch/loongarch/include/asm/atomic-amo.h's void-returning
ATOMIC_OP emits am<op>.w with no _db suffix - unordered. That combination
breaks the documented contract for every user, and there are approximately
270 smp_mb__after_atomic() call sites under kernel/, mm/, net/, fs/, lib/
and drivers/ that rely on it.

Fixing only this one BPF helper hides the arch bug rather than fixing it.
Should the fix go to arch/loongarch/include/asm/barrier.h instead, or does
the commit message need to justify why the arch macro is considered
permissibly weak?

The patch also deletes the only in-code record of the ordering requirement:

> -	atomic_inc(&t->cancelling);
> -	/* Need full barrier after relaxed atomic_inc */
> -	smp_mb__after_atomic();
> +	atomic_fetch_inc(&t->cancelling);

Nothing in the resulting code indicates that the value-returning variant
was chosen purely for its full-ordering semantics, so the line reads as a
plain refcount bump with a discarded return value. The store-load pairing
is the whole mechanism: t->cancelling must be globally visible before
cur_t->cancelling is read, otherwise two callbacks cancelling each other
both read 0 and both enter the unbounded wait in hrtimer_cancel()
(kernel/time/hrtimer.c hrtimer_cancel()).

A discarded-return-value RMW is an obvious target for a later unused
result cleanup back to atomic_inc(), which would silently reintroduce
exactly the lockup this commit fixes. Discarding a fetch op's result is
rare in core code - the only other instances are plain counters
(drivers/gpu/drm/drm_file.c:383, drivers/accel/drm_accel.c:129), none of
them ordering-motivated.

Would it make sense to keep a one-line comment on the new statement
explaining that atomic_fetch_inc() is used (return value ignored) for its
full ordering against the atomic_read() of cur_t->cancelling that follows?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33578169526