linux-kernel - Re: [patch 2/2] x86,apic: Use logical OR in noop-write operation

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

Date:	Wed, 9 Dec 2009 18:50:02 +0300
From:	Cyrill Gorcunov <gorcunov@...il.com>
To:	tglx@...utronix.de, mingo@...e.hu, hpa@...or.com,
	macro@...ux-mips.org, yinghai@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch 2/2] x86,apic: Use logical OR in noop-write operation

On Tue, Dec 08, 2009 at 11:48:38PM +0300, Cyrill Gorcunov wrote:
> On Tue, Dec 08, 2009 at 06:53:18PM +0300, Cyrill Gorcunov wrote:
> > For apic noop'ified we have to use logical OR statement,
> > otherwise any write on systems shipped with 82489DX
> > (where apic presence bit can't be retrieved via cpuid)
> > will not trigger the warning which is not desired.
> >
> ...
> 
> Ingo please dont apply this patch. Perhaps we may warn unconditionally
> (as Peter marked) which would be more clear approach indeed. Need more
> time to check all code flows.
> 
> Sorry for inconvenience.
> 
> 	-- Cyrill

Here is what done at moment. I've grepped x86 arch for apic_write and
except thermal monitoring (the patch is already sent) all other callers
do check if apic is active (either via cpu_has_apic or disable_apic).
So I think we may safely use unconditional warning if apic_write with
disabled apic is called.

Please take a look -- I would be glad to hear any comments/complains.

There is unclear moment for me with "SGI Visual Workstation" which
has ack_cobalt_irq and calls for apic_write which could trigger this
warning.

	-- Cyrill
---
x86,apic: Warn on noop_apic_write unconditionally

In apic noop'ified we should never call for write operation.
Otherwise it's a caller bug (we should be WARNed about).

Also add a comment on noop_apic_read WARN conditions.

CC: H. Peter Anvin <hpa@...or.com>
CC: Thomas Gleixner <tglx@...utronix.de>
Cc: Yinghai Lu <yinghai@...nel.org>
Cc: Maciej W. Rozycki <macro@...ux-mips.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@...nvz.org>
---

 arch/x86/kernel/apic/apic_noop.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -119,15 +119,28 @@ int noop_apicid_to_node(int logical_apic
 	return 0;
 }
 
+/*
+ * note that we allow this routine to be called
+ * under the following conditions:
+ *  - apic was explicitly disabled via boot option
+ *  - on old 486 machines (which has no apic presence bit
+ *    retrieved via cpuid)
+ * this is done only in a sake of callers code simplicity
+ */
 static u32 noop_apic_read(u32 reg)
 {
-	WARN_ON_ONCE((cpu_has_apic && !disable_apic));
+	WARN_ON_ONCE(cpu_has_apic && !disable_apic);
 	return 0;
 }
 
 static void noop_apic_write(u32 reg, u32 v)
 {
-	WARN_ON_ONCE(cpu_has_apic && !disable_apic);
+	/*
+	 * If someone is trying to write apic
+	 * register when it is NOOP'ified
+	 * this is a bug on caller side
+	 */
+	WARN_ON_ONCE(1);
 }
 
 struct apic apic_noop = {
--
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/