[PATCH] alarmtimer: Preserve rearmed alarms during callback expiry

[PATCH] alarmtimer: Preserve rearmed alarms during callback expiry

[Avi Weiss]

An alarm can be rearmed while its previous hrtimer callback is still
running. alarmtimer_fired() then dequeues the newly armed alarm, so
alarmtimer_suspend() no longer sees the wakeup even though the hrtimer
remains queued.

Only dequeue the alarm if the hrtimer is no longer queued.

Fixes: dae373be9fec ("alarmtimer: Use hrtimer per-alarm instead of per-base")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
 kernel/time/alarmtimer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index ea5be5870e515..bf77af5e32c22 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -189,7 +189,8 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer)
 	struct alarm_base *base = &alarm_bases[alarm->type];
 
 	scoped_guard(spinlock_irqsave, &base->lock)
-		alarmtimer_dequeue(base, alarm);
+		if (!hrtimer_is_queued(timer))
+			alarmtimer_dequeue(base, alarm);
 
 	if (alarm->function)
 		alarm->function(alarm, base->get_ktime());

base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a
-- 
2.43.0

Re: [PATCH] alarmtimer: Preserve rearmed alarms during callback expiry

[Thomas Gleixner]

On Sun, Nov 01 2026 at 18:39, Avi Weiss wrote:
> An alarm can be rearmed while its previous hrtimer callback is still
> running. alarmtimer_fired() then dequeues the newly armed alarm, so
> alarmtimer_suspend() no longer sees the wakeup even though the hrtimer
> remains queued.

This fails to explain wich alarm timer user is affected. POSIX-timers
and timerfd timers are not as they cancel the timer before re-arming it.
Which leaves the other two users of alarm_start_timer() which I did not
look at.

But if that actually happens then alarm_start_timer() is subtly broken
because of this sequence:

        alarm->node.expires = expires;
        alarmtime_enqueue(base, alarm);

Setting alarm->node.expires on a queued timer temporarily "corrupts" the
rbtree. That's "harmless" because it's serialized via base->lock and
timerqueue_del() does not care as of today, but that's all but robust
and future proof.

So this needs to be fixed as well and all of this needs to be
documented.
  
>  	scoped_guard(spinlock_irqsave, &base->lock)
> -		alarmtimer_dequeue(base, alarm);
> +		if (!hrtimer_is_queued(timer))
> +			alarmtimer_dequeue(base, alarm);

Reading this without a comment just makes me scratch my head and you
will scratch yours too three month down the road.

Thanks,

        tglx