[PATCH] alarmtimer: Fix use-after-free of timerqueue node in alarmtimer_suspend()

[PATCH] alarmtimer: Fix use-after-free of timerqueue node in alarmtimer_suspend()

[Zhan Xusheng]

In alarmtimer_suspend(), the timerqueue_node pointer 'next' is obtained
under base->lock via scoped_guard(), but its members (next->expires)
are accessed after the lock has been released when the scoped_guard
goes out of scope.

Between the lock release and the dereference, a concurrent timer
cancellation on another CPU could remove and free the timerqueue node,
leading to a use-after-free.

Fix this by copying next->expires into a local ktime_t variable while
still holding the lock.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/time/alarmtimer.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index b64db405ba5c..6e173d70d825 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -234,19 +234,23 @@ static int alarmtimer_suspend(struct device *dev)
 	if (!rtc)
 		return 0;
 
-	/* Find the soonest timer to expire*/
+	/* Find the soonest timer to expire */
 	for (i = 0; i < ALARM_NUMTYPE; i++) {
 		struct alarm_base *base = &alarm_bases[i];
 		struct timerqueue_node *next;
+		ktime_t next_expires;
 		ktime_t delta;
 
-		scoped_guard(spinlock_irqsave, &base->lock)
+		scoped_guard(spinlock_irqsave, &base->lock) {
 			next = timerqueue_getnext(&base->timerqueue);
+			if (next)
+				next_expires = next->expires;
+		}
 		if (!next)
 			continue;
-		delta = ktime_sub(next->expires, base->get_ktime());
+		delta = ktime_sub(next_expires, base->get_ktime());
 		if (!min || (delta < min)) {
-			expires = next->expires;
+			expires = next_expires;
 			min = delta;
 			type = i;
 		}
-- 
2.43.0

Re: [PATCH] alarmtimer: Fix use-after-free of timerqueue node in alarmtimer_suspend()

[Thomas Gleixner]

On Mon, Mar 30 2026 at 14:15, Zhan Xusheng wrote:
> In alarmtimer_suspend(), the timerqueue_node pointer 'next' is obtained
> under base->lock via scoped_guard(), but its members (next->expires)
> are accessed after the lock has been released when the scoped_guard
> goes out of scope.
>
> Between the lock release and the dereference, a concurrent timer
> cancellation on another CPU could remove and free the timerqueue node,
> leading to a use-after-free.

That's correct in theory, but it's a made up scenario as this runs
during suspend after all processes which could remove and free it have
been frozen already.

That said, I have no objections against the patch per se, but I'm not
accepting the unrealistic argument backing it up.

Using AI is fine, but taking the output at face value is not.

Thanks,

        tglx