linux-kernel - [PATCH] RFC: 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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220107153454.391708-1-mat.jonczyk@o2.pl>
Date:   Fri,  7 Jan 2022 16:34: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] RFC: 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.

This is to be applied on top of rtc-next.

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 - which may constantly
occupy one CPU. This looks very unlikely, though.

So, I don't know whether implementing this change is worth it and even if
20ms is enough. So I'm asking for opinions.

This comment from Mr Alexandre Belloni got me thinking and is why I
consider 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/ )

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")
---
 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 f62e658cbe23..55e7d2a7578a 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)
 {
-- 
2.25.1