linux-kernel - [PATCH] x86_64: use HPET in 64-bit mode to avoid a harware 32-bit wraparound bug

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:   Fri, 8 Dec 2017 18:17:00 +0100
From:   Jiri Bohac <jbohac@...e.cz>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     Borislav Petkov <bp@...e.de>, linux-kernel@...r.kernel.org
Subject: [PATCH] x86_64: use HPET in 64-bit mode to avoid a harware 32-bit
 wraparound bug

The HPET clockevent configures the HPET Timer 0 in 32-bit mode. With a
wrap-around time of >4 minutes, this is OK for the clockevent purposes.
Unfortunately, some chipsets contain a documented bug, where this setting
also makes the HPET Main Counter wrap to 32 bits. HPET can be mmapped to
userspace, which makes this bug visible to applications.

The original HPET specification 1.0 from 2004 does not mention any
side-effects of setting TN_32MODE_CNF on the individual timers. But e.g.
the ICH9 documentation documents this bug:

    NOTE: When this bit is set to ‘1’, the hardware counter will
    do a 32-bit operation on comparator match and rollovers, thus
    the upper 32-bit of the Timer 0 Comparator Value register is
    ignored. The upper 32-bit of the main counter is not involved
    in any rollover from lower 32-bit of the main counter and
    becomes all zeros.

(see http://www.intel.com/assets/pdf/datasheet/316972.pdf, page
819, section 21.1.5, Bit 8). 
I've seen this behaviour on many other Intel chipsets as well.

This patch changes the HPET clockevent code to use the Timer 0 in 64-bit
mode (if compiled for x86_64; doing this for 32-bit kernels would bring
a performance penalty as a single read would require three 32-bit HPET
accesses).

Signed-off-by: Jiri Bohac <jbohac@...e.cz>

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 8ce4212e2b8d..1a500220d0dc 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -69,6 +69,31 @@ static inline void hpet_writel(unsigned int d, unsigned int a)
 	writel(d, hpet_virt_address + a);
 }
 
+
+static inline unsigned long hpet_read_long(unsigned int a)
+{
+#ifdef CONFIG_64BIT
+	return readq(hpet_virt_address + a);
+#else
+	return readl(hpet_virt_address + a);
+#endif
+}
+
+static inline void hpet_write_long(unsigned long d, unsigned int a)
+{
+#ifdef CONFIG_64BIT
+	writeq(d, hpet_virt_address + a);
+#else
+	writel(d, hpet_virt_address + a);
+#endif
+}
+
+#ifdef CONFIG_64BIT
+#define HPET_TN_WIDTH_FLAG 0
+#else
+#define HPET_TN_WIDTH_FLAG HPET_TN_32BIT
+#endif
+
 #ifdef CONFIG_X86_64
 #include <asm/pgtable.h>
 #endif
@@ -294,19 +319,20 @@ static void hpet_legacy_clockevent_register(void)
 
 static int hpet_set_periodic(struct clock_event_device *evt, int timer)
 {
-	unsigned int cfg, cmp, now;
+	unsigned int cfg;
+	unsigned long cmp, now;
 	uint64_t delta;
 
 	hpet_stop_counter();
 	delta = ((uint64_t)(NSEC_PER_SEC / HZ)) * evt->mult;
 	delta >>= evt->shift;
-	now = hpet_readl(HPET_COUNTER);
-	cmp = now + (unsigned int)delta;
+	now = hpet_read_long(HPET_COUNTER);
+	cmp = now + (unsigned long)delta;
 	cfg = hpet_readl(HPET_Tn_CFG(timer));
 	cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
-	       HPET_TN_32BIT;
+	       HPET_TN_WIDTH_FLAG;
 	hpet_writel(cfg, HPET_Tn_CFG(timer));
-	hpet_writel(cmp, HPET_Tn_CMP(timer));
+	hpet_write_long(cmp, HPET_Tn_CMP(timer));
 	udelay(1);
 	/*
 	 * HPET on AMD 81xx needs a second write (with HPET_TN_SETVAL
@@ -315,7 +341,7 @@ static int hpet_set_periodic(struct clock_event_device *evt, int timer)
 	 * (See AMD-8111 HyperTransport I/O Hub Data Sheet,
 	 * Publication # 24674)
 	 */
-	hpet_writel((unsigned int)delta, HPET_Tn_CMP(timer));
+	hpet_write_long((unsigned long)delta, HPET_Tn_CMP(timer));
 	hpet_start_counter();
 	hpet_print_config();
 
@@ -328,7 +354,7 @@ static int hpet_set_oneshot(struct clock_event_device *evt, int timer)
 
 	cfg = hpet_readl(HPET_Tn_CFG(timer));
 	cfg &= ~HPET_TN_PERIODIC;
-	cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
+	cfg |= HPET_TN_ENABLE | HPET_TN_WIDTH_FLAG;
 	hpet_writel(cfg, HPET_Tn_CFG(timer));
 
 	return 0;
@@ -355,12 +381,12 @@ static int hpet_resume(struct clock_event_device *evt)
 static int hpet_next_event(unsigned long delta,
 			   struct clock_event_device *evt, int timer)
 {
-	u32 cnt;
-	s32 res;
+	unsigned long cnt;
+	long res;
 
-	cnt = hpet_readl(HPET_COUNTER);
-	cnt += (u32) delta;
-	hpet_writel(cnt, HPET_Tn_CMP(timer));
+	cnt = hpet_read_long(HPET_COUNTER);
+	cnt += delta;
+	hpet_write_long(cnt, HPET_Tn_CMP(timer));
 
 	/*
 	 * HPETs are a complete disaster. The compare register is
@@ -384,7 +410,7 @@ static int hpet_next_event(unsigned long delta,
 	 * the event. The minimum programming delta for the generic
 	 * clockevents code is set to 1.5 * HPET_MIN_CYCLES.
 	 */
-	res = (s32)(cnt - hpet_readl(HPET_COUNTER));
+	res = (long)(cnt - hpet_read_long(HPET_COUNTER));
 
 	return res < HPET_MIN_CYCLES ? -ETIME : 0;
 }
-- 
Jiri Bohac <jbohac@...e.cz>
SUSE Labs, Prague, Czechia