Skip to contentSkip to conversation

Compile-time stack frame pointer validation

PATCH Superseded Josh Poimboeuf 2 participants 6 messages May 11, 2015 – May 13, 2015

Subsystems (3)

v3 of 19 Superseded Latest is v19 →

lore
Share & tools
lore
#0 lore raw

v3 cover Compile-time stack frame pointer validation

In discussions around the live kernel patching consistency model RFC [1], Peter and Ingo correctly pointed out that stack traces aren't reliable. And as Ingo said, there's no "strong force" which ensures we can rely on them.

So I've been thinking about how to fix that. My goal is to eventually make stack traces reliable. Or at the very least, to be able to detect at runtime when a given stack trace *might* be unreliable. But improved stack traces would broadly benefit the entire kernel, regardless of the outcome of the live kernel patching consistency model discussions.

This patch set is just the first in a series of proposed stack trace reliability improvements. Future proposals will include runtime stack reliability checking, as well as compile-time and runtime DWARF validations.

As far as I can tell, there are two main obstacles which prevent frame pointer based stack traces from being reliable:

1) Missing frame pointer logic: currently, most assembly functions don't set up the frame pointer.

2) Interrupts: if a function is interrupted before it can save and set up the frame pointer, its caller won't show up in the stack trace.

This patch set aims to remove the first obstacle by enforcing that all asm functions honor CONFIG_FRAME_POINTER. This is done with a new stackvalidate host tool which is automatically run for every compiled .S file and which validates that every asm function does the proper frame pointer setup.

Also, to make sure somebody didn't forget to annotate their callable asm code as a function, flag an error for any return instructions which are hiding outside of a function. In almost all cases, return instructions are part of callable functions and should be annotated as such so that we can validate their frame pointer usage. A whitelist mechanism exists for those few return instructions which are not actually in callable code.

It currently only supports x86_64. It *almost* supports x86_32, but the stackvalidate code doesn't yet know how to deal with 32-bit REL relocations for the return whitelists. I tried to make the code generic so that support for other architectures can be plugged in pretty easily.

As a first step, all reported non-compliances result in warnings. Right now I'm seeing 200+ warnings. Once we get them all cleaned up, we can change the warnings to build errors so the asm code can stay clean.

The patches are based on linux-next. Patch 1 adds the stackvalidate host tool. Patch 2 is a cleanup which makes the push/pop CFI macros arch-independent, in preparation for patch 3. Patch 3 adds some helper macros for asm functions so that they can comply with stackvalidate.

[1] http://lkml.kernel.org/r/[email protected]

v3:
- Added a patch to make the push/pop CFI macros arch-independent, as suggested by H. Peter Anvin

v2:
- Fixed memory leaks reported by Petr Mladek

Josh Poimboeuf (3):
x86, stackvalidate: Compile-time stack frame pointer validation
x86: Make push/pop CFI macros arch-independent
x86, stackvalidate: Add asm frame pointer setup macros

 MAINTAINERS                           |   6 +
 arch/Kconfig                          |   4 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 arch/x86/ia32/ia32entry.S             |  60 +++---
 arch/x86/include/asm/calling.h        |  28 +--
 arch/x86/include/asm/dwarf2.h         |  92 ++++-----
 arch/x86/include/asm/frame.h          |   4 +-
 arch/x86/include/asm/func.h           |  82 ++++++++
 arch/x86/kernel/entry_32.S            | 214 ++++++++++-----------
 arch/x86/kernel/entry_64.S            |  96 +++++-----
 arch/x86/lib/atomic64_386_32.S        |   4 +-
 arch/x86/lib/atomic64_cx8_32.S        |  40 ++--
 arch/x86/lib/checksum_32.S            |  42 ++--
 arch/x86/lib/cmpxchg16b_emu.S         |   6 +-
 arch/x86/lib/cmpxchg8b_emu.S          |   6 +-
 arch/x86/lib/msr-reg.S                |  34 ++--
 arch/x86/lib/rwsem.S                  |  40 ++--
 arch/x86/lib/thunk_32.S               |  12 +-
 arch/x86/lib/thunk_64.S               |  36 ++--
 lib/Kconfig.debug                     |  11 ++
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  22 ++-
 scripts/stackvalidate/Makefile        |  17 ++
 scripts/stackvalidate/arch-x86.c      | 134 +++++++++++++
 scripts/stackvalidate/arch.h          |  10 +
 scripts/stackvalidate/elf.c           | 352 ++++++++++++++++++++++++++++++++++
 scripts/stackvalidate/elf.h           |  56 ++++++
 scripts/stackvalidate/list.h          | 217 +++++++++++++++++++++
 scripts/stackvalidate/stackvalidate.c | 226 ++++++++++++++++++++++
 30 files changed, 1485 insertions(+), 374 deletions(-)
 create mode 100644 arch/x86/include/asm/func.h
 create mode 100644 scripts/stackvalidate/Makefile
 create mode 100644 scripts/stackvalidate/arch-x86.c
 create mode 100644 scripts/stackvalidate/arch.h
 create mode 100644 scripts/stackvalidate/elf.c
 create mode 100644 scripts/stackvalidate/elf.h
 create mode 100644 scripts/stackvalidate/list.h
 create mode 100644 scripts/stackvalidate/stackvalidate.c
Signature · 1 line
2.1.0
#1 lore raw

PATCH v3 1/3 x86, stackvalidate: Compile-time stack frame pointer validation

Frame pointer based stack traces aren't always reliable. One big reason is that most asm functions don't set up the frame pointer.

Fix that by enforcing that all asm functions honor CONFIG_FRAME_POINTER. This is done with a new stackvalidate host tool which is automatically run for every compiled .S file and which validates that every asm function does the proper frame pointer setup.

Also, to make sure somebody didn't forget to annotate their callable asm code as a function, flag an error for any return instructions which are hiding outside of a function. In almost all cases, return instructions are part of callable functions and should be annotated as such so that we can validate their frame pointer usage. A whitelist mechanism exists for those few return instructions which are not actually in callable code.

It currently only supports x86_64. It *almost* supports x86_32, but the stackvalidate code doesn't yet know how to deal with 32-bit REL relocations for the return whitelists. I tried to make the code generic so that support for other architectures can be plugged in pretty easily.

As a first step, all reported non-compliances result in warnings. Right now I'm seeing 200+ warnings. Once we get them all cleaned up, we can change the warnings to build errors so the asm code can stay clean.

---
 MAINTAINERS                           |   6 +
 arch/Kconfig                          |   4 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 lib/Kconfig.debug                     |  11 ++
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  22 ++-
 scripts/stackvalidate/Makefile        |  17 ++
 scripts/stackvalidate/arch-x86.c      | 134 +++++++++++++
 scripts/stackvalidate/arch.h          |  10 +
 scripts/stackvalidate/elf.c           | 352 ++++++++++++++++++++++++++++++++++
 scripts/stackvalidate/elf.h           |  56 ++++++
 scripts/stackvalidate/list.h          | 217 +++++++++++++++++++++
 scripts/stackvalidate/stackvalidate.c | 226 ++++++++++++++++++++++
 14 files changed, 1060 insertions(+), 3 deletions(-)
 create mode 100644 scripts/stackvalidate/Makefile
 create mode 100644 scripts/stackvalidate/arch-x86.c
 create mode 100644 scripts/stackvalidate/arch.h
 create mode 100644 scripts/stackvalidate/elf.c
 create mode 100644 scripts/stackvalidate/elf.h
 create mode 100644 scripts/stackvalidate/list.h
 create mode 100644 scripts/stackvalidate/stackvalidate.c
MAINTAINERS +6 -0
index ab73992..3eb915e 100644
@@ -9401,6 +9401,12 @@ L:	[email protected]
 S:	Supported
 F:	Documentation/stable_kernel_rules.txt
 
+STACK VALIDATION
+M:	Josh Poimboeuf <[email protected]>
+S:	Supported
+F:	scripts/stackvalidate/
+F:	arch/x86/include/asm/func.h
+
 STAGING SUBSYSTEM
 M:	Greg Kroah-Hartman <[email protected]>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
arch/Kconfig +4 -0
index a65eafb..9e7e388 100644
@@ -499,6 +499,10 @@ config ARCH_HAS_ELF_RANDOMIZE
 	  - arch_mmap_rnd()
 	  - arch_randomize_brk()
 
+config HAVE_STACK_VALIDATION
+	bool
+
+
 #
 # ABI hall of shame
 #
arch/x86/Kconfig +1 -0
index 879a112..155d3e9 100644
@@ -145,6 +145,7 @@ config X86
 	select ACPI_LEGACY_TABLES_LOOKUP if ACPI
 	select X86_FEATURE_NAMES if PROC_FS
 	select SRCU
+	select HAVE_STACK_VALIDATION if FRAME_POINTER && X86_64
 
 config INSTRUCTION_DECODER
 	def_bool y
arch/x86/Makefile +5 -1
index c7c3187..65b2fbd 100644
@@ -174,9 +174,13 @@ KBUILD_CFLAGS += $(call cc-option,-mno-avx,)
 KBUILD_CFLAGS += $(mflags-y)
 KBUILD_AFLAGS += $(mflags-y)
 
-archscripts: scripts_basic
+archscripts: scripts_basic $(objtree)/arch/x86/lib/inat-tables.c
 	$(Q)$(MAKE) $(build)=arch/x86/tools relocs
 
+# this file is needed early by scripts/stackvalidate
+$(objtree)/arch/x86/lib/inat-tables.c:
+	$(Q)$(MAKE) $(build)=arch/x86/lib $@
+
 ###
 # Syscall table generation
 
lib/Kconfig.debug +11 -0
index 8eaed3dd..b8a6884 100644
@@ -332,6 +332,17 @@ config FRAME_POINTER
 	  larger and slower, but it gives very useful debugging information
 	  in case of kernel bugs. (precise oopses/stacktraces/warnings)
 
+
+config STACK_VALIDATION
+	bool "Enable kernel stack validation"
+	depends on HAVE_STACK_VALIDATION
+	default y
+	help
+	  Add compile-time validations which help make kernel stack traces more
+	  reliable.  This includes checks to ensure that assembly functions
+	  save, update and restore the frame pointer or the back chain pointer.
+
+
 config DEBUG_FORCE_WEAK_PER_CPU
 	bool "Force weak per-cpu definitions"
 	depends on DEBUG_KERNEL
scripts/Makefile +1 -0
index 2016a64..c882a91 100644
@@ -37,6 +37,7 @@ subdir-y                     += mod
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_DTC)         += dtc
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
+subdir-$(CONFIG_STACK_VALIDATION) += stackvalidate
 
 # Let clean descend into subdirs
 subdir-	+= basic kconfig package
scripts/Makefile.build +20 -2
index 01df30a..3b05833 100644
@@ -253,6 +253,24 @@ define rule_cc_o_c
 	mv -f $(dot-target).tmp $(dot-target).cmd
 endef
 
+ifdef CONFIG_STACK_VALIDATION
+stackvalidate = $(objtree)/scripts/stackvalidate/stackvalidate
+cmd_stackvalidate =							  \
+	case $(@) in							  \
+		arch/x86/purgatory/*) ;;				  \
+		*) $(stackvalidate) "$(@)"; \
+	esac;
+endif
+
+define rule_as_o_S
+	$(call echo-cmd,as_o_S) $(cmd_as_o_S);				  \
+	$(cmd_stackvalidate)						  \
+	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,as_o_S)' >    \
+	                                              $(dot-target).tmp;  \
+	rm -f $(depfile);						  \
+	mv -f $(dot-target).tmp $(dot-target).cmd
+endef
+
 # Built-in and composite module parts
 $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)
@@ -290,8 +308,8 @@ $(obj)/%.s: $(src)/%.S FORCE
 quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
 cmd_as_o_S       = $(CC) $(a_flags) -c -o $@ $<
 
-$(obj)/%.o: $(src)/%.S FORCE
-	$(call if_changed_dep,as_o_S)
+$(obj)/%.o: $(src)/%.S $(stackvalidate) FORCE
+	$(call if_changed_rule,as_o_S)
 
 targets += $(real-objs-y) $(real-objs-m) $(lib-y)
 targets += $(extra-y) $(MAKECMDGOALS) $(always)
scripts/stackvalidate/Makefile new file +17 -0
new file mode 100644
index 0000000..6027ec4
@@ -0,0 +1,17 @@
+hostprogs-y := stackvalidate
+always := $(hostprogs-y)
+
+stackvalidate-objs := stackvalidate.o elf.o
+
+HOSTCFLAGS += -Werror
+HOSTLOADLIBES_stackvalidate := -lelf
+
+ifdef CONFIG_X86
+
+stackvalidate-objs += arch-x86.o
+
+HOSTCFLAGS_arch-x86.o := -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/
+
+$(obj)/arch-x86.o: $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c $(srctree)/arch/x86/include/asm/inat_types.h $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+
+endif
scripts/stackvalidate/arch-x86.c new file +134 -0

134 lines not on this page · raw message

scripts/stackvalidate/arch.h new file +10 -0
new file mode 100644
index 0000000..3b91b1c
@@ -0,0 +1,10 @@
+#ifndef _ARCH_H_
+#define _ARCH_H_
+
+#include "elf.h"
+
+int arch_validate_function(struct elf *elf, struct symbol *func);
+int arch_is_return_insn(struct elf *elf, unsigned long addr,
+			unsigned int maxlen, unsigned int *len);
+
+#endif /* _ARCH_H_ */
scripts/stackvalidate/elf.c new file +352 -0

352 lines not on this page · raw message

scripts/stackvalidate/elf.h new file +56 -0

56 lines not on this page · raw message

scripts/stackvalidate/list.h new file +217 -0

217 lines not on this page · raw message

scripts/stackvalidate/stackvalidate.c new file +226 -0

226 lines not on this page · raw message

Signature · 1 line
2.1.0
#2 lore raw

Re: [PATCH v3 1/3] x86, stackvalidate: Compile-time stack frame pointer validation

Quoted 25 lines · Josh Poimboeuf

Frame pointer based stack traces aren't always reliable. One big reason is that most asm functions don't set up the frame pointer.

Fix that by enforcing that all asm functions honor CONFIG_FRAME_POINTER. This is done with a new stackvalidate host tool which is automatically run for every compiled .S file and which validates that every asm function does the proper frame pointer setup.

Also, to make sure somebody didn't forget to annotate their callable asm code as a function, flag an error for any return instructions which are hiding outside of a function. In almost all cases, return instructions are part of callable functions and should be annotated as such so that we can validate their frame pointer usage. A whitelist mechanism exists for those few return instructions which are not actually in callable code.

It currently only supports x86_64. It *almost* supports x86_32, but the stackvalidate code doesn't yet know how to deal with 32-bit REL relocations for the return whitelists. I tried to make the code generic so that support for other architectures can be plugged in pretty easily.

As a first step, all reported non-compliances result in warnings. Right now I'm seeing 200+ warnings. Once we get them all cleaned up, we can change the warnings to build errors so the asm code can stay clean.

Quoted 8 lines
---
 MAINTAINERS                           |   6 +
 arch/Kconfig                          |   4 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 lib/Kconfig.debug                     |  11 ++
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  22 ++-

For the kbuild parts: Acked-by: Michal Marek <[email protected]>

Quoted 12 lines
scripts/stackvalidate/stackvalidate.c new file +12 -0
+int main(int argc, char *argv[])
+{
+	struct args args;
+	struct elf *elf;
+	struct section *sec;
+	int ret, warnings = 0;
+
+	argp_parse(&argp, argc, argv, 0, 0, &args);
+
+	elf = elf_open(args.args[0]);
+	if (!elf) {
+		fprintf(stderr, "error reading elf file %s\n", args.args[0]);

scripts/stackvalidate/stackvalidate.c line 219

Quoted 10 lines
scripts/stackvalidate/stackvalidate.c new file +10 -0
+		return 1;
+	}
+
+	if (is_file_whitelisted(elf))
+		return 0;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		ret = validate_section(elf, sec);
+		if (ret < 0)
+			return -1;

return 1? Since this is the exit status of the program.

Michal

#3 lore raw

Re: [PATCH v3 1/3] x86, stackvalidate: Compile-time stack frame pointer validation

Quoted 35 lines · Michal Marek
Quoted 25 lines · Josh Poimboeuf

Frame pointer based stack traces aren't always reliable. One big reason is that most asm functions don't set up the frame pointer.

Fix that by enforcing that all asm functions honor CONFIG_FRAME_POINTER. This is done with a new stackvalidate host tool which is automatically run for every compiled .S file and which validates that every asm function does the proper frame pointer setup.

Also, to make sure somebody didn't forget to annotate their callable asm code as a function, flag an error for any return instructions which are hiding outside of a function. In almost all cases, return instructions are part of callable functions and should be annotated as such so that we can validate their frame pointer usage. A whitelist mechanism exists for those few return instructions which are not actually in callable code.

It currently only supports x86_64. It *almost* supports x86_32, but the stackvalidate code doesn't yet know how to deal with 32-bit REL relocations for the return whitelists. I tried to make the code generic so that support for other architectures can be plugged in pretty easily.

As a first step, all reported non-compliances result in warnings. Right now I'm seeing 200+ warnings. Once we get them all cleaned up, we can change the warnings to build errors so the asm code can stay clean.

Quoted 8 lines
---
 MAINTAINERS                           |   6 +
 arch/Kconfig                          |   4 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 lib/Kconfig.debug                     |  11 ++
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  22 ++-
Quoted 1 line

For the kbuild parts: Acked-by: Michal Marek <[email protected]>

Thanks!

Quoted 16 lines
Quoted 16 lines
scripts/stackvalidate/stackvalidate.c new file +16 -0
+int main(int argc, char *argv[])
+{
+	struct args args;
+	struct elf *elf;
+	struct section *sec;
+	int ret, warnings = 0;
+
+	argp_parse(&argp, argc, argv, 0, 0, &args);
+
+	elf = elf_open(args.args[0]);
+	if (!elf) {
+		fprintf(stderr, "error reading elf file %s\n", args.args[0]);
+		return 1;
+	}
+
+	if (is_file_whitelisted(elf))
Quoted 8 lines

scripts/stackvalidate/stackvalidate.c line 219

Quoted 6 lines
scripts/stackvalidate/stackvalidate.c new file +6 -0
+		return 0;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		ret = validate_section(elf, sec);
+		if (ret < 0)
+			return -1;

return 1? Since this is the exit status of the program.

Ok, I'll change it to 1.

Signature · 1 line
Josh
#4 lore raw

PATCH v3 2/3 x86: Make push/pop CFI macros arch-independent

In reply to Josh Poimboeuf

The separate push{lq}_cfi and pop_{lq}_cfi macros aren't needed. Push and pop only come in one size per architecture, so the trailing 'q' or 'l' characters are redundant, and awkward to use in arch-independent code.

Replace the push/pop CFI macros with architecture-independent versions:
push_cfi, pop_cfi, etc.

This change is purely cosmetic, with no resulting object code changes.

---
 arch/x86/ia32/ia32entry.S      |  60 ++++++------
 arch/x86/include/asm/calling.h |  28 +++---
 arch/x86/include/asm/dwarf2.h  |  92 ++++++------------
 arch/x86/include/asm/frame.h   |   4 +-
 arch/x86/kernel/entry_32.S     | 214 ++++++++++++++++++++---------------------
 arch/x86/kernel/entry_64.S     |  96 +++++++++---------
 arch/x86/lib/atomic64_386_32.S |   4 +-
 arch/x86/lib/atomic64_cx8_32.S |  40 ++++----
 arch/x86/lib/checksum_32.S     |  42 ++++----
 arch/x86/lib/cmpxchg16b_emu.S  |   6 +-
 arch/x86/lib/cmpxchg8b_emu.S   |   6 +-
 arch/x86/lib/msr-reg.S         |  34 +++----
 arch/x86/lib/rwsem.S           |  40 ++++----
 arch/x86/lib/thunk_32.S        |  12 +--
 arch/x86/lib/thunk_64.S        |  36 +++----
 15 files changed, 343 insertions(+), 371 deletions(-)
arch/x86/ia32/ia32entry.S +30 -30
index 63450a5..1664d12 100644
@@ -124,19 +124,19 @@ ENTRY(ia32_sysenter_target)
 	CFI_REGISTER rip,r10
 
 	/* Construct struct pt_regs on stack */
-	pushq_cfi	$__USER32_DS		/* pt_regs->ss */
-	pushq_cfi	%rbp			/* pt_regs->sp */
+	push_cfi	$__USER32_DS		/* pt_regs->ss */
+	push_cfi	%rbp			/* pt_regs->sp */
 	CFI_REL_OFFSET	rsp,0
-	pushfq_cfi				/* pt_regs->flags */
-	pushq_cfi	$__USER32_CS		/* pt_regs->cs */
-	pushq_cfi	%r10 /* pt_regs->ip = thread_info->sysenter_return */
+	pushf_cfi				/* pt_regs->flags */
+	push_cfi	$__USER32_CS		/* pt_regs->cs */
+	push_cfi	%r10 /* pt_regs->ip = thread_info->sysenter_return */
 	CFI_REL_OFFSET	rip,0
-	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
-	pushq_cfi_reg	rdi			/* pt_regs->di */
-	pushq_cfi_reg	rsi			/* pt_regs->si */
-	pushq_cfi_reg	rdx			/* pt_regs->dx */
-	pushq_cfi_reg	rcx			/* pt_regs->cx */
-	pushq_cfi	$-ENOSYS		/* pt_regs->ax */
+	push_cfi_reg	rax			/* pt_regs->orig_ax */
+	push_cfi_reg	rdi			/* pt_regs->di */
+	push_cfi_reg	rsi			/* pt_regs->si */
+	push_cfi_reg	rdx			/* pt_regs->dx */
+	push_cfi_reg	rcx			/* pt_regs->cx */
+	push_cfi	$-ENOSYS		/* pt_regs->ax */
 	cld
 	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
 	CFI_ADJUST_CFA_OFFSET 10*8
@@ -282,8 +282,8 @@ sysexit_audit:
 #endif
 
 sysenter_fix_flags:
-	pushq_cfi $(X86_EFLAGS_IF|X86_EFLAGS_FIXED)
-	popfq_cfi
+	push_cfi $(X86_EFLAGS_IF|X86_EFLAGS_FIXED)
+	popf_cfi
 	jmp sysenter_flags_fixed
 
 sysenter_tracesys:
@@ -353,20 +353,20 @@ ENTRY(ia32_cstar_target)
 	movl	%eax,%eax
 
 	/* Construct struct pt_regs on stack */
-	pushq_cfi	$__USER32_DS		/* pt_regs->ss */
-	pushq_cfi	%r8			/* pt_regs->sp */
+	push_cfi	$__USER32_DS		/* pt_regs->ss */
+	push_cfi	%r8			/* pt_regs->sp */
 	CFI_REL_OFFSET rsp,0
-	pushq_cfi	%r11			/* pt_regs->flags */
-	pushq_cfi	$__USER32_CS		/* pt_regs->cs */
-	pushq_cfi	%rcx			/* pt_regs->ip */
+	push_cfi	%r11			/* pt_regs->flags */
+	push_cfi	$__USER32_CS		/* pt_regs->cs */
+	push_cfi	%rcx			/* pt_regs->ip */
 	CFI_REL_OFFSET rip,0
-	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
-	pushq_cfi_reg	rdi			/* pt_regs->di */
-	pushq_cfi_reg	rsi			/* pt_regs->si */
-	pushq_cfi_reg	rdx			/* pt_regs->dx */
-	pushq_cfi_reg	rbp			/* pt_regs->cx */
+	push_cfi_reg	rax			/* pt_regs->orig_ax */
+	push_cfi_reg	rdi			/* pt_regs->di */
+	push_cfi_reg	rsi			/* pt_regs->si */
+	push_cfi_reg	rdx			/* pt_regs->dx */
+	push_cfi_reg	rbp			/* pt_regs->cx */
 	movl	%ebp,%ecx
-	pushq_cfi	$-ENOSYS		/* pt_regs->ax */
+	push_cfi	$-ENOSYS		/* pt_regs->ax */
 	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
 	CFI_ADJUST_CFA_OFFSET 10*8
 
@@ -506,12 +506,12 @@ ENTRY(ia32_syscall)
 	movl	%eax,%eax
 
 	/* Construct struct pt_regs on stack (iret frame is already on stack) */
-	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
-	pushq_cfi_reg	rdi			/* pt_regs->di */
-	pushq_cfi_reg	rsi			/* pt_regs->si */
-	pushq_cfi_reg	rdx			/* pt_regs->dx */
-	pushq_cfi_reg	rcx			/* pt_regs->cx */
-	pushq_cfi	$-ENOSYS		/* pt_regs->ax */
+	push_cfi_reg	rax			/* pt_regs->orig_ax */
+	push_cfi_reg	rdi			/* pt_regs->di */
+	push_cfi_reg	rsi			/* pt_regs->si */
+	push_cfi_reg	rdx			/* pt_regs->dx */
+	push_cfi_reg	rcx			/* pt_regs->cx */
+	push_cfi	$-ENOSYS		/* pt_regs->ax */
 	cld
 	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
 	CFI_ADJUST_CFA_OFFSET 10*8
arch/x86/include/asm/calling.h +14 -14
index 1c8b50e..4abc60f 100644
@@ -224,23 +224,23 @@ For 32-bit we have the following conventions - kernel is built with
  */
 
 	.macro SAVE_ALL
-	pushl_cfi_reg eax
-	pushl_cfi_reg ebp
-	pushl_cfi_reg edi
-	pushl_cfi_reg esi
-	pushl_cfi_reg edx
-	pushl_cfi_reg ecx
-	pushl_cfi_reg ebx
+	push_cfi_reg eax
+	push_cfi_reg ebp
+	push_cfi_reg edi
+	push_cfi_reg esi
+	push_cfi_reg edx
+	push_cfi_reg ecx
+	push_cfi_reg ebx
 	.endm
 
 	.macro RESTORE_ALL
-	popl_cfi_reg ebx
-	popl_cfi_reg ecx
-	popl_cfi_reg edx
-	popl_cfi_reg esi
-	popl_cfi_reg edi
-	popl_cfi_reg ebp
-	popl_cfi_reg eax
+	pop_cfi_reg ebx
+	pop_cfi_reg ecx
+	pop_cfi_reg edx
+	pop_cfi_reg esi
+	pop_cfi_reg edi
+	pop_cfi_reg ebp
+	pop_cfi_reg eax
 	.endm
 
 #endif /* CONFIG_X86_64 */
arch/x86/include/asm/dwarf2.h +32 -60

124 lines not on this page · raw message

arch/x86/include/asm/frame.h +2 -2
index 3b629f4..325e4e8 100644
@@ -8,12 +8,12 @@
    frame pointer later */
 #ifdef CONFIG_FRAME_POINTER
 	.macro FRAME
-	__ASM_SIZE(push,_cfi)	%__ASM_REG(bp)
+	push_cfi		%__ASM_REG(bp)
 	CFI_REL_OFFSET		__ASM_REG(bp), 0
 	__ASM_SIZE(mov)		%__ASM_REG(sp), %__ASM_REG(bp)
 	.endm
 	.macro ENDFRAME
-	__ASM_SIZE(pop,_cfi)	%__ASM_REG(bp)
+	pop_cfi			%__ASM_REG(bp)
 	CFI_RESTORE		__ASM_REG(bp)
 	.endm
 #else
arch/x86/kernel/entry_32.S +107 -107

533 lines not on this page · raw message

arch/x86/kernel/entry_64.S +48 -48

242 lines not on this page · raw message

arch/x86/lib/atomic64_386_32.S +2 -2
index 00933d5..aa17c69 100644
@@ -15,12 +15,12 @@
 
 /* if you want SMP support, implement these with real spinlocks */
 .macro LOCK reg
-	pushfl_cfi
+	pushf_cfi
 	cli
 .endm
 
 .macro UNLOCK reg
-	popfl_cfi
+	popf_cfi
 .endm
 
 #define BEGIN(op) \
arch/x86/lib/atomic64_cx8_32.S +20 -20
index 082a851..c5dd086 100644
@@ -57,10 +57,10 @@ ENDPROC(atomic64_xchg_cx8)
 .macro addsub_return func ins insc
 ENTRY(atomic64_\func\()_return_cx8)
 	CFI_STARTPROC
-	pushl_cfi_reg ebp
-	pushl_cfi_reg ebx
-	pushl_cfi_reg esi
-	pushl_cfi_reg edi
+	push_cfi_reg ebp
+	push_cfi_reg ebx
+	push_cfi_reg esi
+	push_cfi_reg edi
 
 	movl %eax, %esi
 	movl %edx, %edi
@@ -79,10 +79,10 @@ ENTRY(atomic64_\func\()_return_cx8)
 10:
 	movl %ebx, %eax
 	movl %ecx, %edx
-	popl_cfi_reg edi
-	popl_cfi_reg esi
-	popl_cfi_reg ebx
-	popl_cfi_reg ebp
+	pop_cfi_reg edi
+	pop_cfi_reg esi
+	pop_cfi_reg ebx
+	pop_cfi_reg ebp
 	ret
 	CFI_ENDPROC
 ENDPROC(atomic64_\func\()_return_cx8)
@@ -94,7 +94,7 @@ addsub_return sub sub sbb
 .macro incdec_return func ins insc
 ENTRY(atomic64_\func\()_return_cx8)
 	CFI_STARTPROC
-	pushl_cfi_reg ebx
+	push_cfi_reg ebx
 
 	read64 %esi
 1:
@@ -109,7 +109,7 @@ ENTRY(atomic64_\func\()_return_cx8)
 10:
 	movl %ebx, %eax
 	movl %ecx, %edx
-	popl_cfi_reg ebx
+	pop_cfi_reg ebx
 	ret
 	CFI_ENDPROC
 ENDPROC(atomic64_\func\()_return_cx8)
@@ -120,7 +120,7 @@ incdec_return dec sub sbb
 
 ENTRY(atomic64_dec_if_positive_cx8)
 	CFI_STARTPROC
-	pushl_cfi_reg ebx
+	push_cfi_reg ebx
 
 	read64 %esi
 1:
@@ -136,18 +136,18 @@ ENTRY(atomic64_dec_if_positive_cx8)
 2:
 	movl %ebx, %eax
 	movl %ecx, %edx
-	popl_cfi_reg ebx
+	pop_cfi_reg ebx
 	ret
 	CFI_ENDPROC
 ENDPROC(atomic64_dec_if_positive_cx8)
 
 ENTRY(atomic64_add_unless_cx8)
 	CFI_STARTPROC
-	pushl_cfi_reg ebp
-	pushl_cfi_reg ebx
+	push_cfi_reg ebp
+	push_cfi_reg ebx
 /* these just push these two parameters on the stack */
-	pushl_cfi_reg edi
-	pushl_cfi_reg ecx
+	push_cfi_reg edi
+	push_cfi_reg ecx
 
 	movl %eax, %ebp
 	movl %edx, %edi
@@ -169,8 +169,8 @@ ENTRY(atomic64_add_unless_cx8)
 3:
 	addl $8, %esp
 	CFI_ADJUST_CFA_OFFSET -8
-	popl_cfi_reg ebx
-	popl_cfi_reg ebp
+	pop_cfi_reg ebx
+	pop_cfi_reg ebp
 	ret
 4:
 	cmpl %edx, 4(%esp)
@@ -182,7 +182,7 @@ ENDPROC(atomic64_add_unless_cx8)
 
 ENTRY(atomic64_inc_not_zero_cx8)
 	CFI_STARTPROC
-	pushl_cfi_reg ebx
+	push_cfi_reg ebx
 
 	read64 %esi
 1:
@@ -199,7 +199,7 @@ ENTRY(atomic64_inc_not_zero_cx8)
 
 	movl $1, %eax
 3:
-	popl_cfi_reg ebx
+	pop_cfi_reg ebx
 	ret
 	CFI_ENDPROC
 ENDPROC(atomic64_inc_not_zero_cx8)
arch/x86/lib/checksum_32.S +21 -21

90 lines not on this page · raw message

arch/x86/lib/cmpxchg16b_emu.S +3 -3
index 40a1725..b18f317 100644
@@ -32,7 +32,7 @@ CFI_STARTPROC
 # *atomic* on a single cpu (as provided by the this_cpu_xx class of
 # macros).
 #
-	pushfq_cfi
+	pushf_cfi
 	cli
 
 	cmpq PER_CPU_VAR((%rsi)), %rax
@@ -44,13 +44,13 @@ CFI_STARTPROC
 	movq %rcx, PER_CPU_VAR(8(%rsi))
 
 	CFI_REMEMBER_STATE
-	popfq_cfi
+	popf_cfi
 	mov $1, %al
 	ret
 
 	CFI_RESTORE_STATE
 .Lnot_same:
-	popfq_cfi
+	popf_cfi
 	xor %al,%al
 	ret
 
arch/x86/lib/cmpxchg8b_emu.S +3 -3
index b4807fce..a4862d0 100644
@@ -27,7 +27,7 @@ CFI_STARTPROC
 # set the whole ZF thing (caller will just compare
 # eax:edx with the expected value)
 #
-	pushfl_cfi
+	pushf_cfi
 	cli
 
 	cmpl  (%esi), %eax
@@ -39,7 +39,7 @@ CFI_STARTPROC
 	movl %ecx, 4(%esi)
 
 	CFI_REMEMBER_STATE
-	popfl_cfi
+	popf_cfi
 	ret
 
 	CFI_RESTORE_STATE
@@ -48,7 +48,7 @@ CFI_STARTPROC
 .Lhalf_same:
 	movl 4(%esi), %edx
 
-	popfl_cfi
+	popf_cfi
 	ret
 
 CFI_ENDPROC
arch/x86/lib/msr-reg.S +17 -17
index 3ca5218..046a560 100644
@@ -14,8 +14,8 @@
 .macro op_safe_regs op
 ENTRY(\op\()_safe_regs)
 	CFI_STARTPROC
-	pushq_cfi_reg rbx
-	pushq_cfi_reg rbp
+	push_cfi_reg rbx
+	push_cfi_reg rbp
 	movq	%rdi, %r10	/* Save pointer */
 	xorl	%r11d, %r11d	/* Return value */
 	movl    (%rdi), %eax
@@ -35,8 +35,8 @@ ENTRY(\op\()_safe_regs)
 	movl    %ebp, 20(%r10)
 	movl    %esi, 24(%r10)
 	movl    %edi, 28(%r10)
-	popq_cfi_reg rbp
-	popq_cfi_reg rbx
+	pop_cfi_reg rbp
+	pop_cfi_reg rbx
 	ret
 3:
 	CFI_RESTORE_STATE
@@ -53,12 +53,12 @@ ENDPROC(\op\()_safe_regs)
 .macro op_safe_regs op
 ENTRY(\op\()_safe_regs)
 	CFI_STARTPROC
-	pushl_cfi_reg ebx
-	pushl_cfi_reg ebp
-	pushl_cfi_reg esi
-	pushl_cfi_reg edi
-	pushl_cfi $0              /* Return value */
-	pushl_cfi %eax
+	push_cfi_reg ebx
+	push_cfi_reg ebp
+	push_cfi_reg esi
+	push_cfi_reg edi
+	push_cfi $0              /* Return value */
+	push_cfi %eax
 	movl    4(%eax), %ecx
 	movl    8(%eax), %edx
 	movl    12(%eax), %ebx
@@ -68,9 +68,9 @@ ENTRY(\op\()_safe_regs)
 	movl    (%eax), %eax
 	CFI_REMEMBER_STATE
 1:	\op
-2:	pushl_cfi %eax
+2:	push_cfi %eax
 	movl    4(%esp), %eax
-	popl_cfi (%eax)
+	pop_cfi (%eax)
 	addl    $4, %esp
 	CFI_ADJUST_CFA_OFFSET -4
 	movl    %ecx, 4(%eax)
@@ -79,11 +79,11 @@ ENTRY(\op\()_safe_regs)
 	movl    %ebp, 20(%eax)
 	movl    %esi, 24(%eax)
 	movl    %edi, 28(%eax)
-	popl_cfi %eax
-	popl_cfi_reg edi
-	popl_cfi_reg esi
-	popl_cfi_reg ebp
-	popl_cfi_reg ebx
+	pop_cfi %eax
+	pop_cfi_reg edi
+	pop_cfi_reg esi
+	pop_cfi_reg ebp
+	pop_cfi_reg ebx
 	ret
 3:
 	CFI_RESTORE_STATE
arch/x86/lib/rwsem.S +20 -20
index 2322abe..c630a80 100644
@@ -34,10 +34,10 @@
  */
 
 #define save_common_regs \
-	pushl_cfi_reg ecx
+	push_cfi_reg ecx
 
 #define restore_common_regs \
-	popl_cfi_reg ecx
+	pop_cfi_reg ecx
 
 	/* Avoid uglifying the argument copying x86-64 needs to do. */
 	.macro movq src, dst
@@ -64,22 +64,22 @@
  */
 
 #define save_common_regs \
-	pushq_cfi_reg rdi; \
-	pushq_cfi_reg rsi; \
-	pushq_cfi_reg rcx; \
-	pushq_cfi_reg r8;  \
-	pushq_cfi_reg r9;  \
-	pushq_cfi_reg r10; \
-	pushq_cfi_reg r11
+	push_cfi_reg rdi; \
+	push_cfi_reg rsi; \
+	push_cfi_reg rcx; \
+	push_cfi_reg r8;  \
+	push_cfi_reg r9;  \
+	push_cfi_reg r10; \
+	push_cfi_reg r11
 
 #define restore_common_regs \
-	popq_cfi_reg r11; \
-	popq_cfi_reg r10; \
-	popq_cfi_reg r9; \
-	popq_cfi_reg r8; \
-	popq_cfi_reg rcx; \
-	popq_cfi_reg rsi; \
-	popq_cfi_reg rdi
+	pop_cfi_reg r11; \
+	pop_cfi_reg r10; \
+	pop_cfi_reg r9; \
+	pop_cfi_reg r8; \
+	pop_cfi_reg rcx; \
+	pop_cfi_reg rsi; \
+	pop_cfi_reg rdi
 
 #endif
 
@@ -87,10 +87,10 @@
 ENTRY(call_rwsem_down_read_failed)
 	CFI_STARTPROC
 	save_common_regs
-	__ASM_SIZE(push,_cfi_reg) __ASM_REG(dx)
+	push_cfi_reg __ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_down_read_failed
-	__ASM_SIZE(pop,_cfi_reg) __ASM_REG(dx)
+	pop_cfi_reg __ASM_REG(dx)
 	restore_common_regs
 	ret
 	CFI_ENDPROC
@@ -122,10 +122,10 @@ ENDPROC(call_rwsem_wake)
 ENTRY(call_rwsem_downgrade_wake)
 	CFI_STARTPROC
 	save_common_regs
-	__ASM_SIZE(push,_cfi_reg) __ASM_REG(dx)
+	push_cfi_reg __ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_downgrade_wake
-	__ASM_SIZE(pop,_cfi_reg) __ASM_REG(dx)
+	pop_cfi_reg __ASM_REG(dx)
 	restore_common_regs
 	ret
 	CFI_ENDPROC
arch/x86/lib/thunk_32.S +6 -6
index 5eb7150..bb370de 100644
@@ -13,9 +13,9 @@
 	.globl \name
 \name:
 	CFI_STARTPROC
-	pushl_cfi_reg eax
-	pushl_cfi_reg ecx
-	pushl_cfi_reg edx
+	push_cfi_reg eax
+	push_cfi_reg ecx
+	push_cfi_reg edx
 
 	.if \put_ret_addr_in_eax
 	/* Place EIP in the arg1 */
@@ -23,9 +23,9 @@
 	.endif
 
 	call \func
-	popl_cfi_reg edx
-	popl_cfi_reg ecx
-	popl_cfi_reg eax
+	pop_cfi_reg edx
+	pop_cfi_reg ecx
+	pop_cfi_reg eax
 	ret
 	CFI_ENDPROC
 	_ASM_NOKPROBE(\name)
arch/x86/lib/thunk_64.S +18 -18
index f89ba4e9..39ad268 100644
@@ -17,15 +17,15 @@
 	CFI_STARTPROC
 
 	/* this one pushes 9 elems, the next one would be %rIP */
-	pushq_cfi_reg rdi
-	pushq_cfi_reg rsi
-	pushq_cfi_reg rdx
-	pushq_cfi_reg rcx
-	pushq_cfi_reg rax
-	pushq_cfi_reg r8
-	pushq_cfi_reg r9
-	pushq_cfi_reg r10
-	pushq_cfi_reg r11
+	push_cfi_reg rdi
+	push_cfi_reg rsi
+	push_cfi_reg rdx
+	push_cfi_reg rcx
+	push_cfi_reg rax
+	push_cfi_reg r8
+	push_cfi_reg r9
+	push_cfi_reg r10
+	push_cfi_reg r11
 
 	.if \put_ret_addr_in_rdi
 	/* 9*8(%rsp) is return addr on stack */
@@ -60,15 +60,15 @@
 	CFI_STARTPROC
 	CFI_ADJUST_CFA_OFFSET 9*8
 restore:
-	popq_cfi_reg r11
-	popq_cfi_reg r10
-	popq_cfi_reg r9
-	popq_cfi_reg r8
-	popq_cfi_reg rax
-	popq_cfi_reg rcx
-	popq_cfi_reg rdx
-	popq_cfi_reg rsi
-	popq_cfi_reg rdi
+	pop_cfi_reg r11
+	pop_cfi_reg r10
+	pop_cfi_reg r9
+	pop_cfi_reg r8
+	pop_cfi_reg rax
+	pop_cfi_reg rcx
+	pop_cfi_reg rdx
+	pop_cfi_reg rsi
+	pop_cfi_reg rdi
 	ret
 	CFI_ENDPROC
 	_ASM_NOKPROBE(restore)
Signature · 1 line
2.1.0
#5 lore raw

PATCH v3 3/3 x86, stackvalidate: Add asm frame pointer setup macros

In reply to Josh Poimboeuf

Add some helper macros for asm functions so that they can comply with stackvalidate.

The FUNC_ENTER and FUNC_RETURN macros help asm functions save, set up, and restore frame pointers.

The RET_NOVALIDATE and FILE_NOVALIDATE macros can be used to whitelist the few locations which need a return instruction outside of a callable function.

---
 arch/x86/include/asm/func.h | 82 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 arch/x86/include/asm/func.h
arch/x86/include/asm/func.h new file +82 -0
new file mode 100644
index 0000000..cd27ad4
@@ -0,0 +1,82 @@
+#ifndef _ASM_X86_FUNC_H
+#define _ASM_X86_FUNC_H
+
+#include <linux/linkage.h>
+#include <asm/dwarf2.h>
+#include <asm/asm.h>
+
+.macro FUNC_ENTER_NO_FP name
+	ENTRY(\name)
+	CFI_STARTPROC
+	CFI_DEF_CFA _ASM_SP, __ASM_SEL(4, 8)
+.endm
+
+.macro FUNC_RETURN_NO_FP name
+	CFI_DEF_CFA _ASM_SP, __ASM_SEL(4, 8)
+	ret
+	CFI_ENDPROC
+	ENDPROC(\name)
+.endm
+
+#ifdef CONFIG_FRAME_POINTER
+
+.macro FUNC_ENTER_FP name
+	FUNC_ENTER_NO_FP \name
+	push_cfi %_ASM_BP
+	CFI_REL_OFFSET _ASM_BP, 0
+	_ASM_MOV %_ASM_SP, %_ASM_BP
+	CFI_DEF_CFA_REGISTER _ASM_BP
+.endm
+
+.macro FUNC_RETURN_FP name
+	pop_cfi %_ASM_BP
+	CFI_RESTORE _ASM_BP
+	FUNC_RETURN_NO_FP \name
+.endm
+
+/*
+ * Every callable asm function should be bookended with FUNC_ENTER and
+ * FUNC_RETURN.  They do proper frame pointer and DWARF CFI setups in order to
+ * achieve more reliable stack traces.
+ *
+ * For the sake of simplicity and correct DWARF annotations, use of the macros
+ * requires that the return instruction comes at the end of the function.
+ */
+#define FUNC_ENTER(name) FUNC_ENTER_FP name
+#define FUNC_RETURN(name) FUNC_RETURN_FP name
+
+/*
+ * RET_NOVALIDATE tells the stack validation script to whitelist the return
+ * instruction immediately after the macro.  Only use it if you're completely
+ * sure you need a return instruction outside of a callable function.
+ * Otherwise, if the code can be called and you haven't annotated it with
+ * FUNC_ENTER/FUNC_RETURN, it will break stack trace reliability.
+ */
+.macro RET_NOVALIDATE
+	163:
+	.pushsection __stackvalidate_whitelist_ret, "ae"
+	_ASM_ALIGN
+	.long 163b - .
+	.popsection
+.endm
+
+/*
+ * FILE_NOVALIDATE is like RET_NOVALIDATE except it whitelists the entire file.
+ * Use with extreme caution or you will silently break stack traces.
+ */
+.macro FILE_NOVALIDATE
+	.pushsection __stackvalidate_whitelist_file, "ae"
+	.long 0
+	.popsection
+.endm
+
+#else /* !FRAME_POINTER */
+
+#define FUNC_ENTER(name) FUNC_ENTER_NO_FP name
+#define FUNC_RETURN(name) FUNC_RETURN_NO_FP name
+#define RET_NOVALIDATE
+#define FILE_NOVALIDATE
+
+#endif /* FRAME_POINTER */
+
+#endif /* _ASM_X86_FUNC_H */
Signature · 1 line
2.1.0