linux-kernel - [PATCH] x86: do not allow to optimize flag_is_changeable

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:	Tue, 30 Sep 2008 23:17:51 +0200
From:	Krzysztof Helt <krzysztof.h1@...pl>
To:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Yinghai Lu <yhlu.kernel@...il.com>,
	Jeremy Fitzhardinge <jeremy@...p.org>
Subject: [PATCH] x86: do not allow to optimize flag_is_changeable_p() (rev.
 2)

From: Krzysztof Helt <krzysztof.h1@...pl>

The flag_is_changeable_p() is used by
has_cpuid_p() which can return different results
in the code sequence below:

 if (!have_cpuid_p())
      identify_cpu_without_cpuid(c);

  /* cyrix could have cpuid enabled via c_identify()*/
  if (!have_cpuid_p())
      return;

Otherwise, the gcc 3.4.6 optimizes these two calls
into one which make the code not working correctly.

Cyrix cpus have the CPUID instruction enabled before
the second call to the have_cpuid_p() but
it is not detected due to the gcc optimization.
Thus the ARR registers (mtrr like) are not detected
on such a cpu.

Signed-off-by: Krzysztof Helt <krzysztof.h1@...pl>
---

The second revision of this patch adds comment
after request from Jeremy Fitzhardinge.

I have tested the 6x86MX cpu with the CPUID
disabled. I have used linux-next tree (20080919)
and Yinghai Lu's patch:

x86: identify_cpu_without_cpuid v2

http://marc.info/?l=linux-kernel&m=122138380004347&w=2

The patch below is required to make the patch
above working correctly.

diff -urp linux-ref/arch/x86/kernel/cpu/common.c linux-mm/arch/x86/kernel/cpu/common.c
--- linux-ref/arch/x86/kernel/cpu/common.c	2008-09-30 23:07:08.000000000 +0200
+++ linux-mm/arch/x86/kernel/cpu/common.c	2008-09-30 23:12:19.098190815 +0200
@@ -124,18 +124,25 @@ static inline int flag_is_changeable_p(u
 {
 	u32 f1, f2;
 
-	asm("pushfl\n\t"
-	    "pushfl\n\t"
-	    "popl %0\n\t"
-	    "movl %0,%1\n\t"
-	    "xorl %2,%0\n\t"
-	    "pushl %0\n\t"
-	    "popfl\n\t"
-	    "pushfl\n\t"
-	    "popl %0\n\t"
-	    "popfl\n\t"
-	    : "=&r" (f1), "=&r" (f2)
-	    : "ir" (flag));
+	/*
+	 * Cyrix and IDT cpus allow disabling of CPUID
+	 * so the code below may return different results
+	 * when it is executed before and after enabling
+	 * the CPUID. Add "volatile" to not allow gcc to
+	 * optimize the subsequent calls to this function.
+	 */
+	asm volatile ("pushfl\n\t"
+		      "pushfl\n\t"
+		      "popl %0\n\t"
+		      "movl %0,%1\n\t"
+		      "xorl %2,%0\n\t"
+		      "pushl %0\n\t"
+		      "popfl\n\t"
+		      "pushfl\n\t"
+		      "popl %0\n\t"
+		      "popfl\n\t"
+		      : "=&r" (f1), "=&r" (f2)
+		      : "ir" (flag));
 
 	return ((f1^f2) & flag) != 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/