linux-kernel - Re: [PATCH] rtc: cros-ec: Limit RTC alarm range if needed

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221108165637.GA801591@roeck-us.net>
Date:   Tue, 8 Nov 2022 08:59:18 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Benson Leung <bleung@...omium.org>, linux-rtc@...r.kernel.org,
        chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH] rtc: cros-ec: Limit RTC alarm range if needed

Hi,

On Mon, Nov 07, 2022 at 11:52:50PM +0100, Alexandre Belloni wrote:
[ ... ]
> 
> I'll take the patch as-is so you can backport it and have a solution.
> I'll also work on the alarm range and I'll let you get the series once
> this is ready so you can test.
> 

Excellent, thanks a lot. I also started looking into a poor-man's solution
of range support. I attached what I currently have below for your
reference. It isn't much, but it let me test follow-up changes in the
cros-ec rtc driver. Unfortunately I was not able to find a means to
implement something like "go back to sleep fast" in the alarm timer code.

In this context: Is there a standardized set of error codes for RTC
drivers ? I see -EINVAL, -ETIME, -EDOM, -ERANGE, but those are not
consistently used. I assumed -ETIME for "time expired" and -ERANGE
for "time too far in the future" below, but that was just a wild guess.

Thanks,
Guenter

---
commit 7918f162f947424ec0ad7a318c45febeaea51d2e
Author:     Guenter Roeck <linux@...ck-us.net>
AuthorDate: Wed Nov 2 19:35:09 2022 -0700
Commit:     Guenter Roeck <linux@...ck-us.net>
CommitDate: Fri Nov 4 09:54:06 2022 -0700

    rtc: Add support for limited alarm timer offsets
    
    Some alarm timers are based on time offsets, not on absolute times.
    In some situations, the amount of time that can be scheduled in the
    future is limited. This may result in a refusal to suspend the system,
    causing substantial battery drain.
    
    Some RTC alarm drivers remedy the situation by setting the alarm time
    to the maximum supported time if a request for an out-of-range timeout
    is made. This is not really desirable since it may result in unexpected
    early wakeups.
    
    To reduce the impact of this problem, let RTC drivers report the maximum
    supported alarm timer offset. The code setting alarm timers can then
    decide if it wants to reject setting alarm timers to a larger value, if it
    wants to implement recurring alarms until the actually requested alarm
    time is met, or if it wants to accept the limited alarm time.
    
    Signed-off-by: Guenter Roeck <linux@...ck-us.net>

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 9edd662c69ac..05ec9afbb6ba 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -426,6 +426,10 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
 
 	if (scheduled <= now)
 		return -ETIME;
+
+	if (rtc->range_max_offset && scheduled - now > rtc->range_max_offset)
+		return -ERANGE;
+
 	/*
 	 * XXX - We just checked to make sure the alarm time is not
 	 * in the past, but there is still a race window where if
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 1fd9c6a21ebe..b6d000ab1e5e 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -146,6 +146,7 @@ struct rtc_device {
 
 	time64_t range_min;
 	timeu64_t range_max;
+	timeu64_t range_max_offset;
 	time64_t start_secs;
 	time64_t offset_secs;
 	bool set_start_time;
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 5897828b9d7e..af8e0a9e0d63 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -291,6 +291,19 @@ static int alarmtimer_suspend(struct device *dev)
 	rtc_timer_cancel(rtc, &rtctimer);
 	rtc_read_time(rtc, &tm);
 	now = rtc_tm_to_ktime(tm);
+
+	/*
+	 * If the RTC alarm timer only supports a limited time offset, set
+	 * the alarm time to the maximum supported value.
+	 * The system will wake up earlier than necessary and is expected
+	 * to go back to sleep if it has nothing to do.
+	 * It would be desirable to handle such early wakeups without fully
+	 * waking up the system, but it is unknown if this is even possible.
+	 */
+	if (rtc->range_max_offset &&
+	    rtc->range_max_offset * NSEC_PER_SEC > ktime_to_ns(min))
+		min = ns_to_ktime(rtc->range_max_offset * NSEC_PER_SEC);
+
 	now = ktime_add(now, min);
 
 	/* Set alarm, if in the past reject suspend briefly to handle */