linux-kernel - [RFC][PATCH 1/2] time: alarmtimer: Fix erroneous case of using 0 as an "invalid" initialization value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230211064527.3481754-1-jstultz@google.com>
Date:   Sat, 11 Feb 2023 06:45:26 +0000
From:   John Stultz <jstultz@...gle.com>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     John Stultz <jstultz@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>, Arnd Bergmann <arnd@...db.de>,
        Michael <michael@...isi.de>,
        Michael Trimarchi <michael@...rulasolutions.com>,
        kernel-team@...roid.com
Subject: [RFC][PATCH 1/2] time: alarmtimer: Fix erroneous case of using 0 as
 an "invalid" initialization value

Michael reported seeing an error where alarmtimers would
occasionally not wake the system up.

It was found that in alarmtimer_suspend() it was exiting via
the:
    if (min == 0)
        return 0;
check. This logic was from one of the early versions of the
original alarmtimer patch, where we initialized min to 0, and
then this check would exit early if we found no timers to expire
(leaving min still at 0).

However, its possible for an alarmtimer to expire as we are
checking it, leaving the calculated delta to be zero, and thus
setting min to zero.

This is the result of my using 0 as an invalid time value which
is clearly erroneous. Instead KTIME_MAX should have been used.

This patch, split out from a change originally suggested by
Thomas Gleixner, changes the logic to instead use KTIME_MAX.

Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Stephen Boyd <sboyd@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>
Cc: Michael <michael@...isi.de>
Cc: Michael Trimarchi <michael@...rulasolutions.com>
Cc: kernel-team@...roid.com
Reported-by: Michael <michael@...isi.de>
Reported-by: Michael Trimarchi <michael@...rulasolutions.com>
Fixes: ff3ead96d17f ("timers: Introduce in-kernel alarm-timer interface")
Originally-by: Thomas Gleixner <tglx@...utronix.de>
Link: https://lore.kernel.org/lkml/alpine.DEB.2.21.1909021247250.3955@nanos.tec.linutronix.de/
[jstultz: Forward ported to 6.2-rc, and split out just the
          KTIME_MAX change]
Signed-off-by: John Stultz <jstultz@...gle.com>
---
 kernel/time/alarmtimer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 5897828b9d7e..f7b2128f64e2 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -251,7 +251,7 @@ static int alarmtimer_suspend(struct device *dev)
 	min = freezer_delta;
 	expires = freezer_expires;
 	type = freezer_alarmtype;
-	freezer_delta = 0;
+	freezer_delta = KTIME_MAX;
 	spin_unlock_irqrestore(&freezer_delta_lock, flags);
 
 	rtc = alarmtimer_get_rtcdev();
@@ -271,13 +271,14 @@ static int alarmtimer_suspend(struct device *dev)
 		if (!next)
 			continue;
 		delta = ktime_sub(next->expires, base->get_ktime());
-		if (!min || (delta < min)) {
+		if (delta < min) {
 			expires = next->expires;
 			min = delta;
 			type = i;
 		}
 	}
-	if (min == 0)
+	/* No timers to expire */
+	if (min == KTIME_MAX)
 		return 0;
 
 	if (ktime_to_ns(min) < 2 * NSEC_PER_SEC) {
@@ -503,7 +504,7 @@ static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 	delta = ktime_sub(absexp, base->get_ktime());
 
 	spin_lock_irqsave(&freezer_delta_lock, flags);
-	if (!freezer_delta || (delta < freezer_delta)) {
+	if (delta < freezer_delta) {
 		freezer_delta = delta;
 		freezer_expires = absexp;
 		freezer_alarmtype = type;
-- 
2.39.1.581.gbfd45094c4-goog