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 for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <5458A9600200007800044AE5@mail.emea.novell.com>
Date:	Tue, 04 Nov 2014 09:24:32 +0000
From:	"Jan Beulich" <JBeulich@...e.com>
To:	<mingo@...e.hu>, <tglx@...utronix.de>, <hpa@...or.com>
Cc:	"Tony Jones" <tonyj@...e.de>, <linux-kernel@...r.kernel.org>
Subject: [PATCH, RFC] x86: also CFI-annotate certain inline asm()s

The main obstacle to having done this long ago was the need to
determine whether annotations are needed in the first place: They need
to be avoided when a frame pointer got set up. Since I can't see a way
to determine this before the compilation phase, this is being achieved
by inspecting the memory address generated by the compiler in an
interposed assembler macro. Of course this isn't really nice code, and
this the main reason I'm posting this as RFC only at this point (with
the hope that maybe someone has an idea of how to achieve the same
thing in a more elegant way).

The reason for this being needed are various inline stack pointer
adjustments. In particular in the context of perf and its use of NMIs,
Tony observed the unwinder to get confused when an interruption
happened in the middle of an inline push/pop pair. While the live
unwinder continues to be of no interest upstream, NMIs being used to
trigger crash dumps (via external button or watchdog) would appear to
suffer from the same problem.

With the irqflags generic code generation issue out of the way (see
https://patchwork.kernel.org/patch/5223561/), the only differences in
generated code are a number pointless extra frame pointer adjustments
the compiler decides it needs.

Several pieces of code were intentionally left untouched:

- use site compiled with -fno-omit-frame-pointer:
  arch/x86/include/asm/switch_to.h:switch_to()

- annotations in helper sections (like .fixup) don't work anyway:
  arch/x86/lib/usercopy_32.c

- call stack reaching there unlikely to be of interest:
  arch/x86/boot/
  arch/x86/kernel/cpu/common.c:flag_is_changeable_p()
  arch/x86/include/asm/apm.h
  arch/x86/include/asm/kexec.h:crash_setup_regs()
  arch/x86/pci/pcbios.c:pcibios_get_irq_routing_table()
  arch/x86/xen/enlighten.c:xen_setup_gdt()
  drivers/input/misc/wistron_btns.c:call_bios()
  drivers/pci/hotplug/cpqphp_nvram.c:access_EV()
  drivers/pnp/pnpbios/
  drivers/watchdog/hpwdt.c:asminline_call

- to be done separately (if desired/needed):
  anything pv-ops related (patching as well as wrappers)
  arch/x86/kernel/kprobes/
  arch/x86/kernel/kvm/
  drivers/char/i8k.c:i8k_smm()
  drivers/char/toshiba.c:tosh_smm()
  drivers/lguest/x86/

Signed-off-by: Jan Beulich <jbeulich@...e.com>
Cc: Tony Jones <tonyj@...e.de>
---
 arch/x86/Makefile                |   22 ++++++++
 arch/x86/include/asm/frame.h     |  103 ++++++++++++++++++++++++++++++++++++++-
 arch/x86/include/asm/irqflags.h  |   18 ++++--
 arch/x86/include/asm/processor.h |   22 +++++---
 arch/x86/kernel/irq_32.c         |   15 ++++-
 drivers/cpufreq/speedstep-smi.c  |   32 ++++++------
 drivers/crypto/padlock-aes.c     |   14 +++--
 7 files changed, 188 insertions(+), 38 deletions(-)

--- 3.18-rc3/arch/x86/Makefile
+++ 3.18-rc3-x86-inline-unwind-info/arch/x86/Makefile
@@ -131,6 +131,28 @@ ifdef CONFIG_X86_X32
 endif
 export CONFIG_X86_X32_ABI
 
+ifeq ($(CONFIG_UNWIND_INFO),y)
+# Does gcc support generating CFI annotations via .cfi_* directives?
+ifeq ($(call cc-option,-fdwarf2-cfi-asm),-fdwarf2-cfi-asm)
+cfi-directives := $(call try-run,\
+		    /bin/echo '\
+			void test(void) { \
+				CFI_DECL; \
+				asm volatile(".cfi_undefined 0"); \
+				asm volatile(CFI_ADJUST_CFA_OFFSET(1) :: CFI_INPUTS()); \
+		 	}' | \
+		    $(CC) -DCONFIG_CC_AS_CFI -include $(srctree)/arch/x86/include/asm/frame.h -c -x c -o "$$TMP" - \
+		    ,y,n)
+ifeq ($(cfi-directives),y)
+KBUILD_CFLAGS += -DCONFIG_CC_AS_CFI
+else
+$(warning Inline function CFI annotations not usable with $(CC))
+endif
+else
+$(warning Inline function CFI annotations not supported by $(CC))
+endif
+endif
+
 # Don't unroll struct assignments with kmemcheck enabled
 ifeq ($(CONFIG_KMEMCHECK),y)
 	KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
--- 3.18-rc3/arch/x86/include/asm/frame.h
+++ 3.18-rc3-x86-inline-unwind-info/arch/x86/include/asm/frame.h
@@ -1,3 +1,8 @@
+#if 0
+.if 0
+#elif !defined(_ASM_X86_FRAME_H)
+#define _ASM_X86_FRAME_H
+
 #ifdef __ASSEMBLY__
 
 #include <asm/asm.h>
@@ -23,4 +28,100 @@
 	.endm
 #endif
 
-#endif  /*  __ASSEMBLY__  */
+#elif defined(CONFIG_CC_AS_CFI)
+
+#include <linux/stringify.h>
+
+asm(".include \"" __FILE__ "\"");
+
+#define CFI_DECL volatile struct {} cfi_var
+/* Helper: Even inside .if 0 blocks gas warns when strings start a line. */
+#define CFI_BLANK
+#define CFI_PREFIX "; maybe_cfi_annotate$ %[cfi_var],%c[cfi_sz],"
+#define CFI_ADJUST_CFA_OFFSET(nr) CFI_PREFIX "adjust_cfa_offset" \
+	CFI_BLANK " (" __stringify(nr) ") * (%c[cfi_sz])"
+#define CFI_DEF_CFA_REGISTER(reg) CFI_PREFIX "def_cfa_register " #reg
+#define CFI_INPUTS(more...) [cfi_var] "m" (cfi_var), \
+			    [cfi_sz] "i" (sizeof(void *)), ## more
+
+#else
+
+#define CFI_DECL struct cfi_dummy
+#define CFI_ADJUST_CFA_OFFSET(nr)
+#define CFI_DEF_CFA_REGISTER(reg)
+#define CFI_INPUTS(more...) more
+
+#endif /* __ASSEMBLY__ / CONFIG_CC_AS_CFI */
+
+#elif 0
+.endif
+.macro maybe_cfi_annotate$ mem:req, sz:req, annot:vararg
+ nest$ = 0
+ state$ = 0
+ skip$ = 0
+ .irpc c,\mem
+  .ifeqs "(", "\c"
+   nest$ = nest$ + 1
+   state$ = 0
+  .endif
+
+  .ifeqs ")", "\c"
+   nest$ = nest$ - 1
+  .elseif (nest$ == 1) && (skip$ == 0)
+   .if state$ == 0
+    .ifeqs "%", "\c"
+     state$ = 1
+    .endif
+   .elseif state$ == 1
+    .if \sz == 4
+     .ifeqs "e", "\c"
+      state$ = 2
+     .endif
+    .endif
+
+    .if \sz == 8
+     .ifeqs "r", "\c"
+      state$ = 2
+     .endif
+    .endif
+
+    .if state$ <> 2
+     .exitm
+    .endif
+   .elseif state$ == 2
+    .ifeqs "s", "\c"
+     state$ = state$ | 0x04
+    .endif
+
+    .ifeqs "b", "\c"
+     state$ = state$ | 0x08
+    .endif
+
+    .if (state$ & 0x0c) == 0
+     .exitm
+    .endif
+   .elseif ((state$ & 3) == 2) && ((state$ & 0x0c) <> 0)
+    .ifeqs "p", "\c"
+     state$ = state$ | 0x10
+    .else
+     .exitm
+    .endif
+   .else
+    .ifeqs ",", "\c"
+     skip$ = 1
+    .else
+     .exitm
+    .endif
+   .endif
+  .endif
+ .endr
+
+ .if (nest$ <> 0) && (state$ == 0)
+  .error "unmatched parentheses in '\mem\(')"
+ .elseif (nest$ == 0) && (state$ == 0x16)
+  .cfi_\annot
+ .elseif (nest$ <> 0) || (state$ <> 0x1a)
+  .error "unsupported memory expression '\mem\(')"
+ .endif
+.endm
+#endif
--- 3.18-rc3/arch/x86/include/asm/irqflags.h
+++ 3.18-rc3-x86-inline-unwind-info/arch/x86/include/asm/irqflags.h
@@ -4,6 +4,9 @@
 #include <asm/processor-flags.h>
 
 #ifndef __ASSEMBLY__
+
+#include <asm/frame.h>
+
 /*
  * Interrupt control:
  */
@@ -11,6 +14,7 @@
 static inline unsigned long native_save_fl(void)
 {
 	unsigned long flags;
+	CFI_DECL;
 
 	/*
 	 * "=rm" is safe here, because "pop" adjusts the stack before
@@ -18,9 +22,10 @@ static inline unsigned long native_save_
 	 * documented behavior of the "pop" instruction.
 	 */
 	asm volatile("# __raw_save_flags\n\t"
-		     "pushf ; pop %0"
+		     "pushf" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
+		     "pop %0" CFI_ADJUST_CFA_OFFSET(-1)
 		     : "=rm" (flags)
-		     : /* no input */
+		     : CFI_INPUTS()
 		     : "memory");
 
 	return flags;
@@ -28,10 +33,13 @@ static inline unsigned long native_save_
 
 static inline void native_restore_fl(unsigned long flags)
 {
-	asm volatile("push %0 ; popf"
+	CFI_DECL;
+
+	asm volatile("push %[flags]" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
+		     "popf" CFI_ADJUST_CFA_OFFSET(-1)
 		     : /* no output */
-		     :"g" (flags)
-		     :"memory", "cc");
+		     : CFI_INPUTS([flags] "g" (flags))
+		     : "memory", "cc");
 }
 
 static inline void native_irq_disable(void)
--- 3.18-rc3/arch/x86/include/asm/processor.h
+++ 3.18-rc3-x86-inline-unwind-info/arch/x86/include/asm/processor.h
@@ -8,7 +8,6 @@ struct task_struct;
 struct mm_struct;
 
 #include <asm/vm86.h>
-#include <asm/math_emu.h>
 #include <asm/segment.h>
 #include <asm/types.h>
 #include <asm/sigcontext.h>
@@ -22,6 +21,11 @@ struct mm_struct;
 #include <asm/nops.h>
 #include <asm/special_insns.h>
 
+#ifdef CONFIG_X86_32
+#include <asm/math_emu.h>
+#include <asm/frame.h>
+#endif
+
 #include <linux/personality.h>
 #include <linux/cpumask.h>
 #include <linux/cache.h>
@@ -531,15 +535,17 @@ static inline void native_set_iopl_mask(
 {
 #ifdef CONFIG_X86_32
 	unsigned int reg;
+	CFI_DECL;
 
-	asm volatile ("pushfl;"
-		      "popl %0;"
-		      "andl %1, %0;"
-		      "orl %2, %0;"
-		      "pushl %0;"
-		      "popfl"
+	asm volatile ("pushfl" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
+		      "popl %0" CFI_ADJUST_CFA_OFFSET(-1) "\n\t"
+		      "andl %[invmask], %0\n\t"
+		      "orl %[mask], %0\n\t"
+		      "pushl %0" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
+		      "popfl" CFI_ADJUST_CFA_OFFSET(-1)
 		      : "=&r" (reg)
-		      : "i" (~X86_EFLAGS_IOPL), "r" (mask));
+		      : CFI_INPUTS([invmask] "i" (~X86_EFLAGS_IOPL),
+				   [mask] "r" (mask)));
 #endif
 }
 
--- 3.18-rc3/arch/x86/kernel/irq_32.c
+++ 3.18-rc3-x86-inline-unwind-info/arch/x86/kernel/irq_32.c
@@ -20,6 +20,7 @@
 #include <linux/mm.h>
 
 #include <asm/apic.h>
+#include <asm/frame.h>
 
 DEFINE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 EXPORT_PER_CPU_SYMBOL(irq_stat);
@@ -60,12 +61,15 @@ DEFINE_PER_CPU(struct irq_stack *, softi
 
 static void call_on_stack(void *func, void *stack)
 {
+	CFI_DECL;
+
 	asm volatile("xchgl	%%ebx,%%esp	\n"
+		     CFI_DEF_CFA_REGISTER(ebx) "\n"
 		     "call	*%%edi		\n"
 		     "movl	%%ebx,%%esp	\n"
+		     CFI_DEF_CFA_REGISTER(esp)
 		     : "=b" (stack)
-		     : "0" (stack),
-		       "D"(func)
+		     : CFI_INPUTS("0" (stack), "D" (func))
 		     : "memory", "cc", "edx", "ecx", "eax");
 }
 
@@ -86,6 +90,7 @@ execute_on_irq_stack(int overflow, struc
 {
 	struct irq_stack *curstk, *irqstk;
 	u32 *isp, *prev_esp, arg1, arg2;
+	CFI_DECL;
 
 	curstk = (struct irq_stack *) current_stack();
 	irqstk = __this_cpu_read(hardirq_stack);
@@ -109,11 +114,13 @@ execute_on_irq_stack(int overflow, struc
 		call_on_stack(print_stack_overflow, isp);
 
 	asm volatile("xchgl	%%ebx,%%esp	\n"
+		     CFI_DEF_CFA_REGISTER(ebx) "\n"
 		     "call	*%%edi		\n"
 		     "movl	%%ebx,%%esp	\n"
+		     CFI_DEF_CFA_REGISTER(esp)
 		     : "=a" (arg1), "=d" (arg2), "=b" (isp)
-		     :  "0" (irq),   "1" (desc),  "2" (isp),
-			"D" (desc->handle_irq)
+		     : CFI_INPUTS("0" (irq),   "1" (desc),  "2" (isp),
+				  "D" (desc->handle_irq))
 		     : "memory", "cc", "ecx");
 	return 1;
 }
--- 3.18-rc3/drivers/cpufreq/speedstep-smi.c
+++ 3.18-rc3-x86-inline-unwind-info/drivers/cpufreq/speedstep-smi.c
@@ -19,6 +19,7 @@
 #include <linux/cpufreq.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <asm/frame.h>
 #include <asm/ist.h>
 #include <asm/cpu_device_id.h>
 
@@ -64,6 +65,7 @@ static int speedstep_smi_ownership(void)
 	u32 command, result, magic, dummy;
 	u32 function = GET_SPEEDSTEP_OWNER;
 	unsigned char magic_data[] = "Copyright (c) 1999 Intel Corporation";
+	CFI_DECL;
 
 	command = (smi_sig & 0xffffff00) | (smi_cmd & 0xff);
 	magic = virt_to_phys(magic_data);
@@ -72,14 +74,14 @@ static int speedstep_smi_ownership(void)
 			command, smi_port);
 
 	__asm__ __volatile__(
-		"push %%ebp\n"
+		"push %%ebp" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
 		"out %%al, (%%dx)\n"
-		"pop %%ebp\n"
+		"pop %%ebp" CFI_ADJUST_CFA_OFFSET(-1)
 		: "=D" (result),
 		  "=a" (dummy), "=b" (dummy), "=c" (dummy), "=d" (dummy),
 		  "=S" (dummy)
-		: "a" (command), "b" (function), "c" (0), "d" (smi_port),
-		  "D" (0), "S" (magic)
+		: CFI_INPUTS("a" (command), "b" (function), "c" (0),
+			     "d" (smi_port), "D" (0), "S" (magic))
 		: "memory"
 	);
 
@@ -102,6 +104,7 @@ static int speedstep_smi_get_freqs(unsig
 	u32 command, result = 0, edi, high_mhz, low_mhz, dummy;
 	u32 state = 0;
 	u32 function = GET_SPEEDSTEP_FREQS;
+	CFI_DECL;
 
 	if (!(ist_info.event & 0xFFFF)) {
 		pr_debug("bug #1422 -- can't read freqs from BIOS\n");
@@ -114,17 +117,15 @@ static int speedstep_smi_get_freqs(unsig
 			command, smi_port);
 
 	__asm__ __volatile__(
-		"push %%ebp\n"
+		"push %%ebp" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
 		"out %%al, (%%dx)\n"
-		"pop %%ebp"
+		"pop %%ebp" CFI_ADJUST_CFA_OFFSET(-1)
 		: "=a" (result),
 		  "=b" (high_mhz),
 		  "=c" (low_mhz),
 		  "=d" (state), "=D" (edi), "=S" (dummy)
-		: "a" (command),
-		  "b" (function),
-		  "c" (state),
-		  "d" (smi_port), "S" (0), "D" (0)
+		: CFI_INPUTS("a" (command), "b" (function), "c" (state),
+			     "d" (smi_port), "S" (0), "D" (0))
 	);
 
 	pr_debug("result %x, low_freq %u, high_freq %u\n",
@@ -165,6 +166,8 @@ static void speedstep_set_state(unsigned
 		state, command, smi_port);
 
 	do {
+		CFI_DECL;
+
 		if (retry) {
 			pr_debug("retry %u, previous result %u, waiting...\n",
 					retry, result);
@@ -172,14 +175,15 @@ static void speedstep_set_state(unsigned
 		}
 		retry++;
 		__asm__ __volatile__(
-			"push %%ebp\n"
+			"push %%ebp" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
 			"out %%al, (%%dx)\n"
-			"pop %%ebp"
+			"pop %%ebp" CFI_ADJUST_CFA_OFFSET(-1)
 			: "=b" (new_state), "=D" (result),
 			  "=c" (dummy), "=a" (dummy),
 			  "=d" (dummy), "=S" (dummy)
-			: "a" (command), "b" (function), "c" (state),
-			  "d" (smi_port), "S" (0), "D" (0)
+			: CFI_INPUTS("a" (command), "b" (function),
+				     "c" (state), "d" (smi_port),
+				     "S" (0), "D" (0))
 			);
 	} while ((new_state != state) && (retry <= SMI_TRIES));
 
--- 3.18-rc3/drivers/crypto/padlock-aes.c
+++ 3.18-rc3-x86-inline-unwind-info/drivers/crypto/padlock-aes.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
 #include <asm/byteorder.h>
+#include <asm/frame.h>
 #include <asm/processor.h>
 #include <asm/i387.h>
 
@@ -168,12 +169,13 @@ static inline void padlock_reset_key(str
 {
 	int cpu = raw_smp_processor_id();
 
-	if (cword != per_cpu(paes_last_cword, cpu))
-#ifndef CONFIG_X86_64
-		asm volatile ("pushfl; popfl");
-#else
-		asm volatile ("pushfq; popfq");
-#endif
+	if (cword != per_cpu(paes_last_cword, cpu)) {
+		CFI_DECL;
+
+		asm volatile ("pushf" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
+			      "popf" CFI_ADJUST_CFA_OFFSET(-1)
+			      :: CFI_INPUTS());
+	}
 }
 
 static inline void padlock_store_cword(struct cword *cword)


--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ