linux-kernel - [PATCH] rtc-mc146818-lib: wait longer for UIP to clear

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]

Date:   Sat, 12 Feb 2022 23:04:54 +0100
From:   Mateusz Jończyk <mat.jonczyk@...pl>
To:     linux-rtc@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Mateusz Jończyk <mat.jonczyk@...pl>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: [PATCH] rtc-mc146818-lib: wait longer for UIP to clear

Before reading date / time from the CMOS RTC, we wait for the UIP
(Update in progress) bit to clear --- so that the values are correct and
consistent. To avoid a hang, there is a time limit after which we give
up waiting.

Increase the time limit from 10 to 20ms in case there are RTCs out there
that are much slower than expected.

Note: This may cause problems with hpet_rtc_interrupt() if the CMOS RTC
breaks down while the system is running and RTC update interrupt / RTC
alarm interrupt happens to be enabled (which should be rare).
hpet_rtc_interrupt() is executed usually 64 times per second and after
this patch it may take up to 20ms to complete (always hitting RTC read
timeout) - which may constantly occupy one CPU. This looks very
unlikely, though.

Also fix "then" -> "than" in a comment.

Signed-off-by: Mateusz Jończyk <mat.jonczyk@...pl>
Cc: Alessandro Zummo <a.zummo@...ertech.it>
Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>
Fixes: ec5895c0f2d8 ("rtc: mc146818-lib: extract mc146818_avoid_UIP")
---

This comment from Mr Alexandre Belloni got me thinking and is why I am
submitting this patch:
> We'll probably get some breakage later on because many RTCs using this
> driver are not adhering to the spec.
(See: https://lore.kernel.org/linux-rtc/277177e7-46a0-522c-297c-ad3ee0c15793@o2.pl/T/ )

Googling for dmesg messages that indicate problems with reading from RTC
(such as "unable to read current time" - using quotation marks to force
exact phrase search) produces no results apart from kernel code and
patches. Any problems would happen rarely on affected systems, though.

 drivers/rtc/rtc-mc146818-lib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
index ae9f131b43c0..29ceec9875f4 100644
--- a/drivers/rtc/rtc-mc146818-lib.c
+++ b/drivers/rtc/rtc-mc146818-lib.c
@@ -21,7 +21,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
 	unsigned long flags;
 	unsigned char seconds;

-	for (i = 0; i < 10; i++) {
+	for (i = 0; i < 20; i++) {
 		spin_lock_irqsave(&rtc_lock, flags);

 		/*
@@ -79,8 +79,8 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
 EXPORT_SYMBOL_GPL(mc146818_avoid_UIP);

 /*
- * If the UIP (Update-in-progress) bit of the RTC is set for more then
- * 10ms, the RTC is apparently broken or not present.
+ * If the UIP (Update-in-progress) bit of the RTC is set for more than
+ * 20ms, the RTC is apparently broken or not present.
  */
 bool mc146818_does_rtc_work(void)
 {

base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
-- 
2.25.1