[PATCH v10 00/20] Compile-time stack validation

[PATCH v10 00/20] Compile-time stack validation

[Josh Poimboeuf]

This is v10 of the compile-time stack validation patch set, along with
proposed fixes for many of the warnings it found.  It's based on the
tip/master branch.

The frame pointer macros have been renamed from FRAME/ENDFRAME to
FRAME_BEGIN/FRAME_END.

v9 can be found here:

  https://lkml.kernel.org/r/cover.1438963009.git.jpoimboe@redhat.com

For more information about the motivation behind this patch set, and
more details about what it does, please see the changelog in patch 3.

Patches 1-5 are the stackvalidate tool and some related macros.

Patches 6-20 are some proposed fixes for several of the warnings
reported by stackvalidate.  They've been compile-tested and boot tested
in a VM, but I haven't attempted any meaningful testing for many of
them.

v10:
- add scripts/mod to directory ignores
- remove circular dependencies for ignored objects which are built
  before stackvalidate
- fix CONFIG_MODVERSIONS incompatibility

v9:
- rename FRAME/ENDFRAME -> FRAME_BEGIN/FRAME_END
- fix jump table issue for when the original instruction is a jump
- drop paravirt thunk alignment patch
- add maintainers to CC for proposed warning fixes

v8:
- add proposed fixes for warnings
- fix all memory leaks
- process ignores earlier and add more ignore checks
- always assume POPCNT alternative is enabled
- drop hweight inline asm fix
- drop __schedule() ignore patch
- change .Ltemp_\@ to .Lstackvalidate_ignore_\@ in asm macro
- fix CONFIG_* checks in asm macros
- add C versions of ignore macros and frame macros
- change ";" to "\n" in C macros
- add ifdef CONFIG_STACK_VALIDATION checks in C ignore macros
- use numbered label in C ignore macro
- add missing break in switch case statement in arch-x86.c

v7:
- sibling call support
- document proposed solution for inline asm() frame pointer issues
- say "kernel entry/exit" instead of "context switch"
- clarify the checking of switch statement jump tables
- discard __stackvalidate_ignore_* sections in linker script
- use .Ltemp_\@ to get a unique label instead of static 3-digit number
- change STACKVALIDATE_IGNORE_FUNC variable to a static
- move STACKVALIDATE_IGNORE_INSN to arch-specific .h file

v6:
- rename asmvalidate -> stackvalidate (again)
- gcc-generated object file support
- recursive branch state analysis
- external jump support
- fixup/exception table support
- jump label support
- switch statement jump table support
- added documentation
- detection of "noreturn" dead end functions
- added a Kbuild mechanism for skipping files and dirs
- moved frame pointer macros to arch/x86/include/asm/frame.h
- moved ignore macros to include/linux/stackvalidate.h

v5:
- stackvalidate -> asmvalidate
- frame pointers only required for non-leaf functions
- check for the use of the FP_SAVE/RESTORE macros instead of manually
  analyzing code to detect frame pointer usage
- additional checks to ensure each function doesn't leave its boundaries
- make the macros simpler and more flexible
- support for analyzing ALTERNATIVE macros
- simplified the arch interfaces in scripts/asmvalidate/arch.h
- fixed some asmvalidate warnings
- rebased onto latest tip asm cleanups
- many more small changes

v4:
- Changed the default to CONFIG_STACK_VALIDATION=n, until all the asm
  code can get cleaned up.
- Fixed a stackvalidate error path exit code issue found by Michal
  Marek.

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

Cc: linux-kernel@vger.kernel.org
Cc: Michal Marek <mmarek@suse.cz>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Pedro Alves <palves@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Bernd Petrovitsch <bernd@petrovitsch.priv.at>
Cc: "Chris J Arges" <chris.j.arges@canonical.com>
Cc: live-patching@vger.kernel.org

Josh Poimboeuf (20):
  x86/asm: Frame pointer macro cleanup
  x86/asm: Add C versions of frame pointer macros
  x86/stackvalidate: Compile-time stack validation
  x86/stackvalidate: Add file and directory ignores
  x86/stackvalidate: Add ignore macros
  x86/xen: Add stack frame dependency to hypercall inline asm calls
  x86/paravirt: Add stack frame dependency to PVOP inline asm calls
  x86/paravirt: Create a stack frame in PV_CALLEE_SAVE_REGS_THUNK
  x86/amd: Set ELF function type for vide()
  x86/reboot: Add ljmp instructions to stackvalidate whitelist
  x86/xen: Add xen_cpuid() and xen_setup_gdt() to stackvalidate
    whitelists
  x86/asm/crypto: Create stack frames in aesni-intel_asm.S
  x86/asm/crypto: Move .Lbswap_mask data to .rodata section
  x86/asm/crypto: Move jump_table to .rodata section
  x86/asm/crypto: Create stack frames in clmul_ghash_mul/update()
  x86/asm/entry: Create stack frames in thunk functions
  x86/asm/acpi: Create a stack frame in do_suspend_lowlevel()
  x86/asm: Create stack frames in rwsem functions
  x86/asm/efi: Create a stack frame in efi_call()
  x86/asm/power: Create stack frames in hibernate_asm_64.S

 Documentation/stack-validation.txt        | 193 ++++++
 MAINTAINERS                               |   8 +
 arch/Kconfig                              |   6 +
 arch/x86/Kconfig                          |   1 +
 arch/x86/Makefile                         |   6 +-
 arch/x86/boot/Makefile                    |   3 +-
 arch/x86/boot/compressed/Makefile         |   3 +-
 arch/x86/crypto/aesni-intel_asm.S         |  21 +
 arch/x86/crypto/crc32c-pcl-intel-asm_64.S |   8 +-
 arch/x86/crypto/ghash-clmulni-intel_asm.S |   5 +
 arch/x86/entry/thunk_64.S                 |   4 +
 arch/x86/entry/vdso/Makefile              |   5 +-
 arch/x86/include/asm/frame.h              |  51 +-
 arch/x86/include/asm/paravirt.h           |   9 +-
 arch/x86/include/asm/paravirt_types.h     |  18 +-
 arch/x86/include/asm/stackvalidate.h      |  45 ++
 arch/x86/include/asm/xen/hypercall.h      |   5 +-
 arch/x86/kernel/acpi/wakeup_64.S          |   3 +
 arch/x86/kernel/cpu/amd.c                 |   5 +-
 arch/x86/kernel/reboot.c                  |   7 +-
 arch/x86/kernel/vmlinux.lds.S             |   5 +-
 arch/x86/lib/rwsem.S                      |  11 +-
 arch/x86/platform/efi/efi_stub_64.S       |   3 +
 arch/x86/power/hibernate_asm_64.S         |   7 +
 arch/x86/purgatory/Makefile               |   2 +
 arch/x86/realmode/Makefile                |   4 +-
 arch/x86/realmode/rm/Makefile             |   3 +-
 arch/x86/xen/enlighten.c                  |   4 +-
 drivers/firmware/efi/libstub/Makefile     |   1 +
 include/linux/stackvalidate.h             |  28 +
 lib/Kconfig.debug                         |  11 +
 scripts/Makefile                          |   1 +
 scripts/Makefile.build                    |  37 +-
 scripts/mod/Makefile                      |   2 +
 scripts/stackvalidate/Makefile            |  24 +
 scripts/stackvalidate/arch-x86.c          | 160 +++++
 scripts/stackvalidate/arch.h              |  44 ++
 scripts/stackvalidate/elf.c               | 427 +++++++++++++
 scripts/stackvalidate/elf.h               |  92 +++
 scripts/stackvalidate/list.h              | 217 +++++++
 scripts/stackvalidate/special.c           | 199 ++++++
 scripts/stackvalidate/special.h           |  42 ++
 scripts/stackvalidate/stackvalidate.c     | 976 ++++++++++++++++++++++++++++++
 43 files changed, 2658 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/stack-validation.txt
 create mode 100644 arch/x86/include/asm/stackvalidate.h
 create mode 100644 include/linux/stackvalidate.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/special.c
 create mode 100644 scripts/stackvalidate/special.h
 create mode 100644 scripts/stackvalidate/stackvalidate.c

-- 
2.4.3

[PATCH v10 01/20] x86/asm: Frame pointer macro cleanup

[Josh Poimboeuf]

The asm macros for setting up and restoring the frame pointer aren't
currently being used.  However, they will be needed soon to help asm
functions to comply with stackvalidate.

Rename FRAME/ENDFRAME to FRAME_BEGIN/FRAME_END for more symmetry.  Also
make the code more readable and improve the comments.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/frame.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/frame.h b/arch/x86/include/asm/frame.h
index 793179c..8a6cd26 100644
--- a/arch/x86/include/asm/frame.h
+++ b/arch/x86/include/asm/frame.h
@@ -1,23 +1,26 @@
+#ifndef _ASM_X86_FRAME_H
+#define _ASM_X86_FRAME_H
+
 #ifdef __ASSEMBLY__
 
 #include <asm/asm.h>
 
-/* The annotation hides the frame from the unwinder and makes it look
-   like a ordinary ebp save/restore. This avoids some special cases for
-   frame pointer later */
+/*
+ * These are stack frame creation macros.  They should be used by every
+ * callable non-leaf asm function to make kernel stack traces more reliable.
+ */
+.macro FRAME_BEGIN
+#ifdef CONFIG_FRAME_POINTER
+	push %_ASM_BP
+	_ASM_MOV %_ASM_SP, %_ASM_BP
+#endif
+.endm
+
+.macro FRAME_END
 #ifdef CONFIG_FRAME_POINTER
-	.macro FRAME
-	__ASM_SIZE(push,)	%__ASM_REG(bp)
-	__ASM_SIZE(mov)		%__ASM_REG(sp), %__ASM_REG(bp)
-	.endm
-	.macro ENDFRAME
-	__ASM_SIZE(pop,)	%__ASM_REG(bp)
-	.endm
-#else
-	.macro FRAME
-	.endm
-	.macro ENDFRAME
-	.endm
+	pop %_ASM_BP
 #endif
+.endm
 
 #endif  /*  __ASSEMBLY__  */
+#endif /* _ASM_X86_FRAME_H */
-- 
2.4.3

[PATCH v10 02/20] x86/asm: Add C versions of frame pointer macros

[Josh Poimboeuf]

Add C versions of the frame pointer macros which can be used to create a
stack frame in inline assembly.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/frame.h | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/frame.h b/arch/x86/include/asm/frame.h
index 8a6cd26..9a30ec7 100644
--- a/arch/x86/include/asm/frame.h
+++ b/arch/x86/include/asm/frame.h
@@ -1,10 +1,10 @@
 #ifndef _ASM_X86_FRAME_H
 #define _ASM_X86_FRAME_H
 
-#ifdef __ASSEMBLY__
-
 #include <asm/asm.h>
 
+#ifdef __ASSEMBLY__
+
 /*
  * These are stack frame creation macros.  They should be used by every
  * callable non-leaf asm function to make kernel stack traces more reliable.
@@ -22,5 +22,21 @@
 #endif
 .endm
 
+#else /* !__ASSEMBLY__ */
+
+#ifdef CONFIG_FRAME_POINTER
+
+#define FRAME_BEGIN				\
+	"push %" _ASM_BP "\n"			\
+	_ASM_MOV "%" _ASM_SP ", %" _ASM_BP "\n"
+
+#define FRAME_END "pop %" _ASM_BP "\n"
+
+#else /* !CONFIG_FRAME_POINTER */
+
+#define FRAME_BEGIN ""
+#define FRAME_END ""
+
+#endif /* CONFIG_FRAME_POINTER */
 #endif  /*  __ASSEMBLY__  */
 #endif /* _ASM_X86_FRAME_H */
-- 
2.4.3

[PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Josh Poimboeuf]

This adds a CONFIG_STACK_VALIDATION option which enables a host tool
named stackvalidate which runs at compile time.  It analyzes every .o
file and ensures the validity of its stack metadata.  It enforces a set
of rules on asm code and C inline assembly code so that stack traces can
be reliable.

Currently it checks frame pointer usage.  I also plan to add DWARF CFI
validation for C .o files and CFI generation for asm .o files.

For each function, it recursively follows all possible code paths and
validates the correct frame pointer state at each instruction.

It also follows code paths involving special sections, like
.altinstructions, __jump_table, and __ex_table, which can add
alternative execution paths to a given instruction (or set of
instructions).  Similarly, it knows how to follow switch statements, for
which gcc sometimes uses jump tables.

To achieve the validation, stackvalidate enforces the following rules:

1. Each callable function must be annotated as such with the ELF
   function type.  In asm code, this is typically done using the
   ENTRY/ENDPROC macros.  If stackvalidate finds a return instruction
   outside of a function, it flags an error since that usually indicates
   callable code which should be annotated accordingly.

2. Conversely, each section of code which is *not* callable should *not*
   be annotated as an ELF function.  The ENDPROC macro shouldn't be used
   in this case.

3. Each callable function which calls another function must have the
   correct frame pointer logic, if required by CONFIG_FRAME_POINTER or
   the architecture's back chain rules.  This can by done in asm code
   with the FRAME_BEGIN/FRAME_END macros.

4. Dynamic jumps and jumps to undefined symbols are only allowed if:

   a) the jump is part of a switch statement; or

   b) the jump matches sibling call semantics and the frame pointer has
      the same value it had on function entry.

5. A callable function may not execute kernel entry/exit instructions.
   The only code which needs such instructions is kernel entry code,
   which shouldn't be in callable functions anyway.

It currently only supports x86_64.  I tried to make the code generic so
that support for other architectures can hopefully be plugged in
relatively easily.

As a first step, CONFIG_STACK_VALIDATION is disabled by default, and all
reported non-compliances result in warnings.  Once we get them all
cleaned up, we can change the default to CONFIG_STACK_VALIDATION=y and
change the warnings to errors to keep the stack metadata clean.

On my Lenovo laptop with a i7-4810MQ 4-core/8-thread CPU, building the
kernel with it enabled adds less than 3 seconds of total build time.  It
hasn't been optimized for performance yet, so there are probably some
opportunities for better build performance.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/stack-validation.txt    | 193 +++++++
 MAINTAINERS                           |   8 +
 arch/Kconfig                          |   6 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 lib/Kconfig.debug                     |  11 +
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  37 +-
 scripts/stackvalidate/Makefile        |  24 +
 scripts/stackvalidate/arch-x86.c      | 160 ++++++
 scripts/stackvalidate/arch.h          |  44 ++
 scripts/stackvalidate/elf.c           | 427 +++++++++++++++
 scripts/stackvalidate/elf.h           |  92 ++++
 scripts/stackvalidate/list.h          | 217 ++++++++
 scripts/stackvalidate/special.c       | 199 +++++++
 scripts/stackvalidate/special.h       |  42 ++
 scripts/stackvalidate/stackvalidate.c | 976 ++++++++++++++++++++++++++++++++++
 17 files changed, 2439 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/stack-validation.txt
 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/special.c
 create mode 100644 scripts/stackvalidate/special.h
 create mode 100644 scripts/stackvalidate/stackvalidate.c

diff --git a/Documentation/stack-validation.txt b/Documentation/stack-validation.txt
new file mode 100644
index 0000000..c3d3d35
--- /dev/null
+++ b/Documentation/stack-validation.txt
@@ -0,0 +1,193 @@
+Compile-time stack validation
+=============================
+
+
+Overview
+--------
+
+The CONFIG_STACK_VALIDATION option enables a host tool named
+stackvalidate which runs at compile time.  It analyzes every .o file and
+ensures the validity of its stack metadata.  It enforces a set of rules
+on asm code and C inline assembly code so that stack traces can be
+reliable.
+
+Currently it only checks frame pointer usage, but there are plans to add
+CFI validation for C files and CFI generation for asm files.
+
+For each function, it recursively follows all possible code paths and
+validates the correct frame pointer state at each instruction.
+
+It also follows code paths involving special sections, like
+.altinstructions, __jump_table, and __ex_table, which can add
+alternative execution paths to a given instruction (or set of
+instructions).  Similarly, it knows how to follow switch statements, for
+which gcc sometimes uses jump tables.
+
+
+Rules
+-----
+
+To achieve the validation, stackvalidate enforces the following rules:
+
+1. Each callable function must be annotated as such with the ELF
+   function type.  In asm code, this is typically done using the
+   ENTRY/ENDPROC macros.  If stackvalidate finds a return instruction
+   outside of a function, it flags an error since that usually indicates
+   callable code which should be annotated accordingly.
+
+2. Conversely, each section of code which is *not* callable should *not*
+   be annotated as an ELF function.  The ENDPROC macro shouldn't be used
+   in this case.
+
+3. Each callable function which calls another function must have the
+   correct frame pointer logic, if required by CONFIG_FRAME_POINTER or
+   the architecture's back chain rules.  This can by done in asm code
+   with the FRAME_BEGIN/FRAME_END macros.
+
+4. Dynamic jumps and jumps to undefined symbols are only allowed if:
+
+   a) the jump is part of a switch statement; or
+
+   b) the jump matches sibling call semantics and the frame pointer has
+      the same value it had on function entry.
+
+5. A callable function may not execute kernel entry/exit instructions.
+   The only code which needs such instructions is kernel entry code,
+   which shouldn't be be in callable functions anyway.
+
+
+Errors in .S files
+------------------
+
+If you're getting an error in a compiled .S file which you don't
+understand, first make sure that the affected code follows the above
+rules.
+
+Here are some examples of common problems and suggestions for how to fix
+them.
+
+
+1. stackvalidate: asm_file.o: func()+0x128: call without frame pointer save/setup
+
+   The func() function made a function call without first saving and/or
+   updating the frame pointer.
+
+   If func() is indeed a callable function, add proper frame pointer
+   logic using the FP_SAVE and FP_RESTORE macros.  Otherwise, remove its
+   ELF function annotation by changing ENDPROC to END.
+
+   If you're getting this error in a .c file, see the "Errors in .c
+   files" section.
+
+
+2. stackvalidate: asm_file.o: .text+0x53: return instruction outside of a callable function
+
+   A return instruction was detected, but stackvalidate couldn't find a
+   way for a callable function to reach the instruction.
+
+   If the return instruction is inside (or reachable from) a callable
+   function, the function needs to be annotated with the ENTRY/ENDPROC
+   macros.
+
+   If you _really_ need a return instruction outside of a function, and
+   are 100% sure that it won't affect stack traces, you can tell
+   stackvalidate to ignore it.  See the "Adding exceptions" section
+   below.
+
+
+3. stackvalidate: asm_file.o: func()+0x9: function has unreachable instruction
+
+   The instruction lives inside of a callable function, but there's no
+   possible control flow path from the beginning of the function to the
+   instruction.
+
+   If the instruction is actually needed, and it's actually in a
+   callable function, ensure that its function is properly annotated
+   with ENTRY/ENDPROC.
+
+   If it's not actually in a callable function (e.g. kernel entry code),
+   change ENDPROC to END.
+
+
+4. stackvalidate: asm_file.o: func(): can't find starting instruction
+   or
+   stackvalidate: asm_file.o: func()+0x11dd: can't decode instruction
+
+   Did you put data in a text section?  If so, that can confuse
+   stackvalidate's instruction decoder.  Move the data to a more
+   appropriate section like .data or .rodata.
+
+
+5. stackvalidate: asm_file.o: func()+0x6: kernel entry/exit from callable instruction
+
+   This is a kernel entry/exit instruction like sysenter or sysret.
+   Such instructions aren't allowed in a callable function, and are most
+   likely part of the kernel entry code.
+
+   If the instruction isn't actually in a callable function, change
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: sibling call from callable instruction with changed frame pointer
+
+   This is a dynamic jump or a jump to an undefined symbol.
+   Stackvalidate assumed it's a sibling call and detected that the frame
+   pointer wasn't first restored to its original state.
+
+   If it's not really a sibling call, you may need to move the
+   destination code to the local file.
+
+   If the instruction is not actually in a callable function (e.g.
+   kernel entry code), change ENDPROC to END.
+
+
+7. stackvalidate: asm_file: func()+0x5c: frame pointer state mismatch
+
+   The instruction's frame pointer state is inconsistent, depending on
+   which execution path was taken to reach the instruction.
+
+   Make sure the function pushes and sets up the frame pointer (for
+   x86_64, this means rbp) at the beginning of the function and pops it
+   at the end of the function.  Also make sure that no other code in the
+   function touches the frame pointer.
+
+
+Errors in .c files
+------------------
+
+If you're getting a stackvalidate error in a compiled .c file, chances
+are the file uses an asm() statement which has a "call" instruction.  An
+asm() statement with a call instruction must declare the use of the
+stack pointer in its output operand.  For example, on x86_64:
+
+   register void *__sp asm("rsp");
+   asm volatile("call func" : "+r" (__sp));
+
+Otherwise the stack frame may not get created before the call.
+
+Another possible cause for errors in C code is if the Makefile removes
+-fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.
+
+Also see the above section for .S file errors for more information what
+the individual error messages mean.
+
+
+
+Adding exceptions
+-----------------
+
+If you _really_ need stackvalidate to ignore something, and are 100%
+sure that it won't affect kernel stack traces, you can tell
+stackvalidate to ignore it:
+
+- To skip validation of an instruction, use the
+  STACKVALIDATE_IGNORE_INSN macro immediately before the instruction.
+
+- To skip validation of a function, use the STACKVALIDATE_IGNORE_FUNC
+  macro.
+
+- To skip validation of a file, add "STACKVALIDATE_filename.o := n" to
+  the Makefile.
+
+- To skip validation of a directory, add "STACKVALIDATE := n" to the
+  Makefile.
diff --git a/MAINTAINERS b/MAINTAINERS
index c1452d1..fb3deb4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9679,6 +9679,14 @@ L:	stable@vger.kernel.org
 S:	Supported
 F:	Documentation/stable_kernel_rules.txt
 
+STACK VALIDATION
+M:	Josh Poimboeuf <jpoimboe@redhat.com>
+S:	Supported
+F:	scripts/stackvalidate/
+F:	include/linux/stackvalidate.h
+F:	arch/x86/include/asm/stackvalidate.h
+F:	Documentation/stack-validation.txt
+
 STAGING SUBSYSTEM
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
diff --git a/arch/Kconfig b/arch/Kconfig
index 021f8f9..ceb4eda 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -525,6 +525,12 @@ config HAVE_COPY_THREAD_TLS
 	  normal C parameter passing, rather than extracting the syscall
 	  argument from pt_regs.
 
+config HAVE_STACK_VALIDATION
+	bool
+	help
+	  Architecture supports the stackvalidate host tool, which adds
+	  compile-time stack metadata validation.
+
 #
 # ABI hall of shame
 #
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f37010f..50877dc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -151,6 +151,7 @@ config X86
 	select VIRT_TO_BUS
 	select X86_DEV_DMA_OPS			if X86_64
 	select X86_FEATURE_NAMES		if PROC_FS
+	select HAVE_STACK_VALIDATION		if X86_64
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 747860c..fb7135b 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -181,9 +181,13 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 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
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ab76b99..1c235c6 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -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 compile-time stack metadata validation"
+	depends on HAVE_STACK_VALIDATION
+	default n
+	help
+	  Add compile-time checks to validate stack metadata, including frame
+	  pointers and back chain pointers.  This helps ensure that runtime
+	  stack traces are more reliable.
+
+	  For more information, see Documentation/stack-validation.txt.
+
 config DEBUG_FORCE_WEAK_PER_CPU
 	bool "Force weak per-cpu definitions"
 	depends on DEBUG_KERNEL
diff --git a/scripts/Makefile b/scripts/Makefile
index 2016a64..c882a91 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -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
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 01df30a..a1c5376 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -241,10 +241,30 @@ cmd_record_mcount =						\
 	fi;
 endif
 
+ifdef CONFIG_STACK_VALIDATION
+
+__stackvalidate_obj = $(objtree)/scripts/stackvalidate/stackvalidate
+
+ifndef CONFIG_FRAME_POINTER
+nofp = --no-frame-pointer
+endif
+
+# Set STACKVALIDATE_foo.o=n to skip stack validation for a file.
+# Set STACKVALIDATE=n to skip stack validation for a directory.
+stackvalidate_obj = $(if $(patsubst n%,, \
+	$(STACKVALIDATE_$(basetarget).o)$(STACKVALIDATE)y), \
+	$(__stackvalidate_obj))
+cmd_stackvalidate = $(if $(patsubst n%,, \
+	$(STACKVALIDATE_$(basetarget).o)$(STACKVALIDATE)y), \
+	$(__stackvalidate_obj) $(nofp) "$(@)";)
+
+endif # CONFIG_STACK_VALIDATION
+
 define rule_cc_o_c
 	$(call echo-cmd,checksrc) $(cmd_checksrc)			  \
 	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
 	$(cmd_modversions)						  \
+	$(cmd_stackvalidate)						  \
 	$(call echo-cmd,record_mcount)					  \
 	$(cmd_record_mcount)						  \
 	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
@@ -253,14 +273,23 @@ define rule_cc_o_c
 	mv -f $(dot-target).tmp $(dot-target).cmd
 endef
 
+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
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(stackvalidate_obj) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 
 # Single-part modules are special since we need to mark them in $(MODVERDIR)
 
-$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
+$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(stackvalidate_obj) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)
@@ -290,8 +319,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_obj) FORCE
+	$(call if_changed_rule,as_o_S)
 
 targets += $(real-objs-y) $(real-objs-m) $(lib-y)
 targets += $(extra-y) $(MAKECMDGOALS) $(always)
diff --git a/scripts/stackvalidate/Makefile b/scripts/stackvalidate/Makefile
new file mode 100644
index 0000000..468c075
--- /dev/null
+++ b/scripts/stackvalidate/Makefile
@@ -0,0 +1,24 @@
+hostprogs-y := stackvalidate
+always := $(hostprogs-y)
+
+stackvalidate-objs := stackvalidate.o elf.o special.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
diff --git a/scripts/stackvalidate/arch-x86.c b/scripts/stackvalidate/arch-x86.c
new file mode 100644
index 0000000..edc43a8
--- /dev/null
+++ b/scripts/stackvalidate/arch-x86.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdio.h>
+
+#define unlikely(cond) (cond)
+#include <asm/insn.h>
+#include <inat.c>
+#include <insn.c>
+#include <stdlib.h>
+
+#include "elf.h"
+#include "arch.h"
+
+static int is_x86_64(struct elf *elf)
+{
+	switch (elf->ehdr.e_machine) {
+	case EM_X86_64:
+		return 1;
+	case EM_386:
+		return 0;
+	default:
+		WARN("unexpected ELF machine type %d", elf->ehdr.e_machine);
+		return -1;
+	}
+}
+
+int arch_decode_instruction(struct elf *elf, struct section *sec,
+			    unsigned long offset, unsigned int maxlen,
+			    unsigned int *len, unsigned char *type,
+			    unsigned long *immediate)
+{
+	struct insn insn;
+	int x86_64;
+	unsigned char op1, op2, ext;
+
+	x86_64 = is_x86_64(elf);
+	if (x86_64 == -1)
+		return -1;
+
+	insn_init(&insn, (void *)(sec->data + offset), maxlen, x86_64);
+	insn_get_length(&insn);
+	insn_get_opcode(&insn);
+	insn_get_modrm(&insn);
+	insn_get_immediate(&insn);
+
+	if (!insn.opcode.got || !insn.modrm.got || !insn.immediate.got) {
+		WARN("%s: can't decode instruction",
+		     offstr(sec, offset));
+		return -1;
+	}
+
+	*len = insn.length;
+	*type = INSN_OTHER;
+
+	if (insn.vex_prefix.nbytes)
+		return 0;
+
+	op1 = insn.opcode.bytes[0];
+	op2 = insn.opcode.bytes[1];
+
+	switch (op1) {
+	case 0x55:
+		if (!insn.rex_prefix.nbytes)
+			/* push rbp */
+			*type = INSN_FP_SAVE;
+		break;
+
+	case 0x5d:
+		if (!insn.rex_prefix.nbytes)
+			/* pop rbp */
+			*type = INSN_FP_RESTORE;
+		break;
+
+	case 0x70 ... 0x7f:
+		*type = INSN_JUMP_CONDITIONAL;
+		break;
+
+	case 0x89:
+		if (insn.rex_prefix.nbytes == 1 &&
+		    insn.rex_prefix.bytes[0] == 0x48 &&
+		    insn.modrm.nbytes && insn.modrm.bytes[0] == 0xe5)
+			/* mov rsp, rbp */
+			*type = INSN_FP_SETUP;
+		break;
+
+	case 0x90:
+		*type = INSN_NOP;
+		break;
+
+	case 0x0f:
+		if (op2 >= 0x80 && op2 <= 0x8f)
+			*type = INSN_JUMP_CONDITIONAL;
+		else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 ||
+			 op2 == 0x35)
+			/* sysenter, sysret */
+			*type = INSN_CONTEXT_SWITCH;
+		else if (op2 == 0x0b || op2 == 0xb9)
+			/* ud2 */
+			*type = INSN_BUG;
+		else if (op2 == 0x0d || op2 == 0x1f)
+			/* nopl/nopw */
+			*type = INSN_NOP;
+		break;
+
+	case 0xc9: /* leave */
+		*type = INSN_FP_RESTORE;
+		break;
+
+	case 0xe3: /* jecxz/jrcxz */
+		*type = INSN_JUMP_CONDITIONAL;
+		break;
+
+	case 0xe9:
+	case 0xeb:
+		*type = INSN_JUMP_UNCONDITIONAL;
+		break;
+
+	case 0xc2:
+	case 0xc3:
+		*type = INSN_RETURN;
+		break;
+
+	case 0xc5: /* iret */
+	case 0xca: /* retf */
+	case 0xcb: /* retf */
+		*type = INSN_CONTEXT_SWITCH;
+		break;
+
+	case 0xe8:
+		*type = INSN_CALL;
+		break;
+
+	case 0xff:
+		ext = X86_MODRM_REG(insn.modrm.bytes[0]);
+		if (ext == 2 || ext == 3)
+			*type = INSN_CALL_DYNAMIC;
+		else if (ext == 4 || ext == 5)
+			*type = INSN_JUMP_DYNAMIC;
+		break;
+	}
+
+	*immediate = insn.immediate.nbytes ? insn.immediate.value : 0;
+
+	return 0;
+}
diff --git a/scripts/stackvalidate/arch.h b/scripts/stackvalidate/arch.h
new file mode 100644
index 0000000..f7350fc
--- /dev/null
+++ b/scripts/stackvalidate/arch.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _ARCH_H
+#define _ARCH_H
+
+#include <stdbool.h>
+#include "elf.h"
+
+#define INSN_FP_SAVE		1
+#define INSN_FP_SETUP		2
+#define INSN_FP_RESTORE		3
+#define INSN_JUMP_CONDITIONAL	4
+#define INSN_JUMP_UNCONDITIONAL	5
+#define INSN_JUMP_DYNAMIC	6
+#define INSN_CALL		7
+#define INSN_CALL_DYNAMIC	8
+#define INSN_RETURN		9
+#define INSN_CONTEXT_SWITCH	10
+#define INSN_BUG		11
+#define INSN_NOP		12
+#define INSN_OTHER		13
+#define INSN_LAST		INSN_OTHER
+
+int arch_decode_instruction(struct elf *elf, struct section *sec,
+			    unsigned long offset, unsigned int maxlen,
+			    unsigned int *len, unsigned char *type,
+			    unsigned long *displacement);
+
+#endif /* _ARCH_H */
diff --git a/scripts/stackvalidate/elf.c b/scripts/stackvalidate/elf.c
new file mode 100644
index 0000000..29c3f29
--- /dev/null
+++ b/scripts/stackvalidate/elf.c
@@ -0,0 +1,427 @@
+/*
+ * elf.c - ELF access library
+ *
+ * Adapted from kpatch (https://github.com/dynup/kpatch):
+ * Copyright (C) 2013-2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ * Copyright (C) 2014 Seth Jennings <sjenning@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "elf.h"
+
+char *offstr(struct section *sec, unsigned long offset)
+{
+	struct symbol *func;
+	char *name, *str;
+	unsigned long name_off;
+
+	func = find_containing_func(sec, offset);
+	if (func) {
+		name = func->name;
+		name_off = offset - func->offset;
+	} else {
+		name = sec->name;
+		name_off = offset;
+	}
+
+	str = malloc(strlen(name) + 20);
+
+	if (func)
+		sprintf(str, "%s()+0x%lx", name, name_off);
+	else
+		sprintf(str, "%s+0x%lx", name, name_off);
+
+	return str;
+}
+
+struct section *find_section_by_name(struct elf *elf, const char *name)
+{
+	struct section *sec;
+
+	list_for_each_entry(sec, &elf->sections, list)
+		if (!strcmp(sec->name, name))
+			return sec;
+
+	return NULL;
+}
+
+static struct section *find_section_by_index(struct elf *elf,
+					     unsigned int index)
+{
+	struct section *sec;
+
+	list_for_each_entry(sec, &elf->sections, list)
+		if (sec->index == index)
+			return sec;
+
+	return NULL;
+}
+
+static struct symbol *find_symbol_by_index(struct elf *elf, unsigned int index)
+{
+	struct section *sec;
+	struct symbol *sym;
+
+	list_for_each_entry(sec, &elf->sections, list)
+		list_for_each_entry(sym, &sec->symbols, list)
+			if (sym->index == index)
+				return sym;
+
+	return NULL;
+}
+
+struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset)
+{
+	struct symbol *sym;
+
+	list_for_each_entry(sym, &sec->symbols, list)
+		if (sym->type != STT_SECTION &&
+		    sym->offset == offset)
+			return sym;
+
+	return NULL;
+}
+
+struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
+				     unsigned int len)
+{
+	struct rela *rela;
+
+	if (!sec->rela)
+		return NULL;
+
+	list_for_each_entry(rela, &sec->rela->relas, list)
+		if (rela->offset >= offset && rela->offset < offset + len)
+			return rela;
+
+	return NULL;
+}
+
+struct rela *find_rela_by_dest(struct section *sec, unsigned long offset)
+{
+	return find_rela_by_dest_range(sec, offset, 1);
+}
+
+struct symbol *find_containing_func(struct section *sec, unsigned long offset)
+{
+	struct symbol *func;
+
+	list_for_each_entry(func, &sec->symbols, list)
+		if (func->type == STT_FUNC && offset >= func->offset &&
+		    offset < func->offset + func->len)
+			return func;
+
+	return NULL;
+}
+
+static int read_sections(struct elf *elf)
+{
+	Elf_Scn *s = NULL;
+	struct section *sec;
+	size_t shstrndx, sections_nr;
+	int i;
+
+	if (elf_getshdrnum(elf->elf, &sections_nr)) {
+		perror("elf_getshdrnum");
+		return -1;
+	}
+
+	if (elf_getshdrstrndx(elf->elf, &shstrndx)) {
+		perror("elf_getshdrstrndx");
+		return -1;
+	}
+
+	for (i = 0; i < sections_nr; i++) {
+		sec = malloc(sizeof(*sec));
+		if (!sec) {
+			perror("malloc");
+			return -1;
+		}
+		memset(sec, 0, sizeof(*sec));
+
+		INIT_LIST_HEAD(&sec->symbols);
+		INIT_LIST_HEAD(&sec->relas);
+
+		list_add_tail(&sec->list, &elf->sections);
+
+		s = elf_getscn(elf->elf, i);
+		if (!s) {
+			perror("elf_getscn");
+			return -1;
+		}
+
+		sec->index = elf_ndxscn(s);
+
+		if (!gelf_getshdr(s, &sec->sh)) {
+			perror("gelf_getshdr");
+			return -1;
+		}
+
+		sec->name = elf_strptr(elf->elf, shstrndx, sec->sh.sh_name);
+		if (!sec->name) {
+			perror("elf_strptr");
+			return -1;
+		}
+
+		sec->elf_data = elf_getdata(s, NULL);
+		if (!sec->elf_data) {
+			perror("elf_getdata");
+			return -1;
+		}
+
+		if (sec->elf_data->d_off != 0 ||
+		    sec->elf_data->d_size != sec->sh.sh_size) {
+			WARN("unexpected data attributes for %s", sec->name);
+			return -1;
+		}
+
+		sec->data = (unsigned long)sec->elf_data->d_buf;
+		sec->len = sec->elf_data->d_size;
+	}
+
+	/* sanity check, one more call to elf_nextscn() should return NULL */
+	if (elf_nextscn(elf->elf, s)) {
+		WARN("section entry mismatch");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int read_symbols(struct elf *elf)
+{
+	struct section *symtab;
+	struct symbol *sym;
+	struct list_head *entry, *tmp;
+	int symbols_nr, i;
+
+	symtab = find_section_by_name(elf, ".symtab");
+	if (!symtab) {
+		WARN("missing symbol table");
+		return -1;
+	}
+
+	symbols_nr = symtab->sh.sh_size / symtab->sh.sh_entsize;
+
+	for (i = 0; i < symbols_nr; i++) {
+		sym = malloc(sizeof(*sym));
+		if (!sym) {
+			perror("malloc");
+			return -1;
+		}
+		memset(sym, 0, sizeof(*sym));
+
+		sym->index = i;
+
+		if (!gelf_getsym(symtab->elf_data, i, &sym->sym)) {
+			perror("gelf_getsym");
+			goto err;
+		}
+
+		sym->name = elf_strptr(elf->elf, symtab->sh.sh_link,
+				       sym->sym.st_name);
+		if (!sym->name) {
+			perror("elf_strptr");
+			goto err;
+		}
+
+		sym->type = GELF_ST_TYPE(sym->sym.st_info);
+		sym->bind = GELF_ST_BIND(sym->sym.st_info);
+
+		if (sym->sym.st_shndx > SHN_UNDEF &&
+		    sym->sym.st_shndx < SHN_LORESERVE) {
+			sym->sec = find_section_by_index(elf,
+							 sym->sym.st_shndx);
+			if (!sym->sec) {
+				WARN("couldn't find section for symbol %s",
+				     sym->name);
+				goto err;
+			}
+			if (sym->type == STT_SECTION) {
+				sym->name = sym->sec->name;
+				sym->sec->sym = sym;
+			}
+		} else
+			sym->sec = find_section_by_index(elf, 0);
+
+		sym->offset = sym->sym.st_value;
+		sym->len = sym->sym.st_size;
+
+		/* sorted insert into a per-section list */
+		entry = &sym->sec->symbols;
+		list_for_each_prev(tmp, &sym->sec->symbols) {
+			struct symbol *s;
+
+			s = list_entry(tmp, struct symbol, list);
+
+			if (sym->offset > s->offset) {
+				entry = tmp;
+				break;
+			}
+
+			if (sym->offset == s->offset && sym->len >= s->len) {
+				entry = tmp;
+				break;
+			}
+		}
+		list_add(&sym->list, entry);
+	}
+
+	return 0;
+
+err:
+	free(sym);
+	return -1;
+}
+
+static int read_relas(struct elf *elf)
+{
+	struct section *sec;
+	struct rela *rela;
+	int i;
+	unsigned int symndx;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		if (sec->sh.sh_type != SHT_RELA)
+			continue;
+
+		sec->base = find_section_by_name(elf, sec->name + 5);
+		if (!sec->base) {
+			WARN("can't find base section for rela section %s",
+			     sec->name);
+			return -1;
+		}
+
+		sec->base->rela = sec;
+
+		for (i = 0; i < sec->sh.sh_size / sec->sh.sh_entsize; i++) {
+			rela = malloc(sizeof(*rela));
+			if (!rela) {
+				perror("malloc");
+				return -1;
+			}
+			memset(rela, 0, sizeof(*rela));
+
+			list_add_tail(&rela->list, &sec->relas);
+
+			if (!gelf_getrela(sec->elf_data, i, &rela->rela)) {
+				perror("gelf_getrela");
+				return -1;
+			}
+
+			rela->type = GELF_R_TYPE(rela->rela.r_info);
+			rela->addend = rela->rela.r_addend;
+			rela->offset = rela->rela.r_offset;
+			symndx = GELF_R_SYM(rela->rela.r_info);
+			rela->sym = find_symbol_by_index(elf, symndx);
+			if (!rela->sym) {
+				WARN("can't find rela entry symbol %d for %s",
+				     symndx, sec->name);
+				return -1;
+			}
+		}
+	}
+
+	return 0;
+}
+
+struct elf *elf_open(const char *name)
+{
+	struct elf *elf;
+
+	elf_version(EV_CURRENT);
+
+	elf = malloc(sizeof(*elf));
+	if (!elf) {
+		perror("malloc");
+		return NULL;
+	}
+	memset(elf, 0, sizeof(*elf));
+
+	INIT_LIST_HEAD(&elf->sections);
+
+	elf->name = strdup(name);
+	if (!elf->name) {
+		perror("strdup");
+		goto err;
+	}
+
+	elf->fd = open(name, O_RDONLY);
+	if (elf->fd == -1) {
+		perror("open");
+		goto err;
+	}
+
+	elf->elf = elf_begin(elf->fd, ELF_C_READ_MMAP, NULL);
+	if (!elf->elf) {
+		perror("elf_begin");
+		goto err;
+	}
+
+	if (!gelf_getehdr(elf->elf, &elf->ehdr)) {
+		perror("gelf_getehdr");
+		goto err;
+	}
+
+	if (read_sections(elf))
+		goto err;
+
+	if (read_symbols(elf))
+		goto err;
+
+	if (read_relas(elf))
+		goto err;
+
+	return elf;
+
+err:
+	elf_close(elf);
+	return NULL;
+}
+
+void elf_close(struct elf *elf)
+{
+	struct section *sec, *tmpsec;
+	struct symbol *sym, *tmpsym;
+	struct rela *rela, *tmprela;
+
+	list_for_each_entry_safe(sec, tmpsec, &elf->sections, list) {
+		list_for_each_entry_safe(sym, tmpsym, &sec->symbols, list) {
+			list_del(&sym->list);
+			free(sym);
+		}
+		list_for_each_entry_safe(rela, tmprela, &sec->relas, list) {
+			list_del(&rela->list);
+			free(rela);
+		}
+		list_del(&sec->list);
+		free(sec);
+	}
+	if (elf->name)
+		free(elf->name);
+	if (elf->fd > 0)
+		close(elf->fd);
+	if (elf->elf)
+		elf_end(elf->elf);
+	free(elf);
+}
diff --git a/scripts/stackvalidate/elf.h b/scripts/stackvalidate/elf.h
new file mode 100644
index 0000000..feb1f38
--- /dev/null
+++ b/scripts/stackvalidate/elf.h
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _STACKVALIDATE_ELF_H
+#define _STACKVALIDATE_ELF_H
+
+#include <stdio.h>
+#include <gelf.h>
+#include "list.h"
+
+#define WARN(format, ...)				\
+	fprintf(stderr,					\
+		"stackvalidate: %s: " format "\n",	\
+		elf->name, ##__VA_ARGS__)
+
+#define WARN_FUNC(format, sec, offset, ...)		\
+({							\
+	char *_str = offstr(sec, offset);		\
+	WARN("%s: " format, _str, ##__VA_ARGS__);	\
+	free(_str);					\
+})
+
+struct section {
+	struct list_head list;
+	GElf_Shdr sh;
+	struct list_head symbols;
+	struct list_head relas;
+	struct section *base, *rela;
+	struct symbol *sym;
+	Elf_Data *elf_data;
+	char *name;
+	int index;
+	unsigned long data;
+	unsigned int len;
+};
+
+struct symbol {
+	struct list_head list;
+	GElf_Sym sym;
+	struct section *sec;
+	char *name;
+	int index;
+	unsigned char bind, type;
+	unsigned long offset;
+	unsigned int len;
+};
+
+struct rela {
+	struct list_head list;
+	GElf_Rela rela;
+	struct symbol *sym;
+	unsigned int type;
+	int offset;
+	int addend;
+};
+
+struct elf {
+	Elf *elf;
+	GElf_Ehdr ehdr;
+	int fd;
+	char *name;
+	struct list_head sections;
+};
+
+
+struct elf *elf_open(const char *name);
+struct section *find_section_by_name(struct elf *elf, const char *name);
+struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
+struct rela *find_rela_by_dest(struct section *sec, unsigned long offset);
+struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
+				     unsigned int len);
+struct symbol *find_containing_func(struct section *sec, unsigned long offset);
+char *offstr(struct section *sec, unsigned long offset);
+void elf_close(struct elf *elf);
+
+
+
+#endif /* _STACKVALIDATE_ELF_H */
diff --git a/scripts/stackvalidate/list.h b/scripts/stackvalidate/list.h
new file mode 100644
index 0000000..25716b5
--- /dev/null
+++ b/scripts/stackvalidate/list.h
@@ -0,0 +1,217 @@
+#ifndef _LIST_H
+#define _LIST_H
+
+#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+
+#define container_of(ptr, type, member) ({ \
+	const typeof(((type *)0)->member) *__mptr = (ptr); \
+	(type *)((char *)__mptr - offsetof(type, member)); })
+
+#define LIST_POISON1 ((void *) 0x00100100)
+#define LIST_POISON2 ((void *) 0x00200200)
+
+struct list_head {
+	struct list_head *next, *prev;
+};
+
+#define LIST_HEAD_INIT(name) { &(name), &(name) }
+
+#define LIST_HEAD(name) \
+	struct list_head name = LIST_HEAD_INIT(name)
+
+static inline void INIT_LIST_HEAD(struct list_head *list)
+{
+	list->next = list;
+	list->prev = list;
+}
+
+static inline void __list_add(struct list_head *new,
+			      struct list_head *prev,
+			      struct list_head *next)
+{
+	next->prev = new;
+	new->next = next;
+	new->prev = prev;
+	prev->next = new;
+}
+
+static inline void list_add(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head, head->next);
+}
+
+static inline void list_add_tail(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head->prev, head);
+}
+
+static inline void __list_del(struct list_head *prev, struct list_head *next)
+{
+	next->prev = prev;
+	prev->next = next;
+}
+
+static inline void __list_del_entry(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+}
+
+static inline void list_del(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+	entry->next = LIST_POISON1;
+	entry->prev = LIST_POISON2;
+}
+
+static inline void list_replace(struct list_head *old,
+				struct list_head *new)
+{
+	new->next = old->next;
+	new->next->prev = new;
+	new->prev = old->prev;
+	new->prev->next = new;
+}
+
+static inline void list_replace_init(struct list_head *old,
+					struct list_head *new)
+{
+	list_replace(old, new);
+	INIT_LIST_HEAD(old);
+}
+
+static inline void list_del_init(struct list_head *entry)
+{
+	__list_del_entry(entry);
+	INIT_LIST_HEAD(entry);
+}
+
+static inline void list_move(struct list_head *list, struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add(list, head);
+}
+
+static inline void list_move_tail(struct list_head *list,
+				  struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add_tail(list, head);
+}
+
+static inline int list_is_last(const struct list_head *list,
+				const struct list_head *head)
+{
+	return list->next == head;
+}
+
+static inline int list_empty(const struct list_head *head)
+{
+	return head->next == head;
+}
+
+static inline int list_empty_careful(const struct list_head *head)
+{
+	struct list_head *next = head->next;
+
+	return (next == head) && (next == head->prev);
+}
+
+static inline void list_rotate_left(struct list_head *head)
+{
+	struct list_head *first;
+
+	if (!list_empty(head)) {
+		first = head->next;
+		list_move_tail(first, head);
+	}
+}
+
+static inline int list_is_singular(const struct list_head *head)
+{
+	return !list_empty(head) && (head->next == head->prev);
+}
+
+#define list_entry(ptr, type, member) \
+	container_of(ptr, type, member)
+
+#define list_first_entry(ptr, type, member) \
+	list_entry((ptr)->next, type, member)
+
+#define list_last_entry(ptr, type, member) \
+	list_entry((ptr)->prev, type, member)
+
+#define list_first_entry_or_null(ptr, type, member) \
+	(!list_empty(ptr) ? list_first_entry(ptr, type, member) : NULL)
+
+#define list_next_entry(pos, member) \
+	list_entry((pos)->member.next, typeof(*(pos)), member)
+
+#define list_prev_entry(pos, member) \
+	list_entry((pos)->member.prev, typeof(*(pos)), member)
+
+#define list_for_each(pos, head) \
+	for (pos = (head)->next; pos != (head); pos = pos->next)
+
+#define list_for_each_prev(pos, head) \
+	for (pos = (head)->prev; pos != (head); pos = pos->prev)
+
+#define list_for_each_safe(pos, n, head) \
+	for (pos = (head)->next, n = pos->next; pos != (head); \
+		pos = n, n = pos->next)
+
+#define list_for_each_prev_safe(pos, n, head) \
+	for (pos = (head)->prev, n = pos->prev; \
+	     pos != (head); \
+	     pos = n, n = pos->prev)
+
+#define list_for_each_entry(pos, head, member)				\
+	for (pos = list_first_entry(head, typeof(*pos), member);	\
+	     &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_reverse(pos, head, member)			\
+	for (pos = list_last_entry(head, typeof(*pos), member);		\
+	     &pos->member != (head);					\
+	     pos = list_prev_entry(pos, member))
+
+#define list_prepare_entry(pos, head, member) \
+	((pos) ? : list_entry(head, typeof(*pos), member))
+
+#define list_for_each_entry_continue(pos, head, member)			\
+	for (pos = list_next_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_continue_reverse(pos, head, member)		\
+	for (pos = list_prev_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = list_prev_entry(pos, member))
+
+#define list_for_each_entry_from(pos, head, member)			\
+	for (; &pos->member != (head);					\
+	     pos = list_next_entry(pos, member))
+
+#define list_for_each_entry_safe(pos, n, head, member)			\
+	for (pos = list_first_entry(head, typeof(*pos), member),	\
+		n = list_next_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_continue(pos, n, head, member)		\
+	for (pos = list_next_entry(pos, member),			\
+		n = list_next_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_from(pos, n, head, member)		\
+	for (n = list_next_entry(pos, member);				\
+	     &pos->member != (head);					\
+	     pos = n, n = list_next_entry(n, member))
+
+#define list_for_each_entry_safe_reverse(pos, n, head, member)		\
+	for (pos = list_last_entry(head, typeof(*pos), member),		\
+		n = list_prev_entry(pos, member);			\
+	     &pos->member != (head);					\
+	     pos = n, n = list_prev_entry(n, member))
+
+#endif /* _LIST_H */
diff --git a/scripts/stackvalidate/special.c b/scripts/stackvalidate/special.c
new file mode 100644
index 0000000..3882839
--- /dev/null
+++ b/scripts/stackvalidate/special.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * This file reads all the special sections which have alternate instructions
+ * which can be patched in or redirected to at runtime.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include "special.h"
+
+#define EX_ENTRY_SIZE		8
+#define EX_ORIG_OFFSET		0
+#define EX_NEW_OFFSET		4
+
+#define JUMP_ENTRY_SIZE		24
+#define JUMP_ORIG_OFFSET	0
+#define JUMP_NEW_OFFSET		8
+
+#define ALT_ENTRY_SIZE		13
+#define ALT_ORIG_OFFSET		0
+#define ALT_NEW_OFFSET		4
+#define ALT_FEATURE_OFFSET	8
+#define ALT_ORIG_LEN_OFFSET	10
+#define ALT_NEW_LEN_OFFSET	11
+
+#define X86_FEATURE_POPCNT 4*32+23
+
+struct special_entry {
+	char *sec;
+	bool group, jump_or_nop;
+	unsigned char size, orig, new;
+	unsigned char orig_len, new_len; /* group only */
+	unsigned char feature; /* ALTERNATIVE macro CPU feature */
+};
+
+struct special_entry entries[] = {
+	{
+		.sec = ".altinstructions",
+		.group = true,
+		.size = ALT_ENTRY_SIZE,
+		.orig = ALT_ORIG_OFFSET,
+		.orig_len = ALT_ORIG_LEN_OFFSET,
+		.new = ALT_NEW_OFFSET,
+		.new_len = ALT_NEW_LEN_OFFSET,
+		.feature = ALT_FEATURE_OFFSET,
+	},
+	{
+		.sec = "__jump_table",
+		.jump_or_nop = true,
+		.size = JUMP_ENTRY_SIZE,
+		.orig = JUMP_ORIG_OFFSET,
+		.new = JUMP_NEW_OFFSET,
+	},
+	{
+		.sec = "__ex_table",
+		.size = EX_ENTRY_SIZE,
+		.orig = EX_ORIG_OFFSET,
+		.new = EX_NEW_OFFSET,
+	},
+	{},
+};
+
+static int get_alt_entry(struct elf *elf, struct special_entry *entry,
+			 struct section *sec, int index,
+			 struct special_alt *alt)
+{
+	struct rela *orig_rela, *new_rela;
+	unsigned long offset;
+
+	offset = index * entry->size;
+
+	alt->group = entry->group;
+	alt->jump_or_nop = entry->jump_or_nop;
+
+	if (alt->group) {
+		alt->orig_len = *(unsigned char *)(sec->data + offset +
+						   entry->orig_len);
+		alt->new_len = *(unsigned char *)(sec->data + offset +
+						  entry->new_len);
+	}
+
+	if (entry->feature) {
+		unsigned short feature;
+
+		feature = *(unsigned short *)(sec->data + offset +
+					      entry->feature);
+
+		/*
+		 * It has been requested that we don't validate the !POPCNT
+		 * feature path which is a "very very small percentage of
+		 * machines".
+		 */
+		if (feature == X86_FEATURE_POPCNT)
+			alt->skip_orig = true;
+	}
+
+	orig_rela = find_rela_by_dest(sec, offset + entry->orig);
+	if (!orig_rela) {
+		WARN("%s: can't find orig rela",
+		     offstr(sec, offset + entry->orig));
+		return -1;
+	}
+	if (orig_rela->sym->type != STT_SECTION) {
+		WARN("%s: don't know how to handle non-section rela symbol %s",
+		     offstr(sec, offset + entry->orig),
+		     orig_rela->sym->name);
+		return -1;
+	}
+
+	alt->orig_sec = orig_rela->sym->sec;
+	alt->orig_off = orig_rela->addend;
+
+	if (!entry->group || alt->new_len) {
+		new_rela = find_rela_by_dest(sec, offset + entry->new);
+		if (!new_rela) {
+			WARN("%s: can't find new rela",
+			     offstr(sec, offset + entry->new));
+			return -1;
+		}
+		if (new_rela->sym->type != STT_SECTION) {
+			WARN("%s: don't know how to handle non-section rela symbol %s",
+			     offstr(sec, offset + entry->new),
+			     new_rela->sym->name);
+			return -1;
+		}
+
+		alt->new_sec = new_rela->sym->sec;
+		alt->new_off = (unsigned int)new_rela->addend;
+
+		/* _ASM_EXTABLE_EX hack */
+		if (alt->new_off >= 0x7ffffff0)
+			alt->new_off -= 0x7ffffff0;
+	}
+
+	return 0;
+}
+
+/*
+ * Read all the special sections and create a list of special_alt structs which
+ * describe all the alternate instructions which can be patched in or
+ * redirected to at runtime.
+ */
+int special_get_alts(struct elf *elf, struct list_head *alts)
+{
+	struct special_entry *entry;
+	struct section *sec;
+	unsigned int nr_entries;
+	struct special_alt *alt;
+	int index, ret;
+
+	INIT_LIST_HEAD(alts);
+
+	for (entry = entries; entry->sec; entry++) {
+		sec = find_section_by_name(elf, entry->sec);
+		if (!sec)
+			continue;
+
+		if (sec->len % entry->size != 0) {
+			WARN("%s size not a multiple of %d",
+			     sec->name, JUMP_ENTRY_SIZE);
+			return -1;
+		}
+
+		nr_entries = sec->len / entry->size;
+
+		for (index = 0; index < nr_entries; index++) {
+			alt = malloc(sizeof(*alt));
+			if (!alt) {
+				WARN("malloc failed");
+				return -1;
+			}
+			memset(alt, 0, sizeof(*alt));
+
+			ret = get_alt_entry(elf, entry, sec, index, alt);
+			if (ret)
+				return ret;
+
+			list_add_tail(&alt->list, alts);
+		}
+	}
+
+	return 0;
+}
diff --git a/scripts/stackvalidate/special.h b/scripts/stackvalidate/special.h
new file mode 100644
index 0000000..fad1d09
--- /dev/null
+++ b/scripts/stackvalidate/special.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SPECIAL_H
+#define _SPECIAL_H
+
+#include <stdbool.h>
+#include "elf.h"
+
+struct special_alt {
+	struct list_head list;
+
+	bool group;
+	bool skip_orig;
+	bool jump_or_nop;
+
+	struct section *orig_sec;
+	unsigned long orig_off;
+
+	struct section *new_sec;
+	unsigned long new_off;
+
+	unsigned int orig_len, new_len; /* group only */
+};
+
+int special_get_alts(struct elf *elf, struct list_head *alts);
+
+#endif /* _SPECIAL_H */
diff --git a/scripts/stackvalidate/stackvalidate.c b/scripts/stackvalidate/stackvalidate.c
new file mode 100644
index 0000000..de0d977
--- /dev/null
+++ b/scripts/stackvalidate/stackvalidate.c
@@ -0,0 +1,976 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <jpoimboe@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * stackvalidate:
+ *
+ * This tool analyzes every .o file and ensures the validity of its stack trace
+ * metadata.  It enforces a set of rules on asm code and C inline assembly code
+ * so that stack traces can be reliable.
+ *
+ * For more information, see Documentation/stack-validation.txt.
+ */
+
+#include <argp.h>
+#include <stdbool.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "elf.h"
+#include "arch.h"
+#include "special.h"
+
+#define STATE_FP_SAVED		0x1
+#define STATE_FP_SETUP		0x2
+
+int warnings;
+
+struct instruction {
+	struct list_head list;
+	struct section *sec;
+	unsigned long offset;
+	unsigned int len, state;
+	unsigned char type;
+	unsigned long immediate;
+	bool alt_group, visited, ignore;
+	struct symbol *call_dest;
+	struct instruction *jump_dest;
+	struct list_head alts;
+};
+
+struct list_head insns;
+
+struct alternative {
+	struct list_head list;
+	struct instruction *insn;
+};
+
+struct args {
+	char *args[1];
+	bool nofp;
+};
+struct args args;
+static const char args_doc[] = "file.o";
+static struct argp_option options[] = {
+	{"no-frame-pointer", 1, 0, 0, "Don't check frame pointers" },
+	{0},
+};
+static error_t parse_opt(int key, char *arg, struct argp_state *state)
+{
+	/* Get the input argument from argp_parse, which we
+	   know is a pointer to our args structure. */
+	struct args *args = state->input;
+
+	switch (key) {
+	case 1: /* --no-frame-pointer */
+		args->nofp = true;
+		break;
+
+	case ARGP_KEY_ARG:
+		if (state->arg_num >= 1)
+			/* Too many arguments. */
+			argp_usage(state);
+		args->args[state->arg_num] = arg;
+		break;
+
+	case ARGP_KEY_END:
+		if (state->arg_num < 1)
+			/* Not enough arguments. */
+			argp_usage(state);
+		break;
+
+	default:
+		return ARGP_ERR_UNKNOWN;
+	}
+	return 0;
+}
+static struct argp argp = { options, parse_opt, args_doc, 0 };
+
+/*
+ * Check for the STACKVALIDATE_IGNORE_INSN macro.
+ */
+static bool ignore_insn(struct elf *elf, struct instruction *insn)
+{
+	struct section *macro_sec;
+	struct rela *rela;
+
+	macro_sec = find_section_by_name(elf, "__stackvalidate_ignore_insn");
+	if (!macro_sec || !macro_sec->rela)
+		return false;
+
+	list_for_each_entry(rela, &macro_sec->rela->relas, list)
+		if (rela->sym->type == STT_SECTION &&
+		    rela->sym == insn->sec->sym &&
+		    rela->addend == insn->offset)
+			return true;
+
+	return false;
+}
+
+/*
+ * Check for the STACKVALIDATE_IGNORE_FUNC macro.
+ */
+static bool ignore_func(struct elf *elf, struct symbol *func)
+{
+	struct section *macro_sec;
+	struct rela *rela;
+
+	macro_sec = find_section_by_name(elf, "__stackvalidate_ignore_func");
+	if (!macro_sec || !macro_sec->rela)
+		return false;
+
+	list_for_each_entry(rela, &macro_sec->rela->relas, list)
+		if (rela->sym->sec == func->sec &&
+		    rela->addend == func->offset)
+			return true;
+
+
+	return false;
+}
+
+static struct instruction *find_instruction(struct section *sec,
+					    unsigned long offset)
+{
+	struct instruction *insn;
+
+	list_for_each_entry(insn, &insns, list)
+		if (insn->sec == sec && insn->offset == offset)
+			return insn;
+
+	return NULL;
+}
+
+/*
+ * This checks to see if the given function is a "noreturn" function.
+ *
+ * For global functions which are outside the scope of this object file, we
+ * have to keep a manual list of them.
+ *
+ * For local functions, we have to detect them manually by simply looking for
+ * the lack of a return instruction.
+ */
+static bool dead_end_function(struct symbol *func)
+{
+	struct instruction *insn;
+
+	if (func->bind == STB_GLOBAL &&
+	    (!strcmp(func->name, "__stack_chk_fail") ||
+	     !strcmp(func->name, "panic") ||
+	     !strcmp(func->name, "do_exit")))
+		return true;
+
+	if (!func->sec)
+		return false;
+
+	insn = find_instruction(func->sec, func->offset);
+	if (!insn)
+		return false;
+
+	list_for_each_entry_from(insn, &insns, list) {
+		if (insn->sec != func->sec ||
+		    insn->offset >= func->offset + func->len)
+			break;
+
+		if (insn->type == INSN_RETURN)
+			return false;
+
+		if (insn->type == INSN_JUMP_UNCONDITIONAL) {
+			struct instruction *dest = insn->jump_dest;
+			struct symbol *dest_func;
+
+			if (!dest)
+				/* sibling call to another file */
+				return false;
+
+			if (dest->sec != func->sec ||
+			    dest->offset < func->offset ||
+			    dest->offset >= func->offset + func->len) {
+				/* local sibling call */
+				dest_func = find_symbol_by_offset(dest->sec,
+								  dest->offset);
+				if (!dest_func)
+					continue;
+
+				return dead_end_function(dest_func);
+			}
+		}
+
+		if (insn->type == INSN_JUMP_DYNAMIC)
+			/* sibling call */
+			return false;
+	}
+
+	return true;
+}
+
+/*
+ * Call the arch-specific instruction decoder for all the instructions and add
+ * them to the global insns list.
+ */
+static int decode_instructions(struct elf *elf)
+{
+	struct section *sec;
+	unsigned long offset;
+	struct instruction *insn;
+	int ret;
+
+	INIT_LIST_HEAD(&insns);
+
+	list_for_each_entry(sec, &elf->sections, list) {
+
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+			continue;
+
+		for (offset = 0; offset < sec->len; offset += insn->len) {
+			insn = malloc(sizeof(*insn));
+			memset(insn, 0, sizeof(*insn));
+
+			INIT_LIST_HEAD(&insn->alts);
+			insn->sec = sec;
+			insn->offset = offset;
+
+			ret = arch_decode_instruction(elf, sec, offset,
+						      sec->len - offset,
+						      &insn->len, &insn->type,
+						      &insn->immediate);
+			if (ret)
+				return ret;
+
+			if (!insn->type || insn->type > INSN_LAST) {
+				WARN_FUNC("invalid instruction type %d",
+					  insn->sec, insn->offset, insn->type);
+				return -1;
+			}
+
+			list_add_tail(&insn->list, &insns);
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Warnings shouldn't be reported for ignored instructions.  Set insn->ignore
+ * for each ignored instruction and each instruction contained in an ignored
+ * function.
+ */
+static void get_ignores(struct elf *elf)
+{
+	struct instruction *insn;
+	struct section *sec;
+	struct symbol *func;
+
+	list_for_each_entry(insn, &insns, list)
+		insn->ignore = ignore_insn(elf, insn);
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		list_for_each_entry(func, &sec->symbols, list) {
+			if (func->type != STT_FUNC)
+				continue;
+
+			if (!ignore_func(elf, func))
+				continue;
+
+			insn = find_instruction(sec, func->offset);
+			if (!insn)
+				continue;
+
+			list_for_each_entry_from(insn, &insns, list) {
+				if (insn->sec != func->sec ||
+				    insn->offset >= func->offset + func->len)
+					break;
+
+				insn->ignore = true;
+				insn->visited = true;
+			}
+		}
+	}
+}
+
+/*
+ * Find the destination instructions for all jumps.
+ */
+static int get_jump_destinations(struct elf *elf)
+{
+	struct instruction *insn;
+	struct rela *rela;
+	struct section *dest_sec;
+	unsigned long dest_off;
+
+	list_for_each_entry(insn, &insns, list) {
+		if (insn->type != INSN_JUMP_CONDITIONAL &&
+		    insn->type != INSN_JUMP_UNCONDITIONAL)
+			continue;
+
+		rela = find_rela_by_dest_range(insn->sec, insn->offset,
+					       insn->len);
+		if (!rela) {
+			dest_sec = insn->sec;
+			dest_off = insn->offset + insn->len + insn->immediate;
+		} else if (rela->sym->type == STT_SECTION) {
+			dest_sec = rela->sym->sec;
+			dest_off = rela->addend + 4;
+		} else if (rela->sym->sec->index) {
+			dest_sec = rela->sym->sec;
+			dest_off = rela->sym->sym.st_value + rela->addend + 4;
+		} else {
+			/*
+			 * This error (jump to another file) will be handled
+			 * later in validate_functions().
+			 */
+			continue;
+		}
+
+		insn->jump_dest = find_instruction(dest_sec, dest_off);
+		if (!insn->jump_dest && !insn->ignore) {
+
+			/*
+			 * This is a special case where an alt instruction
+			 * jumps past the end of the section.  These are
+			 * handled later.
+			 */
+			if (!strcmp(insn->sec->name, ".altinstr_replacement"))
+				continue;
+
+			WARN_FUNC("can't find jump dest instruction at %s+0x%lx",
+				  insn->sec, insn->offset, dest_sec->name,
+				  dest_off);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Find the destination instructions for all calls.
+ */
+static int get_call_destinations(struct elf *elf)
+{
+	struct instruction *insn;
+	unsigned long dest_off;
+	struct rela *rela;
+
+	list_for_each_entry(insn, &insns, list) {
+		if (insn->type != INSN_CALL)
+			continue;
+
+		rela = find_rela_by_dest_range(insn->sec, insn->offset,
+					       insn->len);
+		if (!rela) {
+			dest_off = insn->offset + insn->len + insn->immediate;
+			insn->call_dest = find_symbol_by_offset(insn->sec,
+								dest_off);
+			if (!insn->call_dest) {
+				WARN_FUNC("can't find call dest symbol at offset 0x%lx",
+					  insn->sec, insn->offset, dest_off);
+				return -1;
+			}
+		} else if (rela->sym->type == STT_SECTION) {
+			insn->call_dest = find_symbol_by_offset(rela->sym->sec,
+								rela->addend+4);
+			if (!insn->call_dest ||
+			    insn->call_dest->type != STT_FUNC) {
+				WARN_FUNC("can't find call dest symbol at %s+0x%x",
+					  insn->sec, insn->offset,
+					  rela->sym->sec->name,
+					  rela->addend + 4);
+				return -1;
+			}
+		} else
+			insn->call_dest = rela->sym;
+	}
+
+	return 0;
+}
+
+/*
+ * The .alternatives section requires some extra special care, over and above
+ * what other special sections require:
+ *
+ * 1. Because alternatives are patched in-place, we need to insert a fake jump
+ *    instruction at the end so that validate_branch() skips all the original
+ *    replaced instructions when validating the new instruction path.
+ *
+ * 2. An added wrinkle is that the new instruction length might be zero.  In
+ *    that case the old instructions are replaced with noops.  We simulate that
+ *    by creating a fake jump as the only new instruction.
+ *
+ * 3. In some cases, the alternative section includes an instruction which
+ *    conditionally jumps to the _end_ of the entry.  We have to modify these
+ *    jumps' destinations to point back to .text rather than the end of the
+ *    entry in .altinstr_replacement.
+ *
+ * 4. It has been requested that we don't validate the !POPCNT feature path
+ *    which is a "very very small percentage of machines".
+ */
+static int handle_group_alt(struct elf *elf, struct special_alt *special_alt,
+			    struct instruction *orig_insn,
+			    struct instruction **new_insn)
+{
+	struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump;
+	unsigned long dest_off;
+
+	last_orig_insn = NULL;
+	insn = orig_insn;
+	list_for_each_entry_from(insn, &insns, list) {
+		if (insn->sec != special_alt->orig_sec ||
+		    insn->offset >= special_alt->orig_off + special_alt->orig_len)
+			break;
+
+		if (special_alt->skip_orig)
+			insn->ignore = true;
+
+		insn->alt_group = true;
+		last_orig_insn = insn;
+	}
+
+	if (list_is_last(&last_orig_insn->list, &insns) ||
+	    list_next_entry(last_orig_insn, list)->sec != special_alt->orig_sec) {
+		WARN("%s: don't know how to handle alternatives at end of section",
+		     special_alt->orig_sec->name);
+		return -1;
+	}
+
+	fake_jump = malloc(sizeof(*fake_jump));
+	if (!fake_jump) {
+		WARN("malloc failed");
+		return -1;
+	}
+	memset(fake_jump, 0, sizeof(*fake_jump));
+	INIT_LIST_HEAD(&fake_jump->alts);
+	fake_jump->sec = special_alt->new_sec;
+	fake_jump->offset = -1;
+	fake_jump->type = INSN_JUMP_UNCONDITIONAL;
+	fake_jump->jump_dest = list_next_entry(last_orig_insn, list);
+
+	if (!special_alt->new_len) {
+		*new_insn = fake_jump;
+		return 0;
+	}
+
+	last_new_insn = NULL;
+	insn = *new_insn;
+	list_for_each_entry_from(insn, &insns, list) {
+		if (insn->sec != special_alt->new_sec ||
+		    insn->offset >= special_alt->new_off + special_alt->new_len)
+			break;
+
+		last_new_insn = insn;
+
+		if (insn->type != INSN_JUMP_CONDITIONAL &&
+		    insn->type != INSN_JUMP_UNCONDITIONAL)
+			continue;
+
+		if (!insn->immediate)
+			continue;
+
+		dest_off = insn->offset + insn->len + insn->immediate;
+		if (dest_off == special_alt->new_off + special_alt->new_len)
+			insn->jump_dest = fake_jump;
+
+		if (!insn->jump_dest) {
+			WARN_FUNC("can't find alternative jump destination",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+	}
+
+	if (!last_new_insn) {
+		WARN_FUNC("can't find last new alternative instruction",
+			  special_alt->new_sec, special_alt->new_off);
+		return -1;
+	}
+
+	list_add(&fake_jump->list, &last_new_insn->list);
+
+	return 0;
+}
+
+/*
+ * A jump table entry can either convert a nop to a jump or a jump to a nop.
+ * If the original instruction is a jump, make the alt entry an effective nop
+ * by just skipping the original instruction.
+ */
+static int handle_jump_alt(struct elf *elf, struct special_alt *special_alt,
+			   struct instruction *orig_insn,
+			   struct instruction **new_insn)
+{
+	if (orig_insn->type == INSN_NOP)
+		return 0;
+
+	if (orig_insn->type != INSN_JUMP_UNCONDITIONAL) {
+		WARN_FUNC("unsupported instruction at jump label",
+			  orig_insn->sec, orig_insn->offset);
+		return -1;
+	}
+
+	*new_insn = list_next_entry(orig_insn, list);
+	return 0;
+}
+
+/*
+ * Read all the special sections which have alternate instructions which can be
+ * patched in or redirected to at runtime.  Each instruction having alternate
+ * instruction(s) has them added to its insn->alts list, which will be
+ * traversed in validate_branch().
+ */
+static int get_special_section_alts(struct elf *elf)
+{
+	struct list_head special_alts;
+	struct instruction *orig_insn, *new_insn;
+	struct special_alt *special_alt, *tmp;
+	struct alternative *alt;
+	int ret;
+
+	ret = special_get_alts(elf, &special_alts);
+	if (ret)
+		return ret;
+
+	list_for_each_entry_safe(special_alt, tmp, &special_alts, list) {
+		alt = malloc(sizeof(*alt));
+		if (!alt) {
+			WARN("malloc failed");
+			ret = -1;
+			goto out;
+		}
+
+		orig_insn = find_instruction(special_alt->orig_sec,
+					     special_alt->orig_off);
+		if (!orig_insn) {
+			WARN_FUNC("special: can't find orig instruction",
+				  special_alt->orig_sec, special_alt->orig_off);
+			ret = -1;
+			goto out;
+		}
+
+		new_insn = NULL;
+		if (!special_alt->group || special_alt->new_len) {
+			new_insn = find_instruction(special_alt->new_sec,
+						    special_alt->new_off);
+			if (!new_insn) {
+				WARN_FUNC("special: can't find new instruction",
+					  special_alt->new_sec,
+					  special_alt->new_off);
+				ret = -1;
+				goto out;
+			}
+		}
+
+		if (special_alt->group) {
+			ret = handle_group_alt(elf, special_alt, orig_insn,
+					       &new_insn);
+			if (ret)
+				goto out;
+		} else if (special_alt->jump_or_nop) {
+			ret = handle_jump_alt(elf, special_alt, orig_insn,
+					      &new_insn);
+			if (ret)
+				goto out;
+		}
+
+		alt->insn = new_insn;
+		list_add_tail(&alt->list, &orig_insn->alts);
+
+		list_del(&special_alt->list);
+		free(special_alt);
+	}
+
+out:
+	return ret;
+}
+
+/*
+ * For some switch statements, gcc generates a jump table in the .rodata
+ * section which contains a list of addresses within the function to jump to.
+ * This finds these jump tables and adds them to the insn->alts lists.
+ */
+static int get_switch_alts(struct elf *elf)
+{
+	struct instruction *insn, *alt_insn;
+	struct rela *rodata_rela, *rela;
+	struct section *rodata;
+	struct symbol *func;
+	struct alternative *alt;
+
+	list_for_each_entry(insn, &insns, list) {
+		if (insn->type != INSN_JUMP_DYNAMIC)
+			continue;
+
+		rodata_rela = find_rela_by_dest_range(insn->sec, insn->offset,
+						      insn->len);
+		if (!rodata_rela || strcmp(rodata_rela->sym->name, ".rodata"))
+			continue;
+
+		rodata = find_section_by_name(elf, ".rodata");
+		if (!rodata || !rodata->rela)
+			continue;
+
+		rela = find_rela_by_dest(rodata, rodata_rela->addend);
+		if (!rela)
+			continue;
+
+		func = find_containing_func(insn->sec, insn->offset);
+		if (!func) {
+			WARN_FUNC("can't find containing func",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+
+		list_for_each_entry_from(rela, &rodata->rela->relas, list) {
+			if (rela->sym->sec != insn->sec ||
+			    rela->addend <= func->offset ||
+			    rela->addend >= func->offset + func->len)
+				break;
+
+			alt_insn = find_instruction(insn->sec, rela->addend);
+			if (!alt_insn) {
+				WARN("%s: can't find instruction at %s+0x%x",
+				     rodata->rela->name, insn->sec->name,
+				     rela->addend);
+				return -1;
+			}
+
+			alt = malloc(sizeof(*alt));
+			if (!alt) {
+				WARN("malloc failed");
+				return -1;
+			}
+
+			alt->insn = alt_insn;
+			list_add_tail(&alt->list, &insn->alts);
+		}
+	}
+
+	return 0;
+}
+
+static int decode_sections(struct elf *elf)
+{
+	int ret;
+
+	ret = decode_instructions(elf);
+	if (ret)
+		return ret;
+
+	get_ignores(elf);
+
+	ret = get_jump_destinations(elf);
+	if (ret)
+		return ret;
+
+	ret = get_call_destinations(elf);
+	if (ret)
+		return ret;
+
+	ret = get_special_section_alts(elf);
+	if (ret)
+		return ret;
+
+	ret = get_switch_alts(elf);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * Follow the branch starting at the given instruction, and recursively follow
+ * any other branches (jumps).  Meanwhile, track the frame pointer state at
+ * each instruction and validate all the rules described in
+ * Documentation/stack-validation.txt.
+ */
+static int validate_branch(struct elf *elf, struct instruction *first,
+			   unsigned char first_state)
+{
+	struct alternative *alt;
+	struct instruction *insn;
+	struct section *sec;
+	unsigned char state;
+	int ret, warnings = 0;
+
+	insn = first;
+	sec = insn->sec;
+	state = first_state;
+
+	if (insn->alt_group && list_empty(&insn->alts)) {
+		WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
+			  sec, insn->offset);
+		warnings++;
+	}
+
+	while (1) {
+		if (insn->visited) {
+			if (insn->state != state) {
+				WARN_FUNC("frame pointer state mismatch",
+					  sec, insn->offset);
+				warnings++;
+			}
+
+			return warnings;
+		}
+
+		insn->visited = true;
+		insn->state = state;
+
+		list_for_each_entry(alt, &insn->alts, list) {
+			ret = validate_branch(elf, alt->insn, state);
+			warnings += ret;
+		}
+
+		switch (insn->type) {
+
+		case INSN_FP_SAVE:
+			if (!args.nofp) {
+				if (insn->state & STATE_FP_SAVED) {
+					WARN_FUNC("duplicate frame pointer save",
+						  sec, insn->offset);
+					warnings++;
+				}
+				state |= STATE_FP_SAVED;
+			}
+			break;
+
+		case INSN_FP_SETUP:
+			if (!args.nofp) {
+				if (insn->state & STATE_FP_SETUP) {
+					WARN_FUNC("duplicate frame pointer setup",
+						  sec, insn->offset);
+					warnings++;
+				}
+				state |= STATE_FP_SETUP;
+			}
+			break;
+
+		case INSN_FP_RESTORE:
+			if (!args.nofp) {
+				if (!insn->state) {
+					WARN_FUNC("frame pointer restore without save/setup",
+						  sec, insn->offset);
+					warnings++;
+				}
+				state &= ~STATE_FP_SAVED;
+				state &= ~STATE_FP_SETUP;
+			}
+			break;
+
+		case INSN_RETURN:
+			if (!args.nofp && insn->state) {
+				WARN_FUNC("return without frame pointer restore",
+					  sec, insn->offset);
+				warnings++;
+			}
+			return warnings;
+
+		case INSN_CALL:
+			if (insn->call_dest->type == STT_NOTYPE &&
+			    !strcmp(insn->call_dest->name, "__fentry__"))
+				break;
+
+			if (dead_end_function(insn->call_dest))
+				return warnings;
+
+			if (!args.nofp && !insn->state && !insn->ignore) {
+				WARN_FUNC("call without frame pointer save/setup",
+					  sec, insn->offset);
+				warnings++;
+			}
+			break;
+
+		case INSN_CALL_DYNAMIC:
+			if (!args.nofp && !insn->state && !insn->ignore) {
+				WARN_FUNC("call without frame pointer save/setup",
+					  sec, insn->offset);
+				warnings++;
+			}
+			break;
+
+		case INSN_JUMP_CONDITIONAL:
+		case INSN_JUMP_UNCONDITIONAL:
+			if (insn->jump_dest) {
+				ret = validate_branch(elf, insn->jump_dest,
+						      state);
+				warnings += ret;
+			} else if (insn->state) {
+				WARN_FUNC("sibling call from callable instruction with changed frame pointer",
+					  sec, insn->offset);
+				warnings++;
+			} /* else it's a sibling call */
+
+			if (insn->type == INSN_JUMP_UNCONDITIONAL)
+				return warnings;
+
+			break;
+
+		case INSN_JUMP_DYNAMIC:
+			if (list_empty(&insn->alts) && insn->state &&
+			    !insn->ignore) {
+				WARN_FUNC("sibling call from callable instruction with changed frame pointer",
+					  sec, insn->offset);
+				warnings++;
+			}
+
+			return warnings;
+
+		case INSN_CONTEXT_SWITCH:
+			if (!insn->ignore) {
+				WARN_FUNC("kernel entry/exit from callable instruction",
+					  sec, insn->offset);
+				warnings++;
+			}
+
+			return warnings;
+
+		case INSN_BUG:
+			return warnings;
+
+		}
+
+		insn = list_next_entry(insn, list);
+
+		if (&insn->list == &insns || insn->sec != sec) {
+			WARN("%s: unexpected end of section", sec->name);
+			warnings++;
+			return warnings;
+		}
+	}
+
+	return warnings;
+}
+
+static int validate_functions(struct elf *elf)
+{
+	struct section *sec;
+	struct symbol *func;
+	struct instruction *insn;
+	int ret, warnings = 0;
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		list_for_each_entry(func, &sec->symbols, list) {
+			if (func->type != STT_FUNC)
+				continue;
+
+			insn = find_instruction(sec, func->offset);
+			if (!insn) {
+				WARN("%s(): can't find starting instruction",
+				     func->name);
+				warnings++;
+				continue;
+			}
+
+			ret = validate_branch(elf, insn, 0);
+			warnings += ret;
+		}
+	}
+
+	list_for_each_entry(sec, &elf->sections, list) {
+		list_for_each_entry(func, &sec->symbols, list) {
+			if (func->type != STT_FUNC)
+				continue;
+
+			insn = find_instruction(sec, func->offset);
+			if (!insn)
+				continue;
+
+			list_for_each_entry_from(insn, &insns, list) {
+				if (insn->sec != func->sec ||
+				    insn->offset >= func->offset + func->len)
+					break;
+
+				if (!insn->visited && insn->type != INSN_NOP) {
+					WARN_FUNC("function has unreachable instruction",
+						  insn->sec, insn->offset);
+					warnings++;
+				}
+
+				insn->visited = true;
+			}
+		}
+	}
+
+	return warnings;
+}
+
+static int validate_uncallable_instructions(struct elf *elf)
+{
+	struct instruction *insn;
+	int warnings = 0;
+
+	list_for_each_entry(insn, &insns, list) {
+		if (!insn->visited && insn->type == INSN_RETURN &&
+		    !insn->ignore) {
+			WARN_FUNC("return instruction outside of a callable function",
+				  insn->sec, insn->offset);
+			warnings++;
+		}
+	}
+
+	return warnings;
+}
+
+static void cleanup(struct elf *elf)
+{
+	struct instruction *insn, *tmpinsn;
+	struct alternative *alt, *tmpalt;
+
+	list_for_each_entry_safe(insn, tmpinsn, &insns, list) {
+		list_for_each_entry_safe(alt, tmpalt, &insn->alts, list) {
+			list_del(&alt->list);
+			free(alt);
+		}
+		list_del(&insn->list);
+		free(insn);
+	}
+	elf_close(elf);
+}
+
+int main(int argc, char *argv[])
+{
+	struct elf *elf;
+	int ret = 0, 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;
+	}
+
+	ret = decode_sections(elf);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
+
+	ret = validate_functions(elf);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
+
+	ret = validate_uncallable_instructions(elf);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
+
+out:
+	cleanup(elf);
+
+	/* ignore warnings for now until we get all the code cleaned up */
+	if (ret || warnings)
+		return 0;
+	return 0;
+}
-- 
2.4.3

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Andrew Morton]

On Thu, 13 Aug 2015 22:10:24 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> This adds a CONFIG_STACK_VALIDATION option which enables a host tool
> named stackvalidate which runs at compile time.

It would be useful to

- show example output for a typical error site

- describe the consequences of that error (ie: why should we fix
  these things?)

- describe what needs to be done to repair these sites.


IOW, what do we gain from merging all this stuff?

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Josh Poimboeuf]

On Sat, Aug 15, 2015 at 12:23:54AM -0700, Andrew Morton wrote:
> On Thu, 13 Aug 2015 22:10:24 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > This adds a CONFIG_STACK_VALIDATION option which enables a host tool
> > named stackvalidate which runs at compile time.
> 
> It would be useful to
> 
> - show example output for a typical error site
> 
> - describe the consequences of that error (ie: why should we fix
>   these things?)
> 
> - describe what needs to be done to repair these sites.
> 
> 
> IOW, what do we gain from merging all this stuff?

I attempted to do all that in Documentation/stack-validation.txt which
is in patch 03/20.  Does it address your concerns?  Here it is:


Compile-time stack validation
=============================


Overview
--------

The CONFIG_STACK_VALIDATION option enables a host tool named
stackvalidate which runs at compile time.  It analyzes every .o file and
ensures the validity of its stack metadata.  It enforces a set of rules
on asm code and C inline assembly code so that stack traces can be
reliable.

Currently it only checks frame pointer usage, but there are plans to add
CFI validation for C files and CFI generation for asm files.

For each function, it recursively follows all possible code paths and
validates the correct frame pointer state at each instruction.

It also follows code paths involving special sections, like
.altinstructions, __jump_table, and __ex_table, which can add
alternative execution paths to a given instruction (or set of
instructions).  Similarly, it knows how to follow switch statements, for
which gcc sometimes uses jump tables.


Rules
-----

To achieve the validation, stackvalidate enforces the following rules:

1. Each callable function must be annotated as such with the ELF
   function type.  In asm code, this is typically done using the
   ENTRY/ENDPROC macros.  If stackvalidate finds a return instruction
   outside of a function, it flags an error since that usually indicates
   callable code which should be annotated accordingly.

2. Conversely, each section of code which is *not* callable should *not*
   be annotated as an ELF function.  The ENDPROC macro shouldn't be used
   in this case.

3. Each callable function which calls another function must have the
   correct frame pointer logic, if required by CONFIG_FRAME_POINTER or
   the architecture's back chain rules.  This can by done in asm code
   with the FRAME_BEGIN/FRAME_END macros.

4. Dynamic jumps and jumps to undefined symbols are only allowed if:

   a) the jump is part of a switch statement; or

   b) the jump matches sibling call semantics and the frame pointer has
      the same value it had on function entry.

5. A callable function may not execute kernel entry/exit instructions.
   The only code which needs such instructions is kernel entry code,
   which shouldn't be be in callable functions anyway.


Errors in .S files
------------------

If you're getting an error in a compiled .S file which you don't
understand, first make sure that the affected code follows the above
rules.

Here are some examples of common problems and suggestions for how to fix
them.


1. stackvalidate: asm_file.o: func()+0x128: call without frame pointer save/setup

   The func() function made a function call without first saving and/or
   updating the frame pointer.

   If func() is indeed a callable function, add proper frame pointer
   logic using the FP_SAVE and FP_RESTORE macros.  Otherwise, remove its
   ELF function annotation by changing ENDPROC to END.

   If you're getting this error in a .c file, see the "Errors in .c
   files" section.


2. stackvalidate: asm_file.o: .text+0x53: return instruction outside of a callable function

   A return instruction was detected, but stackvalidate couldn't find a
   way for a callable function to reach the instruction.

   If the return instruction is inside (or reachable from) a callable
   function, the function needs to be annotated with the ENTRY/ENDPROC
   macros.

   If you _really_ need a return instruction outside of a function, and
   are 100% sure that it won't affect stack traces, you can tell
   stackvalidate to ignore it.  See the "Adding exceptions" section
   below.


3. stackvalidate: asm_file.o: func()+0x9: function has unreachable instruction

   The instruction lives inside of a callable function, but there's no
   possible control flow path from the beginning of the function to the
   instruction.

   If the instruction is actually needed, and it's actually in a
   callable function, ensure that its function is properly annotated
   with ENTRY/ENDPROC.

   If it's not actually in a callable function (e.g. kernel entry code),
   change ENDPROC to END.


4. stackvalidate: asm_file.o: func(): can't find starting instruction
   or
   stackvalidate: asm_file.o: func()+0x11dd: can't decode instruction

   Did you put data in a text section?  If so, that can confuse
   stackvalidate's instruction decoder.  Move the data to a more
   appropriate section like .data or .rodata.


5. stackvalidate: asm_file.o: func()+0x6: kernel entry/exit from callable instruction

   This is a kernel entry/exit instruction like sysenter or sysret.
   Such instructions aren't allowed in a callable function, and are most
   likely part of the kernel entry code.

   If the instruction isn't actually in a callable function, change
   ENDPROC to END.


6. stackvalidate: asm_file.o: func()+0x26: sibling call from callable instruction with changed frame pointer

   This is a dynamic jump or a jump to an undefined symbol.
   Stackvalidate assumed it's a sibling call and detected that the frame
   pointer wasn't first restored to its original state.

   If it's not really a sibling call, you may need to move the
   destination code to the local file.

   If the instruction is not actually in a callable function (e.g.
   kernel entry code), change ENDPROC to END.


7. stackvalidate: asm_file: func()+0x5c: frame pointer state mismatch

   The instruction's frame pointer state is inconsistent, depending on
   which execution path was taken to reach the instruction.

   Make sure the function pushes and sets up the frame pointer (for
   x86_64, this means rbp) at the beginning of the function and pops it
   at the end of the function.  Also make sure that no other code in the
   function touches the frame pointer.


Errors in .c files
------------------

If you're getting a stackvalidate error in a compiled .c file, chances
are the file uses an asm() statement which has a "call" instruction.  An
asm() statement with a call instruction must declare the use of the
stack pointer in its output operand.  For example, on x86_64:

   register void *__sp asm("rsp");
   asm volatile("call func" : "+r" (__sp));

Otherwise the stack frame may not get created before the call.

Another possible cause for errors in C code is if the Makefile removes
-fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.

Also see the above section for .S file errors for more information what
the individual error messages mean.



Adding exceptions
-----------------

If you _really_ need stackvalidate to ignore something, and are 100%
sure that it won't affect kernel stack traces, you can tell
stackvalidate to ignore it:

- To skip validation of an instruction, use the
  STACKVALIDATE_IGNORE_INSN macro immediately before the instruction.

- To skip validation of a function, use the STACKVALIDATE_IGNORE_FUNC
  macro.

- To skip validation of a file, add "STACKVALIDATE_filename.o := n" to
  the Makefile.

- To skip validation of a directory, add "STACKVALIDATE := n" to the
  Makefile.



-- 
Josh

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Ingo Molnar]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Sat, Aug 15, 2015 at 12:23:54AM -0700, Andrew Morton wrote:
> > On Thu, 13 Aug 2015 22:10:24 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > This adds a CONFIG_STACK_VALIDATION option which enables a host tool
> > > named stackvalidate which runs at compile time.
> > 
> > It would be useful to
> > 
> > - show example output for a typical error site
> > 
> > - describe the consequences of that error (ie: why should we fix
> >   these things?)
> > 
> > - describe what needs to be done to repair these sites.
> > 
> > 
> > IOW, what do we gain from merging all this stuff?
> 
> I attempted to do all that in Documentation/stack-validation.txt which
> is in patch 03/20.  Does it address your concerns?  Here it is:

I think it answers the first and third question, but not the second one:

    - describe the consequences of that error (ie: why should we fix
      these things?)

would be nice to document all that richly.

Thanks,

	Ingo

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Josh Poimboeuf]

On Wed, Aug 19, 2015 at 12:01:38PM +0200, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > On Sat, Aug 15, 2015 at 12:23:54AM -0700, Andrew Morton wrote:
> > > On Thu, 13 Aug 2015 22:10:24 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > 
> > > > This adds a CONFIG_STACK_VALIDATION option which enables a host tool
> > > > named stackvalidate which runs at compile time.
> > > 
> > > It would be useful to
> > > 
> > > - show example output for a typical error site
> > > 
> > > - describe the consequences of that error (ie: why should we fix
> > >   these things?)
> > > 
> > > - describe what needs to be done to repair these sites.
> > > 
> > > 
> > > IOW, what do we gain from merging all this stuff?
> > 
> > I attempted to do all that in Documentation/stack-validation.txt which
> > is in patch 03/20.  Does it address your concerns?  Here it is:
> 
> I think it answers the first and third question, but not the second one:
> 
>     - describe the consequences of that error (ie: why should we fix
>       these things?)
> 
> would be nice to document all that richly.

Ok.  I've tried to answer the "why" question from both a broad
perspective ("why do we need stackvalidate?") as well as for each of the
rules that it enforces.

---8<---

Subject: [PATCH] stackvalidate: Document why it's needed

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/stack-validation.txt | 122 +++++++++++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 4 deletions(-)

diff --git a/Documentation/stack-validation.txt b/Documentation/stack-validation.txt
index c3d3d35..94dad40 100644
--- a/Documentation/stack-validation.txt
+++ b/Documentation/stack-validation.txt
@@ -24,6 +24,101 @@ instructions).  Similarly, it knows how to follow switch statements, for
 which gcc sometimes uses jump tables.
 
 
+Why do we need stack validation?
+--------------------------------
+
+Here are some of the benefits of validating stack metadata:
+
+a) More reliable stack traces for frame pointer enabled kernels
+
+   Frame pointers are used for debugging purposes.  They allow runtime
+   code and debug tools to be able to walk the stack to determine the
+   chain of function call sites that led to the currently executing
+   code.
+
+   For some architectures, frame pointers are enabled by
+   CONFIG_FRAME_POINTER.  For some other architectures they may be
+   required by the ABI (sometimes referred to as "backchain pointers").
+
+   For C code, gcc automatically generates instructions for setting up
+   frame pointers when the -fno-omit-frame-pointer option is used.
+
+   But for asm code, the frame setup instructions have to be written by
+   hand, which most people don't do.  So the end result is that
+   CONFIG_FRAME_POINTER is honored for C code but not for most asm code.
+
+   For stack traces based on frame pointers to be reliable, all
+   functions which call other functions must first create a stack frame
+   and update the frame pointer.  If a first function doesn't properly
+   create a stack frame before calling a second function, the *caller*
+   of the first function will be skipped on the stack trace.
+
+   The benefit of stackvalidate here is that it ensures that *all*
+   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
+   ever [*] be skipped on a stack trace.
+
+   [*] unless an interrupt or exception has occurred at the very
+       beginning of a function before the stack frame has been created,
+       or at the very end of the function after the stack frame has been
+       destroyed.  This is an inherent limitation of frame pointers.
+
+b) 100% reliable stack traces for DWARF enabled kernels
+
+   (NOTE: This is not yet implemented)
+
+   As an alternative to frame pointers, DWARF Call Frame Information
+   (CFI) metadata can be used to walk the stack.  Unlike frame pointers,
+   CFI metadata is out of band.  So it doesn't affect runtime
+   performance and it can be reliable even when interrupts or exceptions
+   are involved.
+
+   For C code, gcc automatically generates DWARF CFI metadata.  But for
+   asm code, generating CFI is a tedious manual approach which requires
+   manually placed .cfi assembler macros to be scattered throughout the
+   code.  It's clumsy and very easy to get wrong, and it makes the real
+   code harder to read.
+
+   Stackvalidate will improve this situation in several ways.  For code
+   which already has CFI annotations, it will validate them.  For code
+   which doesn't have CFI annotations, it will generate them.  So an
+   architecture can opt to strip out all the manual .cfi annotations
+   from their asm code and have stackvalidate generate them instead.
+
+   We might also add a runtime stack validation debug option where we
+   periodically walk the stack from schedule() and/or an NMI to ensure
+   that the stack metadata is sane and that we reach the bottom of the
+   stack.
+
+   So the benefit of stackvalidate here will be that external tooling
+   should always show perfect stack traces.  And the same will be true
+   for kernel warning/oops traces if the architecture has a runtime
+   DWARF unwinder.
+
+c) Higher live patching compatibility rate
+
+   (NOTE: This is not yet implemented)
+
+   Currently with CONFIG_LIVEPATCH there's a basic live patching
+   framework which is safe for roughly 85-90% of "security" fixes.  But
+   patches can't have complex features like function dependency or
+   prototype changes, or data structure changes.
+
+   There's a strong need to support patches which have the more complex
+   features so that the patch compatibility rate for security fixes can
+   eventually approach something resembling 100%.  To achieve that, a
+   "consistency model" is needed, which allows tasks to be safely
+   transitioned from an unpatched state to a patched state.
+
+   One of the key requirements of the currently proposed livepatch
+   consistency model [*] is that it needs to walk the stack of each
+   sleeping task to determine if it can be transitioned to the patched
+   state.  If stackvalidate can ensure that stack traces are reliable,
+   this consistency model can be used and the live patching
+   compatibility rate can be improved significantly.
+
+   [*] https://lkml.kernel.org/r/cover.1423499826.git.jpoimboe@redhat.com
+
+
 Rules
 -----
 
@@ -35,15 +130,26 @@ To achieve the validation, stackvalidate enforces the following rules:
    outside of a function, it flags an error since that usually indicates
    callable code which should be annotated accordingly.
 
+   This rule is needed so that stackvalidate can properly identify each
+   callable function in order to analyze its stack metadata.
+
 2. Conversely, each section of code which is *not* callable should *not*
    be annotated as an ELF function.  The ENDPROC macro shouldn't be used
    in this case.
 
+   This rule is needed so that stackvalidate can ignore non-callable
+   code.  Such code doesn't have to follow any of the other rules.
+
 3. Each callable function which calls another function must have the
    correct frame pointer logic, if required by CONFIG_FRAME_POINTER or
    the architecture's back chain rules.  This can by done in asm code
    with the FRAME_BEGIN/FRAME_END macros.
 
+   This rule ensures that frame pointer based stack traces will work as
+   designed.  If function A doesn't create a stack frame before calling
+   function B, the _caller_ of function A will be skipped on the stack
+   trace.
+
 4. Dynamic jumps and jumps to undefined symbols are only allowed if:
 
    a) the jump is part of a switch statement; or
@@ -51,10 +157,18 @@ To achieve the validation, stackvalidate enforces the following rules:
    b) the jump matches sibling call semantics and the frame pointer has
       the same value it had on function entry.
 
+   This rule is needed so that stackvalidate can reliably analyze all of
+   a function's code paths.  If a function jumps to code in another
+   file, and it's not a sibling call, stackvalidate has no way to follow
+   the jump because it only analyzes a single file at a time.
+
 5. A callable function may not execute kernel entry/exit instructions.
    The only code which needs such instructions is kernel entry code,
    which shouldn't be be in callable functions anyway.
 
+   This rule is just a sanity check to ensure that callable functions
+   return normally.
+
 
 Errors in .S files
 ------------------
@@ -63,8 +177,8 @@ If you're getting an error in a compiled .S file which you don't
 understand, first make sure that the affected code follows the above
 rules.
 
-Here are some examples of common problems and suggestions for how to fix
-them.
+Here are some examples of common warnings reported by stackvalidate,
+what they mean, and suggestions for how to fix them.
 
 
 1. stackvalidate: asm_file.o: func()+0x128: call without frame pointer save/setup
@@ -73,8 +187,8 @@ them.
    updating the frame pointer.
 
    If func() is indeed a callable function, add proper frame pointer
-   logic using the FP_SAVE and FP_RESTORE macros.  Otherwise, remove its
-   ELF function annotation by changing ENDPROC to END.
+   logic using the FRAME_BEGIN and FRAME_END macros.  Otherwise, remove
+   its ELF function annotation by changing ENDPROC to END.
 
    If you're getting this error in a .c file, see the "Errors in .c
    files" section.
-- 
2.4.3

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Ingo Molnar]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> +Why do we need stack validation?
> +--------------------------------
> +
> +Here are some of the benefits of validating stack metadata:
> +
> +a) More reliable stack traces for frame pointer enabled kernels
> +
> +   Frame pointers are used for debugging purposes.  They allow runtime
> +   code and debug tools to be able to walk the stack to determine the
> +   chain of function call sites that led to the currently executing
> +   code.
> +
> +   For some architectures, frame pointers are enabled by
> +   CONFIG_FRAME_POINTER.  For some other architectures they may be
> +   required by the ABI (sometimes referred to as "backchain pointers").
> +
> +   For C code, gcc automatically generates instructions for setting up
> +   frame pointers when the -fno-omit-frame-pointer option is used.
> +
> +   But for asm code, the frame setup instructions have to be written by
> +   hand, which most people don't do.  So the end result is that
> +   CONFIG_FRAME_POINTER is honored for C code but not for most asm code.
> +
> +   For stack traces based on frame pointers to be reliable, all
> +   functions which call other functions must first create a stack frame
> +   and update the frame pointer.  If a first function doesn't properly
> +   create a stack frame before calling a second function, the *caller*
> +   of the first function will be skipped on the stack trace.
> +
> +   The benefit of stackvalidate here is that it ensures that *all*
> +   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
> +   ever [*] be skipped on a stack trace.
> +
> +   [*] unless an interrupt or exception has occurred at the very
> +       beginning of a function before the stack frame has been created,
> +       or at the very end of the function after the stack frame has been
> +       destroyed.  This is an inherent limitation of frame pointers.

What this section does not point out is the actual effects of missing frame 
pointer annotations. I.e. how about quoting a before/after stack backtrace to 
demonstrate it?

Thanks,

	Ingo

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Josh Poimboeuf]

On Fri, Aug 21, 2015 at 09:54:49AM +0200, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > +Why do we need stack validation?
> > +--------------------------------
> > +
> > +Here are some of the benefits of validating stack metadata:
> > +
> > +a) More reliable stack traces for frame pointer enabled kernels
> > +
> > +   Frame pointers are used for debugging purposes.  They allow runtime
> > +   code and debug tools to be able to walk the stack to determine the
> > +   chain of function call sites that led to the currently executing
> > +   code.
> > +
> > +   For some architectures, frame pointers are enabled by
> > +   CONFIG_FRAME_POINTER.  For some other architectures they may be
> > +   required by the ABI (sometimes referred to as "backchain pointers").
> > +
> > +   For C code, gcc automatically generates instructions for setting up
> > +   frame pointers when the -fno-omit-frame-pointer option is used.
> > +
> > +   But for asm code, the frame setup instructions have to be written by
> > +   hand, which most people don't do.  So the end result is that
> > +   CONFIG_FRAME_POINTER is honored for C code but not for most asm code.
> > +
> > +   For stack traces based on frame pointers to be reliable, all
> > +   functions which call other functions must first create a stack frame
> > +   and update the frame pointer.  If a first function doesn't properly
> > +   create a stack frame before calling a second function, the *caller*
> > +   of the first function will be skipped on the stack trace.
> > +
> > +   The benefit of stackvalidate here is that it ensures that *all*
> > +   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
> > +   ever [*] be skipped on a stack trace.
> > +
> > +   [*] unless an interrupt or exception has occurred at the very
> > +       beginning of a function before the stack frame has been created,
> > +       or at the very end of the function after the stack frame has been
> > +       destroyed.  This is an inherent limitation of frame pointers.
> 
> What this section does not point out is the actual effects of missing frame 
> pointer annotations. I.e. how about quoting a before/after stack backtrace to 
> demonstrate it?

How about this (on top of the last one):

---8<---

From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] stackvalidate: Add missing frame pointer example

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/stack-validation.txt | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/Documentation/stack-validation.txt b/Documentation/stack-validation.txt
index 94dad40..87a5ab8 100644
--- a/Documentation/stack-validation.txt
+++ b/Documentation/stack-validation.txt
@@ -53,9 +53,40 @@ a) More reliable stack traces for frame pointer enabled kernels
    create a stack frame before calling a second function, the *caller*
    of the first function will be skipped on the stack trace.
 
-   The benefit of stackvalidate here is that it ensures that *all*
-   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
-   ever [*] be skipped on a stack trace.
+   For example, consider the following example backtrace with frame
+   pointers enabled:
+
+     [<ffffffff81812584>] dump_stack+0x4b/0x63
+     [<ffffffff812d6dc2>] cmdline_proc_show+0x12/0x30
+     [<ffffffff8127f568>] seq_read+0x108/0x3e0
+     [<ffffffff812cce62>] proc_reg_read+0x42/0x70
+     [<ffffffff81256197>] __vfs_read+0x37/0x100
+     [<ffffffff81256b16>] vfs_read+0x86/0x130
+     [<ffffffff81257898>] SyS_read+0x58/0xd0
+     [<ffffffff8181c1f2>] entry_SYSCALL_64_fastpath+0x12/0x76
+
+   It correctly shows that the caller of cmdline_proc_show() is
+   seq_read().
+
+   If we remove the frame pointer logic from cmdline_proc_show() by
+   replacing the frame pointer related instructions with nops, here's
+   what it looks like instead:
+
+     [<ffffffff81812584>] dump_stack+0x4b/0x63
+     [<ffffffff812d6dc2>] cmdline_proc_show+0x12/0x30
+     [<ffffffff812cce62>] proc_reg_read+0x42/0x70
+     [<ffffffff81256197>] __vfs_read+0x37/0x100
+     [<ffffffff81256b16>] vfs_read+0x86/0x130
+     [<ffffffff81257898>] SyS_read+0x58/0xd0
+     [<ffffffff8181c1f2>] entry_SYSCALL_64_fastpath+0x12/0x76
+
+   Notice that cmdline_proc_show()'s caller, seq_read(), has been
+   skipped.  Instead the stack trace seems to show that
+   cmdline_proc_show() was called by proc_reg_read().
+
+   The benefit of stackvalidate here is that because it ensures that
+   *all* functions honor CONFIG_FRAME_POINTER, no functions will ever[*]
+   be skipped on a stack trace.
 
    [*] unless an interrupt or exception has occurred at the very
        beginning of a function before the stack frame has been created,
-- 
2.4.3

Re: [PATCH v10 03/20] x86/stackvalidate: Compile-time stack validation

[Ingo Molnar]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Fri, Aug 21, 2015 at 09:54:49AM +0200, Ingo Molnar wrote:
> > 
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > +Why do we need stack validation?
> > > +--------------------------------
> > > +
> > > +Here are some of the benefits of validating stack metadata:
> > > +
> > > +a) More reliable stack traces for frame pointer enabled kernels
> > > +
> > > +   Frame pointers are used for debugging purposes.  They allow runtime
> > > +   code and debug tools to be able to walk the stack to determine the
> > > +   chain of function call sites that led to the currently executing
> > > +   code.
> > > +
> > > +   For some architectures, frame pointers are enabled by
> > > +   CONFIG_FRAME_POINTER.  For some other architectures they may be
> > > +   required by the ABI (sometimes referred to as "backchain pointers").
> > > +
> > > +   For C code, gcc automatically generates instructions for setting up
> > > +   frame pointers when the -fno-omit-frame-pointer option is used.
> > > +
> > > +   But for asm code, the frame setup instructions have to be written by
> > > +   hand, which most people don't do.  So the end result is that
> > > +   CONFIG_FRAME_POINTER is honored for C code but not for most asm code.
> > > +
> > > +   For stack traces based on frame pointers to be reliable, all
> > > +   functions which call other functions must first create a stack frame
> > > +   and update the frame pointer.  If a first function doesn't properly
> > > +   create a stack frame before calling a second function, the *caller*
> > > +   of the first function will be skipped on the stack trace.
> > > +
> > > +   The benefit of stackvalidate here is that it ensures that *all*
> > > +   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
> > > +   ever [*] be skipped on a stack trace.
> > > +
> > > +   [*] unless an interrupt or exception has occurred at the very
> > > +       beginning of a function before the stack frame has been created,
> > > +       or at the very end of the function after the stack frame has been
> > > +       destroyed.  This is an inherent limitation of frame pointers.
> > 
> > What this section does not point out is the actual effects of missing frame 
> > pointer annotations. I.e. how about quoting a before/after stack backtrace to 
> > demonstrate it?
> 
> How about this (on top of the last one):
> 
> ---8<---
> 
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] stackvalidate: Add missing frame pointer example
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  Documentation/stack-validation.txt | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/stack-validation.txt b/Documentation/stack-validation.txt
> index 94dad40..87a5ab8 100644
> --- a/Documentation/stack-validation.txt
> +++ b/Documentation/stack-validation.txt
> @@ -53,9 +53,40 @@ a) More reliable stack traces for frame pointer enabled kernels
>     create a stack frame before calling a second function, the *caller*
>     of the first function will be skipped on the stack trace.
>  
> -   The benefit of stackvalidate here is that it ensures that *all*
> -   functions honor CONFIG_FRAME_POINTER.  As a result, no functions will
> -   ever [*] be skipped on a stack trace.
> +   For example, consider the following example backtrace with frame
> +   pointers enabled:
> +
> +     [<ffffffff81812584>] dump_stack+0x4b/0x63
> +     [<ffffffff812d6dc2>] cmdline_proc_show+0x12/0x30
> +     [<ffffffff8127f568>] seq_read+0x108/0x3e0
> +     [<ffffffff812cce62>] proc_reg_read+0x42/0x70
> +     [<ffffffff81256197>] __vfs_read+0x37/0x100
> +     [<ffffffff81256b16>] vfs_read+0x86/0x130
> +     [<ffffffff81257898>] SyS_read+0x58/0xd0
> +     [<ffffffff8181c1f2>] entry_SYSCALL_64_fastpath+0x12/0x76
> +
> +   It correctly shows that the caller of cmdline_proc_show() is
> +   seq_read().
> +
> +   If we remove the frame pointer logic from cmdline_proc_show() by
> +   replacing the frame pointer related instructions with nops, here's
> +   what it looks like instead:
> +
> +     [<ffffffff81812584>] dump_stack+0x4b/0x63
> +     [<ffffffff812d6dc2>] cmdline_proc_show+0x12/0x30
> +     [<ffffffff812cce62>] proc_reg_read+0x42/0x70
> +     [<ffffffff81256197>] __vfs_read+0x37/0x100
> +     [<ffffffff81256b16>] vfs_read+0x86/0x130
> +     [<ffffffff81257898>] SyS_read+0x58/0xd0
> +     [<ffffffff8181c1f2>] entry_SYSCALL_64_fastpath+0x12/0x76
> +
> +   Notice that cmdline_proc_show()'s caller, seq_read(), has been
> +   skipped.  Instead the stack trace seems to show that
> +   cmdline_proc_show() was called by proc_reg_read().
> +
> +   The benefit of stackvalidate here is that because it ensures that
> +   *all* functions honor CONFIG_FRAME_POINTER, no functions will ever[*]
> +   be skipped on a stack trace.

Ok, this sounds good to me!

Thanks,

	Ingo

[PATCH v10 04/20] x86/stackvalidate: Add file and directory ignores

[Josh Poimboeuf]

Tell stackvalidate to skip validation of the following code:

- boot image
- vdso image
- kexec purgatory
- realmode
- efi libstub
- scripts/mod

They all run outside the kernel's normal mode of operation and they
don't affect runtime kernel stack traces, so they're free to skirt the
stackvalidate rules.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/boot/Makefile                | 3 ++-
 arch/x86/boot/compressed/Makefile     | 3 ++-
 arch/x86/entry/vdso/Makefile          | 5 ++++-
 arch/x86/purgatory/Makefile           | 2 ++
 arch/x86/realmode/Makefile            | 4 +++-
 arch/x86/realmode/rm/Makefile         | 3 ++-
 drivers/firmware/efi/libstub/Makefile | 1 +
 scripts/mod/Makefile                  | 2 ++
 8 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 0d553e5..d31808c 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -14,7 +14,8 @@
 # Set it to -DSVGA_MODE=NORMAL_VGA if you just want the EGA/VGA mode.
 # The number is the same as you would ordinarily press at bootup.
 
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
 
 SVGA_MODE	:= -DSVGA_MODE=NORMAL_VGA
 
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 0a291cd..530a46f 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -16,7 +16,8 @@
 #	(see scripts/Makefile.lib size_append)
 #	compressed vmlinux.bin.all + u32 size of vmlinux.bin.all
 
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
 
 targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
 	vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index a3d0767..b7cbf61 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -3,7 +3,9 @@
 #
 
 KBUILD_CFLAGS += $(DISABLE_LTO)
-KASAN_SANITIZE := n
+
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
 
 VDSO64-$(CONFIG_X86_64)		:= y
 VDSOX32-$(CONFIG_X86_X32_ABI)	:= y
@@ -15,6 +17,7 @@ vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
 
 # files to link into kernel
 obj-y				+= vma.o
+STACKVALIDATE_vma.o		:= y
 
 # vDSO images to build
 vdso_img-$(VDSO64-y)		+= 64
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 2c835e3..a736c19 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -1,3 +1,5 @@
+STACKVALIDATE := n
+
 purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string.o
 
 targets += $(purgatory-y)
diff --git a/arch/x86/realmode/Makefile b/arch/x86/realmode/Makefile
index e02c2c6..7a2d4df 100644
--- a/arch/x86/realmode/Makefile
+++ b/arch/x86/realmode/Makefile
@@ -6,7 +6,9 @@
 # for more details.
 #
 #
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
+
 subdir- := rm
 
 obj-y += init.o
diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
index 2730d77..d462a57 100644
--- a/arch/x86/realmode/rm/Makefile
+++ b/arch/x86/realmode/rm/Makefile
@@ -6,7 +6,8 @@
 # for more details.
 #
 #
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
 
 always := realmode.bin realmode.relocs
 
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 816dbe9..b392f3f 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -20,6 +20,7 @@ KBUILD_CFLAGS			:= $(cflags-y) \
 
 GCOV_PROFILE			:= n
 KASAN_SANITIZE			:= n
+STACKVALIDATE			:= n
 
 lib-y				:= efi-stub-helper.o
 lib-$(CONFIG_EFI_ARMSTUB)	+= arm-stub.o fdt.o
diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile
index c11212f..374c413 100644
--- a/scripts/mod/Makefile
+++ b/scripts/mod/Makefile
@@ -1,3 +1,5 @@
+STACKVALIDATE	:= n
+
 hostprogs-y	:= modpost mk_elfconfig
 always		:= $(hostprogs-y) empty.o
 
-- 
2.4.3

[PATCH v10 05/20] x86/stackvalidate: Add ignore macros

[Josh Poimboeuf]

Add new stackvalidate ignore macros: STACKVALIDATE_IGNORE_INSN and
STACKVALIDATE_IGNORE_FUNC.  These can be used to tell stackvalidate to
skip validation of an instruction or a function, respectively.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/stackvalidate.h | 45 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/vmlinux.lds.S        |  5 +++-
 include/linux/stackvalidate.h        | 28 ++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/include/asm/stackvalidate.h
 create mode 100644 include/linux/stackvalidate.h

diff --git a/arch/x86/include/asm/stackvalidate.h b/arch/x86/include/asm/stackvalidate.h
new file mode 100644
index 0000000..95db052
--- /dev/null
+++ b/arch/x86/include/asm/stackvalidate.h
@@ -0,0 +1,45 @@
+#ifndef _ASM_X86_STACKVALIDATE_H
+#define _ASM_X86_STACKVALIDATE_H
+
+#include <asm/asm.h>
+
+#ifdef __ASSEMBLY__
+
+/*
+ * This asm macro tells the stack validation script to ignore the instruction
+ * immediately after the macro.  It should only be used in special cases where
+ * you're 100% sure it won't affect the reliability of frame pointers and
+ * kernel stack traces.
+ *
+ * For more information, see Documentation/stack-validation.txt.
+ */
+.macro STACKVALIDATE_IGNORE_INSN
+#ifdef CONFIG_STACK_VALIDATION
+	.Lstackvalidate_ignore_\@:
+	.pushsection __stackvalidate_ignore_insn, "a"
+	_ASM_ALIGN
+	.long .Lstackvalidate_ignore_\@ - .
+	.popsection
+#endif
+.endm
+
+#else /* !__ASSEMBLY__ */
+
+#ifdef CONFIG_STACK_VALIDATION
+
+#define STACKVALIDATE_IGNORE_INSN				\
+	"1:\n"							\
+	".pushsection __stackvalidate_ignore_insn, \"a\"\n"	\
+	_ASM_ALIGN "\n"						\
+	".long 1b - .\n"					\
+	".popsection\n"
+
+#else /* !CONFIG_STACK_VALIDATION */
+
+#define STACKVALIDATE_IGNORE_INSN ""
+
+#endif /* CONFIG_STACK_VALIDATION */
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_X86_STACKVALIDATE_H */
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 00bf300..f2f8d7a 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -332,7 +332,10 @@ SECTIONS
 
 	/* Sections to be discarded */
 	DISCARDS
-	/DISCARD/ : { *(.eh_frame) }
+	/DISCARD/ : {
+		*(.eh_frame)
+		*(__stackvalidate_ignore_*)
+	}
 }
 
 
diff --git a/include/linux/stackvalidate.h b/include/linux/stackvalidate.h
new file mode 100644
index 0000000..4ae242c
--- /dev/null
+++ b/include/linux/stackvalidate.h
@@ -0,0 +1,28 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#include <asm/stackvalidate.h>
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_STACK_VALIDATION
+/*
+ * This C macro tells the stack validation script to ignore the function.  It
+ * should only be used in special cases where you're 100% sure it won't affect
+ * the reliability of frame pointers and kernel stack traces.
+ *
+ * For more information, see Documentation/stack-validation.txt.
+ */
+#define STACKVALIDATE_IGNORE_FUNC(_func) \
+	static void __used __section(__stackvalidate_ignore_func) \
+		*__stackvalidate_ignore_func_##_func = _func
+
+#else /* !CONFIG_STACK_VALIDATION */
+
+#define STACKVALIDATE_IGNORE_FUNC(_func)
+
+#endif /* CONFIG_STACK_VALIDATION */
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* _LINUX_STACKVALIDATE_H */
-- 
2.4.3

[PATCH v10 06/20] x86/xen: Add stack frame dependency to hypercall inline asm calls

[Josh Poimboeuf]

If a hypercall is inlined at the beginning of a function, gcc can insert
the call instruction before setting up a stack frame, which breaks frame
pointer convention if CONFIG_FRAME_POINTER is enabled and can result in
a bad stack trace.

Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by
listing the stack pointer as an output operand for the hypercall inline
asm statements.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/include/asm/xen/hypercall.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index ca08a27..4488fbc 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -110,9 +110,10 @@ extern struct { char _entry[32]; } hypercall_page[];
 	register unsigned long __arg2 asm(__HYPERCALL_ARG2REG) = __arg2; \
 	register unsigned long __arg3 asm(__HYPERCALL_ARG3REG) = __arg3; \
 	register unsigned long __arg4 asm(__HYPERCALL_ARG4REG) = __arg4; \
-	register unsigned long __arg5 asm(__HYPERCALL_ARG5REG) = __arg5;
+	register unsigned long __arg5 asm(__HYPERCALL_ARG5REG) = __arg5; \
+	register void *__sp asm(_ASM_SP);
 
-#define __HYPERCALL_0PARAM	"=r" (__res)
+#define __HYPERCALL_0PARAM	"=r" (__res), "+r" (__sp)
 #define __HYPERCALL_1PARAM	__HYPERCALL_0PARAM, "+r" (__arg1)
 #define __HYPERCALL_2PARAM	__HYPERCALL_1PARAM, "+r" (__arg2)
 #define __HYPERCALL_3PARAM	__HYPERCALL_2PARAM, "+r" (__arg3)
-- 
2.4.3

[PATCH v10 07/20] x86/paravirt: Add stack frame dependency to PVOP inline asm calls

[Josh Poimboeuf]

If a PVOP call macro is inlined at the beginning of a function, gcc can
insert the call instruction before setting up a stack frame, which
breaks frame pointer convention if CONFIG_FRAME_POINTER is enabled and
can result in a bad stack trace.

Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by
listing the stack pointer as an output operand for the PVOP inline asm
statements.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Alok Kataria <akataria@vmware.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
 arch/x86/include/asm/paravirt_types.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index ce029e4..1812294 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -497,8 +497,9 @@ int paravirt_disable_iospace(void);
  * makes sure the incoming and outgoing types are always correct.
  */
 #ifdef CONFIG_X86_32
-#define PVOP_VCALL_ARGS				\
-	unsigned long __eax = __eax, __edx = __edx, __ecx = __ecx
+#define PVOP_VCALL_ARGS							\
+	unsigned long __eax = __eax, __edx = __edx, __ecx = __ecx;	\
+	register void *__sp asm("esp")
 #define PVOP_CALL_ARGS			PVOP_VCALL_ARGS
 
 #define PVOP_CALL_ARG1(x)		"a" ((unsigned long)(x))
@@ -516,9 +517,10 @@ int paravirt_disable_iospace(void);
 #define VEXTRA_CLOBBERS
 #else  /* CONFIG_X86_64 */
 /* [re]ax isn't an arg, but the return val */
-#define PVOP_VCALL_ARGS					\
-	unsigned long __edi = __edi, __esi = __esi,	\
-		__edx = __edx, __ecx = __ecx, __eax = __eax
+#define PVOP_VCALL_ARGS						\
+	unsigned long __edi = __edi, __esi = __esi,		\
+		__edx = __edx, __ecx = __ecx, __eax = __eax;	\
+	register void *__sp asm("rsp")
 #define PVOP_CALL_ARGS		PVOP_VCALL_ARGS
 
 #define PVOP_CALL_ARG1(x)		"D" ((unsigned long)(x))
@@ -557,7 +559,7 @@ int paravirt_disable_iospace(void);
 			asm volatile(pre				\
 				     paravirt_alt(PARAVIRT_CALL)	\
 				     post				\
-				     : call_clbr			\
+				     : "+r" (__sp), call_clbr		\
 				     : paravirt_type(op),		\
 				       paravirt_clobber(clbr),		\
 				       ##__VA_ARGS__			\
@@ -567,7 +569,7 @@ int paravirt_disable_iospace(void);
 			asm volatile(pre				\
 				     paravirt_alt(PARAVIRT_CALL)	\
 				     post				\
-				     : call_clbr			\
+				     : "+r" (__sp), call_clbr		\
 				     : paravirt_type(op),		\
 				       paravirt_clobber(clbr),		\
 				       ##__VA_ARGS__			\
@@ -594,7 +596,7 @@ int paravirt_disable_iospace(void);
 		asm volatile(pre					\
 			     paravirt_alt(PARAVIRT_CALL)		\
 			     post					\
-			     : call_clbr				\
+			     : "+r" (__sp), call_clbr			\
 			     : paravirt_type(op),			\
 			       paravirt_clobber(clbr),			\
 			       ##__VA_ARGS__				\
-- 
2.4.3

[PATCH v10 08/20] x86/paravirt: Create a stack frame in PV_CALLEE_SAVE_REGS_THUNK

[Josh Poimboeuf]

A function created with the PV_CALLEE_SAVE_REGS_THUNK macro doesn't set
up a new stack frame before the call instruction, which breaks frame
pointer convention if CONFIG_FRAME_POINTER is enabled and can result in
a bad stack trace.  Also, the thunk functions aren't annotated as ELF
callable functions.

Create a stack frame when CONFIG_FRAME_POINTER is enabled and add the
ELF function type.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Alok Kataria <akataria@vmware.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
 arch/x86/include/asm/paravirt.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c2be037..0d28801 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -13,6 +13,7 @@
 #include <linux/bug.h>
 #include <linux/types.h>
 #include <linux/cpumask.h>
+#include <asm/frame.h>
 
 static inline int paravirt_enabled(void)
 {
@@ -772,15 +773,19 @@ static __always_inline void __ticket_unlock_kick(struct arch_spinlock *lock,
  * call. The return value in rax/eax will not be saved, even for void
  * functions.
  */
+#define PV_THUNK_NAME(func) "__raw_callee_save_" #func
 #define PV_CALLEE_SAVE_REGS_THUNK(func)					\
 	extern typeof(func) __raw_callee_save_##func;			\
 									\
 	asm(".pushsection .text;"					\
-	    ".globl __raw_callee_save_" #func " ; "			\
-	    "__raw_callee_save_" #func ": "				\
+	    ".globl " PV_THUNK_NAME(func) ";"				\
+	    ".type " PV_THUNK_NAME(func) ", @function;"			\
+	    PV_THUNK_NAME(func) ":"					\
+	    FRAME_BEGIN							\
 	    PV_SAVE_ALL_CALLER_REGS					\
 	    "call " #func ";"						\
 	    PV_RESTORE_ALL_CALLER_REGS					\
+	    FRAME_END							\
 	    "ret;"							\
 	    ".popsection")
 
-- 
2.4.3

[PATCH v10 09/20] x86/amd: Set ELF function type for vide()

[Josh Poimboeuf]

vide() is a callable function, but is missing the ELF function type,
which confuses tools like stackvalidate.

Properly annotate it to be a callable function.  The generated code is
unchanged.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/cpu/amd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 51ad2af..d19c9ad 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -74,7 +74,10 @@ static inline int wrmsrl_amd_safe(unsigned msr, unsigned long long val)
  */
 
 extern __visible void vide(void);
-__asm__(".globl vide\n\t.align 4\nvide: ret");
+__asm__(".globl vide\n"
+	".type vide, @function\n"
+	".align 4\n"
+	"vide: ret\n");
 
 static void init_amd_k5(struct cpuinfo_x86 *c)
 {
-- 
2.4.3

[PATCH v10 10/20] x86/reboot: Add ljmp instructions to stackvalidate whitelist

[Josh Poimboeuf]

stackvalidate reports a false positive warning for the ljmp instruction
in machine_real_restart().  Normally, ljmp isn't allowed in a function,
but this is a special case where it's jumping into real mode.

Add the jumps to a whitelist which tells stackvalidate to ignore them.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/reboot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 86db4bc..0931d87 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -9,6 +9,7 @@
 #include <linux/sched.h>
 #include <linux/tboot.h>
 #include <linux/delay.h>
+#include <linux/stackvalidate.h>
 #include <acpi/reboot.h>
 #include <asm/io.h>
 #include <asm/apic.h>
@@ -97,11 +98,13 @@ void __noreturn machine_real_restart(unsigned int type)
 
 	/* Jump to the identity-mapped low memory code */
 #ifdef CONFIG_X86_32
-	asm volatile("jmpl *%0" : :
+	asm volatile(STACKVALIDATE_IGNORE_INSN
+		     "jmpl *%0;" : :
 		     "rm" (real_mode_header->machine_real_restart_asm),
 		     "a" (type));
 #else
-	asm volatile("ljmpl *%0" : :
+	asm volatile(STACKVALIDATE_IGNORE_INSN
+		     "ljmpl *%0" : :
 		     "m" (real_mode_header->machine_real_restart_asm),
 		     "D" (type));
 #endif
-- 
2.4.3

[PATCH v10 11/20] x86/xen: Add xen_cpuid() and xen_setup_gdt() to stackvalidate whitelists

[Josh Poimboeuf]

stackvalidate reports the following false positive warnings:

  stackvalidate: arch/x86/xen/enlighten.o: xen_cpuid()+0x41: can't find jump dest instruction at .text+0x108
  stackvalidate: arch/x86/xen/enlighten.o: xen_setup_gdt.constprop.23()+0x2e: kernel entry/exit from callable instruction

The first warning is due to xen_cpuid()'s use of XEN_EMULATE_PREFIX to
insert some fake instructions which stackvalidate doesn't know how to
decode.

The second warning is due to xen_setup_gdt()'s use of an lret
instruction, which stackvalidate normally doesn't allow in callable
functions.  But this seems to be a valid use of the instruction.

Add both functions to the stackvalidate whitelist.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/enlighten.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d9cfa45..4ca89ac 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -32,6 +32,7 @@
 #include <linux/gfp.h>
 #include <linux/memblock.h>
 #include <linux/edd.h>
+#include <linux/stackvalidate.h>
 
 #include <xen/xen.h>
 #include <xen/events.h>
@@ -345,8 +346,8 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 	*cx &= maskecx;
 	*cx |= setecx;
 	*dx &= maskedx;
-
 }
+STACKVALIDATE_IGNORE_FUNC(xen_cpuid);
 
 static bool __init xen_check_mwait(void)
 {
@@ -1443,6 +1444,7 @@ static void __ref xen_setup_gdt(int cpu)
 	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry;
 	pv_cpu_ops.load_gdt = xen_load_gdt;
 }
+STACKVALIDATE_IGNORE_FUNC(xen_setup_gdt);
 
 #ifdef CONFIG_XEN_PVH
 /*
-- 
2.4.3

[PATCH v10 12/20] x86/asm/crypto: Create stack frames in aesni-intel_asm.S

[Josh Poimboeuf]

aesni-intel_asm.S has several callable non-leaf functions which don't
honor CONFIG_FRAME_POINTER, which can result in bad stack traces.

Create stack frames for them when CONFIG_FRAME_POINTER is enabled.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/x86/crypto/aesni-intel_asm.S | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index 6bd2c6c..289ef12 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -31,6 +31,7 @@
 
 #include <linux/linkage.h>
 #include <asm/inst.h>
+#include <asm/frame.h>
 
 /*
  * The following macros are used to move an (un)aligned 16 byte value to/from
@@ -1800,6 +1801,7 @@ ENDPROC(_key_expansion_256b)
  *                   unsigned int key_len)
  */
 ENTRY(aesni_set_key)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl KEYP
 	movl 8(%esp), KEYP		# ctx
@@ -1905,6 +1907,7 @@ ENTRY(aesni_set_key)
 #ifndef __x86_64__
 	popl KEYP
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_set_key)
 
@@ -1912,6 +1915,7 @@ ENDPROC(aesni_set_key)
  * void aesni_enc(struct crypto_aes_ctx *ctx, u8 *dst, const u8 *src)
  */
 ENTRY(aesni_enc)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl KEYP
 	pushl KLEN
@@ -1927,6 +1931,7 @@ ENTRY(aesni_enc)
 	popl KLEN
 	popl KEYP
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_enc)
 
@@ -2101,6 +2106,7 @@ ENDPROC(_aesni_enc4)
  * void aesni_dec (struct crypto_aes_ctx *ctx, u8 *dst, const u8 *src)
  */
 ENTRY(aesni_dec)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl KEYP
 	pushl KLEN
@@ -2117,6 +2123,7 @@ ENTRY(aesni_dec)
 	popl KLEN
 	popl KEYP
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_dec)
 
@@ -2292,6 +2299,7 @@ ENDPROC(_aesni_dec4)
  *		      size_t len)
  */
 ENTRY(aesni_ecb_enc)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl LEN
 	pushl KEYP
@@ -2342,6 +2350,7 @@ ENTRY(aesni_ecb_enc)
 	popl KEYP
 	popl LEN
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_ecb_enc)
 
@@ -2350,6 +2359,7 @@ ENDPROC(aesni_ecb_enc)
  *		      size_t len);
  */
 ENTRY(aesni_ecb_dec)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl LEN
 	pushl KEYP
@@ -2401,6 +2411,7 @@ ENTRY(aesni_ecb_dec)
 	popl KEYP
 	popl LEN
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_ecb_dec)
 
@@ -2409,6 +2420,7 @@ ENDPROC(aesni_ecb_dec)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_cbc_enc)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl IVP
 	pushl LEN
@@ -2443,6 +2455,7 @@ ENTRY(aesni_cbc_enc)
 	popl LEN
 	popl IVP
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_cbc_enc)
 
@@ -2451,6 +2464,7 @@ ENDPROC(aesni_cbc_enc)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_cbc_dec)
+	FRAME_BEGIN
 #ifndef __x86_64__
 	pushl IVP
 	pushl LEN
@@ -2534,6 +2548,7 @@ ENTRY(aesni_cbc_dec)
 	popl LEN
 	popl IVP
 #endif
+	FRAME_END
 	ret
 ENDPROC(aesni_cbc_dec)
 
@@ -2598,6 +2613,7 @@ ENDPROC(_aesni_inc)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_ctr_enc)
+	FRAME_BEGIN
 	cmp $16, LEN
 	jb .Lctr_enc_just_ret
 	mov 480(KEYP), KLEN
@@ -2651,6 +2667,7 @@ ENTRY(aesni_ctr_enc)
 .Lctr_enc_ret:
 	movups IV, (IVP)
 .Lctr_enc_just_ret:
+	FRAME_END
 	ret
 ENDPROC(aesni_ctr_enc)
 
@@ -2677,6 +2694,7 @@ ENDPROC(aesni_ctr_enc)
  *			 bool enc, u8 *iv)
  */
 ENTRY(aesni_xts_crypt8)
+	FRAME_BEGIN
 	cmpb $0, %cl
 	movl $0, %ecx
 	movl $240, %r10d
@@ -2777,6 +2795,7 @@ ENTRY(aesni_xts_crypt8)
 	pxor INC, STATE4
 	movdqu STATE4, 0x70(OUTP)
 
+	FRAME_END
 	ret
 ENDPROC(aesni_xts_crypt8)
 
-- 
2.4.3

[PATCH v10 13/20] x86/asm/crypto: Move .Lbswap_mask data to .rodata section

[Josh Poimboeuf]

stackvalidate reports the following warning:

  stackvalidate: arch/x86/crypto/aesni-intel_asm.o: _aesni_inc_init(): can't find starting instruction

stackvalidate gets confused when it tries to disassemble the following
data in the .text section:

  .Lbswap_mask:
          .byte 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0

Move it to .rodata which is a more appropriate section for read-only
data.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/x86/crypto/aesni-intel_asm.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index 289ef12..18f0ced 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -2553,9 +2553,11 @@ ENTRY(aesni_cbc_dec)
 ENDPROC(aesni_cbc_dec)
 
 #ifdef __x86_64__
+.pushsection .rodata
 .align 16
 .Lbswap_mask:
 	.byte 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0
+.popsection
 
 /*
  * _aesni_inc_init:	internal ABI
-- 
2.4.3

[PATCH v10 14/20] x86/asm/crypto: Move jump_table to .rodata section

[Josh Poimboeuf]

stackvalidate reports the following warning:

  stackvalidate: arch/x86/crypto/crc32c-pcl-intel-asm_64.o: crc_pcl()+0x11dd: can't decode instruction

It gets confused when trying to decode jump_table data.  Move jump_table
to the .rodata section which is a more appropriate home for read-only
data.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
index 225be06..dc05f01 100644
--- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
+++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
@@ -170,8 +170,8 @@ continue_block:
 	## branch into array
 	lea	jump_table(%rip), bufp
 	movzxw  (bufp, %rax, 2), len
-	offset=crc_array-jump_table
-	lea     offset(bufp, len, 1), bufp
+	lea	crc_array(%rip), bufp
+	lea     (bufp, len, 1), bufp
 	jmp     *bufp
 
 	################################################################
@@ -310,7 +310,9 @@ do_return:
 	popq    %rdi
 	popq    %rbx
         ret
+ENDPROC(crc_pcl)
 
+.section	.rodata, "a", %progbits
         ################################################################
         ## jump table        Table is 129 entries x 2 bytes each
         ################################################################
@@ -324,13 +326,11 @@ JMPTBL_ENTRY %i
 	i=i+1
 .endr
 
-ENDPROC(crc_pcl)
 
 	################################################################
 	## PCLMULQDQ tables
 	## Table is 128 entries x 2 words (8 bytes) each
 	################################################################
-.section	.rotata, "a", %progbits
 .align 8
 K_table:
 	.long 0x493c7d27, 0x00000001
-- 
2.4.3

[PATCH v10 15/20] x86/asm/crypto: Create stack frames in clmul_ghash_mul/update()

[Josh Poimboeuf]

clmul_ghash_mul() and clmul_ghash_update() are callable non-leaf
functions which don't honor CONFIG_FRAME_POINTER, which can result in
bad stack traces.

Create stack frames for them when CONFIG_FRAME_POINTER is enabled.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/x86/crypto/ghash-clmulni-intel_asm.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/crypto/ghash-clmulni-intel_asm.S b/arch/x86/crypto/ghash-clmulni-intel_asm.S
index 5d1e007..eed55c8 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_asm.S
+++ b/arch/x86/crypto/ghash-clmulni-intel_asm.S
@@ -18,6 +18,7 @@
 
 #include <linux/linkage.h>
 #include <asm/inst.h>
+#include <asm/frame.h>
 
 .data
 
@@ -94,6 +95,7 @@ ENDPROC(__clmul_gf128mul_ble)
 
 /* void clmul_ghash_mul(char *dst, const u128 *shash) */
 ENTRY(clmul_ghash_mul)
+	FRAME_BEGIN
 	movups (%rdi), DATA
 	movups (%rsi), SHASH
 	movaps .Lbswap_mask, BSWAP
@@ -101,6 +103,7 @@ ENTRY(clmul_ghash_mul)
 	call __clmul_gf128mul_ble
 	PSHUFB_XMM BSWAP DATA
 	movups DATA, (%rdi)
+	FRAME_END
 	ret
 ENDPROC(clmul_ghash_mul)
 
@@ -109,6 +112,7 @@ ENDPROC(clmul_ghash_mul)
  *			   const u128 *shash);
  */
 ENTRY(clmul_ghash_update)
+	FRAME_BEGIN
 	cmp $16, %rdx
 	jb .Lupdate_just_ret	# check length
 	movaps .Lbswap_mask, BSWAP
@@ -128,5 +132,6 @@ ENTRY(clmul_ghash_update)
 	PSHUFB_XMM BSWAP DATA
 	movups DATA, (%rdi)
 .Lupdate_just_ret:
+	FRAME_END
 	ret
 ENDPROC(clmul_ghash_update)
-- 
2.4.3

[PATCH v10 16/20] x86/asm/entry: Create stack frames in thunk functions

[Josh Poimboeuf]

Thunk functions are callable non-leaf functions that don't honor
CONFIG_FRAME_POINTER, which can result in bad stack traces.  Also they
aren't annotated as ELF callable functions which can confuse tooling.

Create stack frames for them when CONFIG_FRAME_POINTER is enabled and
add the ELF function type.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/entry/thunk_64.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index efb2b93..98df1fa 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -8,11 +8,14 @@
 #include <linux/linkage.h>
 #include "calling.h"
 #include <asm/asm.h>
+#include <asm/frame.h>
 
 	/* rdi:	arg1 ... normal C conventions. rax is saved/restored. */
 	.macro THUNK name, func, put_ret_addr_in_rdi=0
 	.globl \name
+	.type \name, @function
 \name:
+	FRAME_BEGIN
 
 	/* this one pushes 9 elems, the next one would be %rIP */
 	pushq %rdi
@@ -62,6 +65,7 @@ restore:
 	popq %rdx
 	popq %rsi
 	popq %rdi
+	FRAME_END
 	ret
 	_ASM_NOKPROBE(restore)
 #endif
-- 
2.4.3

[PATCH v10 17/20] x86/asm/acpi: Create a stack frame in do_suspend_lowlevel()

[Josh Poimboeuf]

do_suspend_lowlevel() is a callable non-leaf function which doesn't
honor CONFIG_FRAME_POINTER, which can result in bad stack traces.

Create a stack frame for it when CONFIG_FRAME_POINTER is enabled.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
---
 arch/x86/kernel/acpi/wakeup_64.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S
index 8c35df4..169963f 100644
--- a/arch/x86/kernel/acpi/wakeup_64.S
+++ b/arch/x86/kernel/acpi/wakeup_64.S
@@ -5,6 +5,7 @@
 #include <asm/page_types.h>
 #include <asm/msr.h>
 #include <asm/asm-offsets.h>
+#include <asm/frame.h>
 
 # Copyright 2003 Pavel Machek <pavel@suse.cz>, distribute under GPLv2
 
@@ -39,6 +40,7 @@ bogus_64_magic:
 	jmp	bogus_64_magic
 
 ENTRY(do_suspend_lowlevel)
+	FRAME_BEGIN
 	subq	$8, %rsp
 	xorl	%eax, %eax
 	call	save_processor_state
@@ -109,6 +111,7 @@ ENTRY(do_suspend_lowlevel)
 
 	xorl	%eax, %eax
 	addq	$8, %rsp
+	FRAME_END
 	jmp	restore_processor_state
 ENDPROC(do_suspend_lowlevel)
 
-- 
2.4.3

[PATCH v10 18/20] x86/asm: Create stack frames in rwsem functions

[Josh Poimboeuf]

rwsem.S has several callable non-leaf functions which don't honor
CONFIG_FRAME_POINTER, which can result in bad stack traces.

Create stack frames for them when CONFIG_FRAME_POINTER is enabled.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/lib/rwsem.S | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
index 40027db..be110ef 100644
--- a/arch/x86/lib/rwsem.S
+++ b/arch/x86/lib/rwsem.S
@@ -15,6 +15,7 @@
 
 #include <linux/linkage.h>
 #include <asm/alternative-asm.h>
+#include <asm/frame.h>
 
 #define __ASM_HALF_REG(reg)	__ASM_SEL(reg, e##reg)
 #define __ASM_HALF_SIZE(inst)	__ASM_SEL(inst##w, inst##l)
@@ -84,24 +85,29 @@
 
 /* Fix up special calling conventions */
 ENTRY(call_rwsem_down_read_failed)
+	FRAME_BEGIN
 	save_common_regs
 	__ASM_SIZE(push,) %__ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_down_read_failed
 	__ASM_SIZE(pop,) %__ASM_REG(dx)
 	restore_common_regs
+	FRAME_END
 	ret
 ENDPROC(call_rwsem_down_read_failed)
 
 ENTRY(call_rwsem_down_write_failed)
+	FRAME_BEGIN
 	save_common_regs
 	movq %rax,%rdi
 	call rwsem_down_write_failed
 	restore_common_regs
+	FRAME_END
 	ret
 ENDPROC(call_rwsem_down_write_failed)
 
 ENTRY(call_rwsem_wake)
+	FRAME_BEGIN
 	/* do nothing if still outstanding active readers */
 	__ASM_HALF_SIZE(dec) %__ASM_HALF_REG(dx)
 	jnz 1f
@@ -109,15 +115,18 @@ ENTRY(call_rwsem_wake)
 	movq %rax,%rdi
 	call rwsem_wake
 	restore_common_regs
-1:	ret
+1:	FRAME_END
+	ret
 ENDPROC(call_rwsem_wake)
 
 ENTRY(call_rwsem_downgrade_wake)
+	FRAME_BEGIN
 	save_common_regs
 	__ASM_SIZE(push,) %__ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_downgrade_wake
 	__ASM_SIZE(pop,) %__ASM_REG(dx)
 	restore_common_regs
+	FRAME_END
 	ret
 ENDPROC(call_rwsem_downgrade_wake)
-- 
2.4.3

[PATCH v10 19/20] x86/asm/efi: Create a stack frame in efi_call()

[Josh Poimboeuf]

efi_call() is a callable non-leaf function which doesn't honor
CONFIG_FRAME_POINTER, which can result in bad stack traces.

Create a stack frame for it when CONFIG_FRAME_POINTER is enabled.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/platform/efi/efi_stub_64.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S
index 86d0f9e..0df2dcc 100644
--- a/arch/x86/platform/efi/efi_stub_64.S
+++ b/arch/x86/platform/efi/efi_stub_64.S
@@ -11,6 +11,7 @@
 #include <asm/msr.h>
 #include <asm/processor-flags.h>
 #include <asm/page_types.h>
+#include <asm/frame.h>
 
 #define SAVE_XMM			\
 	mov %rsp, %rax;			\
@@ -74,6 +75,7 @@
 	.endm
 
 ENTRY(efi_call)
+	FRAME_BEGIN
 	SAVE_XMM
 	mov (%rsp), %rax
 	mov 8(%rax), %rax
@@ -88,6 +90,7 @@ ENTRY(efi_call)
 	RESTORE_PGT
 	addq $48, %rsp
 	RESTORE_XMM
+	FRAME_END
 	ret
 ENDPROC(efi_call)
 
-- 
2.4.3

Re: [PATCH v10 19/20] x86/asm/efi: Create a stack frame in efi_call()

[Matt Fleming]

On Thu, 13 Aug, at 10:10:40PM, Josh Poimboeuf wrote:
> efi_call() is a callable non-leaf function which doesn't honor
> CONFIG_FRAME_POINTER, which can result in bad stack traces.
> 
> Create a stack frame for it when CONFIG_FRAME_POINTER is enabled.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/x86/platform/efi/efi_stub_64.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S
> index 86d0f9e..0df2dcc 100644
> --- a/arch/x86/platform/efi/efi_stub_64.S
> +++ b/arch/x86/platform/efi/efi_stub_64.S
> @@ -11,6 +11,7 @@
>  #include <asm/msr.h>
>  #include <asm/processor-flags.h>
>  #include <asm/page_types.h>
> +#include <asm/frame.h>
>  
>  #define SAVE_XMM			\
>  	mov %rsp, %rax;			\
> @@ -74,6 +75,7 @@
>  	.endm
>  
>  ENTRY(efi_call)
> +	FRAME_BEGIN
>  	SAVE_XMM
>  	mov (%rsp), %rax
>  	mov 8(%rax), %rax
> @@ -88,6 +90,7 @@ ENTRY(efi_call)
>  	RESTORE_PGT
>  	addq $48, %rsp
>  	RESTORE_XMM
> +	FRAME_END
>  	ret
>  ENDPROC(efi_call)

You mention that stackvalidate will recursively validate the frame
pointers in all code paths. Since we're calling into firmware code from
efi_call(), we don't need to do anything special here right?

I'm guessing stackvalidate would just stop since it has no way of
knowing the target address of the %call instruction, but I just wanted
to check (especially since the firmware ABI is different).

Reviewed-by: Matt Fleming <matt.fleming@intel.com>

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v10 19/20] x86/asm/efi: Create a stack frame in efi_call()

[Josh Poimboeuf]

On Fri, Aug 14, 2015 at 10:11:59AM +0100, Matt Fleming wrote:
> On Thu, 13 Aug, at 10:10:40PM, Josh Poimboeuf wrote:
> > efi_call() is a callable non-leaf function which doesn't honor
> > CONFIG_FRAME_POINTER, which can result in bad stack traces.
> > 
> > Create a stack frame for it when CONFIG_FRAME_POINTER is enabled.
> > 
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > Cc: Matt Fleming <matt.fleming@intel.com>
> > ---
> >  arch/x86/platform/efi/efi_stub_64.S | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S
> > index 86d0f9e..0df2dcc 100644
> > --- a/arch/x86/platform/efi/efi_stub_64.S
> > +++ b/arch/x86/platform/efi/efi_stub_64.S
> > @@ -11,6 +11,7 @@
> >  #include <asm/msr.h>
> >  #include <asm/processor-flags.h>
> >  #include <asm/page_types.h>
> > +#include <asm/frame.h>
> >  
> >  #define SAVE_XMM			\
> >  	mov %rsp, %rax;			\
> > @@ -74,6 +75,7 @@
> >  	.endm
> >  
> >  ENTRY(efi_call)
> > +	FRAME_BEGIN
> >  	SAVE_XMM
> >  	mov (%rsp), %rax
> >  	mov 8(%rax), %rax
> > @@ -88,6 +90,7 @@ ENTRY(efi_call)
> >  	RESTORE_PGT
> >  	addq $48, %rsp
> >  	RESTORE_XMM
> > +	FRAME_END
> >  	ret
> >  ENDPROC(efi_call)
> 
> You mention that stackvalidate will recursively validate the frame
> pointers in all code paths. Since we're calling into firmware code from
> efi_call(), we don't need to do anything special here right?
>
> I'm guessing stackvalidate would just stop since it has no way of
> knowing the target address of the %call instruction, but I just wanted
> to check (especially since the firmware ABI is different).

It recursively follows all code paths *inside* each function, including
jumps.  It doesn't try to follow calls across functions, which can't be
done reliably because of function pointers.  Instead it just verifies
that each function follows the calling conventions.

The result of creating a stack frame is that the *caller* of the
function shows up in the stack trace.  So this patch ensures that
efi_call()'s caller would show up if, for example, the stack was dumped
from an interrupt which occurred in efi_call() between FRAME_BEGIN and
FRAME_END.

As to whether it helps in case the stack is dumped from firmware code
(or from an interrupt to firmware), it really depends on what the
firmware does:

- If it follows frame pointer convention, great (but I'm guessing this
  is unlikely...)

- If it doesn't follow frame pointer convention, but still leaves rbp
  alone, then efi_call() would be skipped in the stack trace but
  efi_call()'s caller and the rest of the stack would still show up.

- If it trashes rbp, then we're out of luck and there's no stack trace.

But regardless of what firmware does, this patch still helps in the case
where the stack is dumped starting from efi_call().

> Reviewed-by: Matt Fleming <matt.fleming@intel.com>

Thanks!

-- 
Josh

[PATCH v10 20/20] x86/asm/power: Create stack frames in hibernate_asm_64.S

[Josh Poimboeuf]

swsusp_arch_suspend() and restore_registers() are callable non-leaf
functions which don't honor CONFIG_FRAME_POINTER, which can result in
bad stack traces.  Also they aren't annotated as ELF callable functions
which can confuse tooling.

Create a stack frame for them when CONFIG_FRAME_POINTER is enabled and
give them proper ELF function annotations.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 arch/x86/power/hibernate_asm_64.S | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/power/hibernate_asm_64.S b/arch/x86/power/hibernate_asm_64.S
index e2386cb..4400a43 100644
--- a/arch/x86/power/hibernate_asm_64.S
+++ b/arch/x86/power/hibernate_asm_64.S
@@ -21,8 +21,10 @@
 #include <asm/page_types.h>
 #include <asm/asm-offsets.h>
 #include <asm/processor-flags.h>
+#include <asm/frame.h>
 
 ENTRY(swsusp_arch_suspend)
+	FRAME_BEGIN
 	movq	$saved_context, %rax
 	movq	%rsp, pt_regs_sp(%rax)
 	movq	%rbp, pt_regs_bp(%rax)
@@ -50,7 +52,9 @@ ENTRY(swsusp_arch_suspend)
 	movq	%rax, restore_cr3(%rip)
 
 	call swsusp_save
+	FRAME_END
 	ret
+ENDPROC(swsusp_arch_suspend)
 
 ENTRY(restore_image)
 	/* switch to temporary page tables */
@@ -107,6 +111,7 @@ ENTRY(core_restore_code)
 	 */
 
 ENTRY(restore_registers)
+	FRAME_BEGIN
 	/* go back to the original page tables */
 	movq    %rbx, %cr3
 
@@ -147,4 +152,6 @@ ENTRY(restore_registers)
 	/* tell the hibernation core that we've just restored the memory */
 	movq	%rax, in_suspend(%rip)
 
+	FRAME_END
 	ret
+ENDPROC(restore_registers)
-- 
2.4.3