Skip to contentSkip to conversation

Compile-time stack validation

PATCH Superseded Josh Poimboeuf 2 participants 16 messages Jul 7, 2015 – Jul 9, 2015

Subsystems (7)

v6 of 19 Superseded Latest is v19 →

lore
Share & tools
lore
#0 lore raw

v6 cover Compile-time stack validation

This is (another) complete rewrite of the stack validation code, based on tip/master. The last version (v5) was named "Compile-time asm code validation" [1]. Once again I've renamed the patch series to reflect its changed scope. For similar reasons I've renamed the tool from asmvalidate back to stackvalidate again.

This cover letter only describes the changes since last time. For more information about the motivation behind this patch set, and more details about what it does, please see the changelog in patch 2.

The biggest changes:

1. Validation of C object files

Instead of only validating hand-coded asm object code (generated from .S files), it now also validates gcc-generated object code (from .c files).

This change was inspired by a comment from Andy where he mentioned some inconsistencies in how inline assembly interacts with CFI annotations.

I did some more looking and it turns out that inline assembly doesn't play nicely with frame pointers at all. If the inline asm is at the beginning of the function, gcc sometimes emits the inline asm code before setting up the frame pointer. That can break stack traces when the inline asm has a call instruction.

That turns out to be a very common problem. Stackvalidate found 37 C object files which break frame pointer rules, thanks to inline asm.

I don't know of a solution to this problem yet. Basically I think we need a way to ensure that gcc emits the frame pointer setup before inserting any inline asm (particularly when the inline asm has a call instruction).

2. Exhaustive searching of all code paths

Several people complained that the constraints proposed in v5 were too limiting, particularly the rule that a function can't jump outside of its bounds. So that restriction has been removed. Now the code is a lot smarter.

It starts at the entry to the function and recursively follows all possible branches, tracking the frame pointer state along the way, going outside the scope of the ELF-defined function bounds if it needs to.

3. Support for special sections which add alternate instructions

In addition to the validation of "alternate" code in .alternative sections, it supports fixup/exception tables, jump labels and gcc switch statement jump tables. All of those are needed for proper support of C object files.

4. Added documentation in Documentation/stack-validation.txt, with a list of common warnings and what they mean.

Those are the highlights. The version log below has a more complete list of changes.

To reduce churn, I didn't bother with fixing any of the warnings this time around. If there's agreement on this approach, I can start proposing fixes (but the inline asm issues still need more thinking).

Also posting a listing of the reported warnings in a reply to this email.

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

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

Josh Poimboeuf (4):
x86/asm: Frame pointer macro cleanup x86/stackvalidate: Compile-time stack validation x86/stackvalidate: Add file and directory ignores
stackvalidate: Add ignore macros

 Documentation/stack-validation.txt    | 189 ++++++++
 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/entry/vdso/Makefile          |   5 +-
 arch/x86/include/asm/frame.h          |  38 +-
 arch/x86/purgatory/Makefile           |   2 +
 arch/x86/realmode/Makefile            |   4 +-
 arch/x86/realmode/rm/Makefile         |   3 +-
 drivers/firmware/efi/libstub/Makefile |   1 +
 include/linux/stackvalidate.h         |  38 ++
 lib/Kconfig.debug                     |  11 +
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  34 +-
 scripts/stackvalidate/Makefile        |  24 +
 scripts/stackvalidate/arch-x86.c      | 147 ++++++
 scripts/stackvalidate/arch.h          |  44 ++
 scripts/stackvalidate/elf.c           | 421 +++++++++++++++++
 scripts/stackvalidate/elf.h           |  85 ++++
 scripts/stackvalidate/list.h          | 217 +++++++++
 scripts/stackvalidate/special.c       | 176 +++++++
 scripts/stackvalidate/special.h       |  40 ++
 scripts/stackvalidate/stackvalidate.c | 858 ++++++++++++++++++++++++++++++++++
 26 files changed, 2338 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/stack-validation.txt
 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
Signature · 1 line
2.1.0
#1 lore raw

PATCH v6 1/4 x86/asm: Frame pointer macro cleanup

The FRAME/ENDFRAME 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.

Make the code more readable and improve the comments.

---
 arch/x86/include/asm/frame.h | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)
arch/x86/include/asm/frame.h +21 -17
index 793179c..7addd0a 100644
@@ -1,23 +1,27 @@
+#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 */
-#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
-#endif
+/*
+ * These are frame pointer save and restore macros.  They should be used by
+ * every callable non-leaf asm function so that kernel stack traces can be
+ * more reliable.
+ */
+.macro FRAME
+	.if CONFIG_FRAME_POINTER
+		push %_ASM_BP
+		_ASM_MOV %_ASM_SP, %_ASM_BP
+	.endif
+.endm
+
+.macro ENDFRAME
+	.if CONFIG_FRAME_POINTER
+		pop %_ASM_BP
+	.endif
+.endm
 
 #endif  /*  __ASSEMBLY__  */
+#endif /* _ASM_X86_FRAME_H */
Signature · 1 line
2.1.0
#2 lore raw

PATCH v6 2/4 x86/stackvalidate: Compile-time stack validation

In reply to 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 plan to add DWARF CFI validation as well.

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 external 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/ENDFRAME macros.

4. A callable function may not jump to a symbol in another file. Such cross-file jumps can't be validated because stackvalidate only analyzes a single .o file at a time.

5. A callable function may not jump to a dynamically determined address. Such jumps can't be validated since the jump destination is unknown at compile time.

6. A callable function may not execute a context switching instruction. The only code which needs to do context switches is kernel entry code, which shouldn't be annotated to 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.

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

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 T430s 4-core laptop, building the kernel with it enabled adds about 3.6 seconds (14.4 seconds of total CPU). It hasn't been optimized for performance yet, so there are probably some opportunities for better build performance.

---
 Documentation/stack-validation.txt    | 189 ++++++++
 MAINTAINERS                           |   8 +
 arch/Kconfig                          |   6 +
 arch/x86/Kconfig                      |   1 +
 arch/x86/Makefile                     |   6 +-
 lib/Kconfig.debug                     |  11 +
 scripts/Makefile                      |   1 +
 scripts/Makefile.build                |  34 +-
 scripts/stackvalidate/Makefile        |  24 +
 scripts/stackvalidate/arch-x86.c      | 147 ++++++
 scripts/stackvalidate/arch.h          |  44 ++
 scripts/stackvalidate/elf.c           | 421 +++++++++++++++++
 scripts/stackvalidate/elf.h           |  85 ++++
 scripts/stackvalidate/list.h          | 217 +++++++++
 scripts/stackvalidate/special.c       | 176 +++++++
 scripts/stackvalidate/special.h       |  40 ++
 scripts/stackvalidate/stackvalidate.c | 858 ++++++++++++++++++++++++++++++++++
 17 files changed, 2263 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
Documentation/stack-validation.txt new file +189 -0
new file mode 100644
index 0000000..4b66336
@@ -0,0 +1,189 @@
+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
+DWARF CFI validation as well.
+
+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 external 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/ENDFRAME macros.
+
+4. A callable function may not jump to a symbol in another file.  Such
+   cross-file jumps can't be validated because stackvalidate only
+   analyzes a single .o file at a time.
+
+5. A callable function may not jump to a dynamically determined address.
+   Such jumps can't be validated since the jump destination is unknown
+   at compile time.
+
+6. A callable function may not execute a context switching instruction.
+   The only code which needs to do context switches is kernel entry
+   code, which shouldn't be annotated to 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.
+
+
+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 PROC/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 PROC/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: context switch from callable function
+
+   This is a context switch instruction like sysenter or sysret.  Such
+   instructions aren't allowed in a callable function, and are most
+   likely part of kernel entry code.
+
+   If the instruction isn't actually in a callable function, change
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.
+
+   If the instruction is not actually in a callable function (e.g.
+   kernel entry code), change ENDPROC to END.
+
+   Otherwise you may need to move the destination code to the local
+   file.
+
+
+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 inline assembly (or uses a macro or an inline function
+which uses inline assembly).  See the above section for .S file errors
+for an understanding what the individual error messages mean.
+
+Many of these errors seem to be caused by the fact that gcc doesn't seem
+to provide a way to deal properly with frame pointers when using inline
+assembly.  This needs more investigation and discussion...
+
+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.
+
+
+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.
MAINTAINERS +8 -0
index e63e337..ded7729 100644
@@ -9652,6 +9652,14 @@ L:	[email protected]
 S:	Supported
 F:	Documentation/stable_kernel_rules.txt
 
+STACK VALIDATION
+M:	Josh Poimboeuf <[email protected]>
+S:	Supported
+F:	scripts/stackvalidate/
+F:	arch/x86/include/asm/frame.h
+F:	include/linux/stackvalidate.h
+F:	Documentation/stack-validation.txt
+
 STAGING SUBSYSTEM
 M:	Greg Kroah-Hartman <[email protected]>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
arch/Kconfig +6 -0
index 150a01b..8a921d5 100644
@@ -515,6 +515,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
 #
arch/x86/Kconfig +1 -0
index 31fc0cd..e7370d1 100644
@@ -150,6 +150,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
arch/x86/Makefile +5 -1
index 118e6de..438c153 100644
@@ -174,9 +174,13 @@ KBUILD_CFLAGS += $(call cc-option,-mno-avx,)
 KBUILD_CFLAGS += $(mflags-y)
 KBUILD_AFLAGS += $(mflags-y)
 
-archscripts: scripts_basic
+archscripts: scripts_basic $(objtree)/arch/x86/lib/inat-tables.c
 	$(Q)$(MAKE) $(build)=arch/x86/tools relocs
 
+# this file is needed early by scripts/stackvalidate
+$(objtree)/arch/x86/lib/inat-tables.c:
+	$(Q)$(MAKE) $(build)=arch/x86/lib $@
+
 ###
 # Syscall table generation
 
lib/Kconfig.debug +11 -0
index e2894b2..6c96f36 100644
@@ -332,6 +332,17 @@ config FRAME_POINTER
 	  larger and slower, but it gives very useful debugging information
 	  in case of kernel bugs. (precise oopses/stacktraces/warnings)
 
+config STACK_VALIDATION
+	bool "Enable 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
scripts/Makefile +1 -0
index 2016a64..c882a91 100644
@@ -37,6 +37,7 @@ subdir-y                     += mod
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_DTC)         += dtc
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
+subdir-$(CONFIG_STACK_VALIDATION) += stackvalidate
 
 # Let clean descend into subdirs
 subdir-	+= basic kconfig package
scripts/Makefile.build +30 -4
index 01df30a..a1270d3 100644
@@ -241,9 +241,26 @@ cmd_record_mcount =						\
 	fi;
 endif
 
+ifdef CONFIG_STACK_VALIDATION
+
+stackvalidate = $(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.
+cmd_stackvalidate = $(if $(patsubst n%,, \
+	$(STACKVALIDATE_$(basetarget).o)$(STACKVALIDATE)y), \
+	$(stackvalidate) $(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_stackvalidate)						  \
 	$(cmd_modversions)						  \
 	$(call echo-cmd,record_mcount)					  \
 	$(cmd_record_mcount)						  \
@@ -253,14 +270,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) 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) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
 	@{ echo $(@:.o=.ko); echo $@; } > $(MODVERDIR)/$(@F:.o=.mod)
@@ -290,8 +316,8 @@ $(obj)/%.s: $(src)/%.S FORCE
 quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
 cmd_as_o_S       = $(CC) $(a_flags) -c -o $@ $<
 
-$(obj)/%.o: $(src)/%.S FORCE
-	$(call if_changed_dep,as_o_S)
+$(obj)/%.o: $(src)/%.S $(stackvalidate) FORCE
+	$(call if_changed_rule,as_o_S)
 
 targets += $(real-objs-y) $(real-objs-m) $(lib-y)
 targets += $(extra-y) $(MAKECMDGOALS) $(always)
scripts/stackvalidate/Makefile new file +24 -0
new file mode 100644
index 0000000..468c075
@@ -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
scripts/stackvalidate/arch-x86.c new file +147 -0

147 lines not on this page · raw message

scripts/stackvalidate/arch.h new file +44 -0

44 lines not on this page · raw message

scripts/stackvalidate/elf.c new file +421 -0

421 lines not on this page · raw message

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

85 lines not on this page · raw message

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

217 lines not on this page · raw message

scripts/stackvalidate/special.c new file +176 -0

176 lines not on this page · raw message

scripts/stackvalidate/special.h new file +40 -0
new file mode 100644
index 0000000..fc7ea8e
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <[email protected]>
+ *
+ * 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;
+
+	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 */
scripts/stackvalidate/stackvalidate.c new file +858 -0

858 lines not on this page · raw message

Signature · 1 line
2.1.0
#3 lore raw

Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

Quoted 3 lines · Josh Poimboeuf

5. A callable function may not jump to a dynamically determined address. Such jumps can't be validated since the jump destination is unknown at compile time.

Hmm. Are there no switch statements that get optimized into jump tables for which this breaks?

Quoted 6 lines

6. A callable function may not execute a context switching instruction. The only code which needs to do context switches is kernel entry code, which shouldn't be annotated to be in callable functions anyway.

Quoted 8 lines

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.

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

This issue might be worth bringing up on the gcc and binutils lists. If we need better toolchain support, let's ask for it.

Documentation/stack-validation.txt line 94

Quoted 13 lines

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

Documentation/stack-validation.txt new file +12 -0
+
+   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 PROC/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.

This will happen as soon as someone implements iretless-return-to-kernel for real. We can add an exception :)

Quoted 2 lines

+
+5. stackvalidate: asm_file.o: func()+0x6: context switch from callable function

This description threw me off for a bit. When I read "context switch", I thought of __switch_to. Would something like "kernel entry/exit instruction" instead of "context switch" be clearer?

Quoted 6 lines
Documentation/stack-validation.txt new file +6 -0
+
+   This is a context switch instruction like sysenter or sysret.  Such
+   instructions aren't allowed in a callable function, and are most
+   likely part of kernel entry code.
+
+   If the instruction isn't actually in a callable function, change

Documentation/stack-validation.txt line 136

Quoted 10 lines
Documentation/stack-validation.txt new file +10 -0
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?

--Andy

#4 lore raw

Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

Quoted 7 lines · Andy Lutomirski
Quoted 3 lines · Josh Poimboeuf

5. A callable function may not jump to a dynamically determined address. Such jumps can't be validated since the jump destination is unknown at compile time.

Hmm. Are there no switch statements that get optimized into jump tables for which this breaks?

Yeah. I discovered this the hard way and had to add support for reading the switch jump tables. I should probably make that clearer here in the changelog and in the documentation.

Quoted 9 lines
Quoted 9 lines

6. A callable function may not execute a context switching instruction. The only code which needs to do context switches is kernel entry code, which shouldn't be annotated to 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.

Quoted 7 lines
Quoted 4 lines

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

This issue might be worth bringing up on the gcc and binutils lists. If we need better toolchain support, let's ask for it.

I think we found a good solution for this. See my update at:

https://lkml.kernel.org/r/[email protected]

Quoted 17 lines

Documentation/stack-validation.txt line 94

Quoted 13 lines

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

Documentation/stack-validation.txt new file +12 -0
+
+   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 PROC/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.

This will happen as soon as someone implements iretless-return-to-kernel for real. We can add an exception :)

Quoted 6 lines
Quoted 2 lines

+
+5. stackvalidate: asm_file.o: func()+0x6: context switch from callable function

This description threw me off for a bit. When I read "context switch", I thought of __switch_to. Would something like "kernel entry/exit instruction" instead of "context switch" be clearer?

Yeah, I'll rename it.

Quoted 10 lines
Quoted 10 lines
Documentation/stack-validation.txt new file +10 -0
+
+   This is a context switch instruction like sysenter or sysret.  Such
+   instructions aren't allowed in a callable function, and are most
+   likely part of kernel entry code.
+
+   If the instruction isn't actually in a callable function, change
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
Quoted 8 lines

Documentation/stack-validation.txt line 136

Quoted 6 lines
Documentation/stack-validation.txt new file +6 -0
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?

This is a great point. With CONFIG_FRAME_POINTER it's not a problem, because it adds -fno-optimize-sibling-calls.

Without it, I think stackvalidate would spit out a ton of "jump to outside file" warnings.

I haven't yet looked at the details of how exactly sibling calls work. I'd assume they're disabled because they break frame pointers somehow. Any idea if they'd also break DWARF CFI stack traces?

I probably need to do some digging there. If sibling calls don't break CFI stack traces and we end up needing them, stackvalidate might need to analyze the entire kernel image at once instead of its current per-.o checking.

Anyway, thanks a bunch for all your insightful feedback Andy!

Signature · 1 line
Josh
#5 lore raw

Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

Quoted 12 lines · Josh Poimboeuf
Quoted 11 lines · Andy Lutomirski
Quoted 9 lines · Josh Poimboeuf

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.

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

Quoted 6 lines
Quoted 2 lines

This issue might be worth bringing up on the gcc and binutils lists. If we need better toolchain support, let's ask for it.

I think we found a good solution for this. See my update at:

https://lkml.kernel.org/r/[email protected]

Does that force frame pointer generation? If so, then once we have a real kernel unwinder, we might want a non-frame-pointer-forcing version for better code generation. (That can wait, of course.)

Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
Documentation/stack-validation.txt new file +10 -0
+
+   This is a context switch instruction like sysenter or sysret.  Such
+   instructions aren't allowed in a callable function, and are most
+   likely part of kernel entry code.
+
+   If the instruction isn't actually in a callable function, change
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
Quoted 8 lines

Documentation/stack-validation.txt line 136

Quoted 6 lines
Documentation/stack-validation.txt new file +6 -0
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?

This is a great point. With CONFIG_FRAME_POINTER it's not a problem, because it adds -fno-optimize-sibling-calls.

Quoted 6 lines

Without it, I think stackvalidate would spit out a ton of "jump to outside file" warnings.

I haven't yet looked at the details of how exactly sibling calls work. I'd assume they're disabled because they break frame pointers somehow. Any idea if they'd also break DWARF CFI stack traces?

They'll certainly prevent unwinding from finding the pre-optimization caller, but the rest of unwinding should work. I don't know why we turn it off, though.

You might want special-case jump-out-of-translation-unit to be okay if the stack frame is in its initial state. That is:

func:
jmp elsewhere

could be considered okay, as could:

func:
push %rax
pop %rax
jmp elsewhere

and similar.

Quoted 8 lines

I probably need to do some digging there. If sibling calls don't break CFI stack traces and we end up needing them, stackvalidate might need to analyze the entire kernel image at once instead of its current per-.o checking.

Anyway, thanks a bunch for all your insightful feedback Andy!

I'm just pretending to be insightful :)

--Andy

#6 lore raw

Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

Quoted 16 lines · Andy Lutomirski
Quoted 15 lines · Josh Poimboeuf
Quoted 13 lines · Andy Lutomirski
Quoted 9 lines · Josh Poimboeuf

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.

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

This issue might be worth bringing up on the gcc and binutils lists. If we need better toolchain support, let's ask for it.

Quoted 7 lines
Quoted 3 lines

I think we found a good solution for this. See my update at:

https://lkml.kernel.org/r/[email protected]

Does that force frame pointer generation? If so, then once we have a real kernel unwinder, we might want a non-frame-pointer-forcing version for better code generation. (That can wait, of course.)

I strongly doubt it would force frame pointer generation if -fomit-frame-pointer is set. But I'll verify :-)

Quoted 39 lines
Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
Documentation/stack-validation.txt new file +10 -0
+
+   This is a context switch instruction like sysenter or sysret.  Such
+   instructions aren't allowed in a callable function, and are most
+   likely part of kernel entry code.
+
+   If the instruction isn't actually in a callable function, change
+   ENDPROC to END.
+
+
+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function
Quoted 8 lines

Documentation/stack-validation.txt line 136

Quoted 6 lines
Documentation/stack-validation.txt new file +6 -0
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?

This is a great point. With CONFIG_FRAME_POINTER it's not a problem, because it adds -fno-optimize-sibling-calls.

Quoted 6 lines

Without it, I think stackvalidate would spit out a ton of "jump to outside file" warnings.

I haven't yet looked at the details of how exactly sibling calls work. I'd assume they're disabled because they break frame pointers somehow. Any idea if they'd also break DWARF CFI stack traces?

They'll certainly prevent unwinding from finding the pre-optimization caller, but the rest of unwinding should work. I don't know why we turn it off, though.

You might want special-case jump-out-of-translation-unit to be okay if the stack frame is in its initial state. That is:

func:
jmp elsewhere

Quoted 8 lines

could be considered okay, as could:

func:
push %rax
pop %rax
jmp elsewhere

and similar.

Ah, nice idea. That might cover all the cases. I'll try it.

Quoted 11 lines
Quoted 9 lines

I probably need to do some digging there. If sibling calls don't break CFI stack traces and we end up needing them, stackvalidate might need to analyze the entire kernel image at once instead of its current per-.o checking.

Anyway, thanks a bunch for all your insightful feedback Andy!

I'm just pretending to be insightful :)

Insightful or not, your comments have been very helpful!

Signature · 1 line
Josh
#7 lore raw

Re: [PATCH v6 2/4] x86/stackvalidate: Compile-time stack validation

In reply to Andy Lutomirski
Quoted 16 lines · Andy Lutomirski
Quoted 15 lines · Josh Poimboeuf
Quoted 13 lines · Andy Lutomirski
Quoted 9 lines · Josh Poimboeuf

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.

Currently with my Fedora config it's reporting over 1400 warnings, but most of them are duplicates. The warnings affect 37 .c files and 18 .S files. The C file warnings are generally due to inline assembly, which doesn't seem to play nice with frame pointers.

This issue might be worth bringing up on the gcc and binutils lists. If we need better toolchain support, let's ask for it.

Quoted 7 lines
Quoted 3 lines

I think we found a good solution for this. See my update at:

https://lkml.kernel.org/r/[email protected]

Does that force frame pointer generation? If so, then once we have a real kernel unwinder, we might want a non-frame-pointer-forcing version for better code generation. (That can wait, of course.)

I got some clarification on this solution of adding the stack pointer to the output operand of the asm() statement.

If frame pointers are enabled, it forces frame pointer generation and ensures the stack frame is set up before the asm() code. If frame pointers are disabled, it appears to have no effect on the generated code. So it's looking good for now and for the future.

Quoted 30 lines
Quoted 13 lines
Quoted 9 lines

Documentation/stack-validation.txt line 136

Quoted 7 lines

+6. stackvalidate: asm_file.o: func()+0x26: jump to outside file from callable function

Documentation/stack-validation.txt new file +6 -0
+   or
+   stackvalidate: asm_file.o: func()+0xd9: jump to dynamic address from callable function
+
+   These are constraints imposed by stackvalidate so that it can
+   properly analyze all jump targets.  Dynamic jump targets and jumps to
+   code in other object files aren't allowed.

Does this not trigger due to optimized sibling calls to different files?

This is a great point. With CONFIG_FRAME_POINTER it's not a problem, because it adds -fno-optimize-sibling-calls.

Quoted 6 lines

Without it, I think stackvalidate would spit out a ton of "jump to outside file" warnings.

I haven't yet looked at the details of how exactly sibling calls work. I'd assume they're disabled because they break frame pointers somehow. Any idea if they'd also break DWARF CFI stack traces?

They'll certainly prevent unwinding from finding the pre-optimization caller, but the rest of unwinding should work. I don't know why we turn it off, though.

You might want special-case jump-out-of-translation-unit to be okay if the stack frame is in its initial state. That is:

func:
jmp elsewhere

Quoted 8 lines

could be considered okay, as could:

func:
push %rax
pop %rax
jmp elsewhere

and similar.

This approach works great for all sibling calls. Wrapping up v7 now.

Signature · 1 line
Josh
#8 lore raw

PATCH v6 3/4 x86/stackvalidate: Add file and directory ignores

In reply to Josh Poimboeuf

Tell stackvalidate to skip validation of the following code:

- boot image
- vdso image
- kexec purgatory
- realmode
- efi libstub

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.

---
 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 +
 7 files changed, 16 insertions(+), 5 deletions(-)
arch/x86/boot/Makefile +2 -1
index 57bbf2f..f3f0c35 100644
@@ -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
 
arch/x86/boot/compressed/Makefile +2 -1
index 0a291cd..530a46f 100644
@@ -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
arch/x86/entry/vdso/Makefile +4 -1
index 96c0617..ccaf812 100644
@@ -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
arch/x86/purgatory/Makefile +2 -0
index 2c835e3..a736c19 100644
@@ -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)
arch/x86/realmode/Makefile +3 -1
index e02c2c6..7a2d4df 100644
@@ -6,7 +6,9 @@
 # for more details.
 #
 #
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
+
 subdir- := rm
 
 obj-y += init.o
arch/x86/realmode/rm/Makefile +2 -1
index 2730d77..d462a57 100644
@@ -6,7 +6,8 @@
 # for more details.
 #
 #
-KASAN_SANITIZE := n
+KASAN_SANITIZE	:= n
+STACKVALIDATE	:= n
 
 always := realmode.bin realmode.relocs
 
drivers/firmware/efi/libstub/Makefile +1 -0
index 816dbe9..b392f3f 100644
@@ -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
Signature · 1 line
2.1.0
#9 lore raw

PATCH v6 4/4 stackvalidate: Add ignore macros

In reply to 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.

---
 include/linux/stackvalidate.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/linux/stackvalidate.h
include/linux/stackvalidate.h new file +38 -0
new file mode 100644
index 0000000..30d4a60
@@ -0,0 +1,38 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#ifndef __ASSEMBLY__
+
+/*
+ * 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) \
+	void __attribute__((section("__stackvalidate_ignore_func,\"ae\"#"))) \
+		*__stackvalidate_ignore_func_##_func = _func
+
+#else /* __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
+	.if CONFIG_STACK_VALIDATION
+		163:
+		.pushsection __stackvalidate_ignore_insn, "ae"
+		_ASM_ALIGN
+		.long 163b - .
+		.popsection
+	.endif
+.endm
+
+#endif /* __ASSEMBLY__ */
+#endif /* _LINUX_STACKVALIDATE_H */
Signature · 1 line
2.1.0
#10 lore raw

Re: [PATCH v6 4/4] stackvalidate: Add ignore macros

Quoted 22 lines · 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.

---
 include/linux/stackvalidate.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/linux/stackvalidate.h
include/linux/stackvalidate.h new file +6 -0
new file mode 100644
index 0000000..30d4a60
@@ -0,0 +1,38 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#ifndef __ASSEMBLY__
+
+/*

include/linux/stackvalidate.h line 16

Quoted 10 lines
include/linux/stackvalidate.h new file +10 -0
+ * 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) \
+       void __attribute__((section("__stackvalidate_ignore_func,\"ae\"#"))) \
+               *__stackvalidate_ignore_func_##_func = _func
+

static? Otherwise there's some risk that ignoring a static function will cause a duplicate symbol. Alternatively you could generate a more unique name.

Also, should the linker script be updated to discard this section?

include/linux/stackvalidate.h line 29

Quoted 13 lines
include/linux/stackvalidate.h new file +13 -0
+#else /* __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
+       .if CONFIG_STACK_VALIDATION
+               163:

Some day we should come up with a better way to do this than using a random three-digit number. I wonder if something like .Ltemp_\@ could be used for this purpose.

--Andy

#11 lore raw

Re: [PATCH v6 4/4] stackvalidate: Add ignore macros

Quoted 38 lines · Andy Lutomirski
Quoted 22 lines · 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.

---
 include/linux/stackvalidate.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/linux/stackvalidate.h
include/linux/stackvalidate.h new file +6 -0
new file mode 100644
index 0000000..30d4a60
@@ -0,0 +1,38 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#ifndef __ASSEMBLY__
+
+/*

include/linux/stackvalidate.h line 16

Quoted 10 lines
include/linux/stackvalidate.h new file +10 -0
+ * 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) \
+       void __attribute__((section("__stackvalidate_ignore_func,\"ae\"#"))) \
+               *__stackvalidate_ignore_func_##_func = _func
+

static? Otherwise there's some risk that ignoring a static function will cause a duplicate symbol. Alternatively you could generate a more unique name.

Quoted 1 line

Also, should the linker script be updated to discard this section?

It validates per individual object file, so there shouldn't be a risk of duplicate symbols. The 'e' flag in ,\"ae\"#" tells the linker to discard the section.

(The ,\"ae\"# stuff is a horrible hack, but it's the only way I could figure out how to set the section flags from C code. The '#' is used to comment out gcc's default arguments to the .section directive in favor of the "ae" flags.)

Quoted 9 lines
Quoted 9 lines
include/linux/stackvalidate.h new file +9 -0
+#else /* __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.
Quoted 8 lines

include/linux/stackvalidate.h line 29

Quoted 4 lines
include/linux/stackvalidate.h new file +4 -0
+ */
+.macro STACKVALIDATE_IGNORE_INSN
+       .if CONFIG_STACK_VALIDATION
+               163:

Some day we should come up with a better way to do this than using a random three-digit number. I wonder if something like .Ltemp_\@ could be used for this purpose.

Agreed. And .Ltemp_\@ sounds like a good idea. I'll try it.

Signature · 1 line
Josh
#12 lore raw

Re: [PATCH v6 4/4] stackvalidate: Add ignore macros

Quoted 41 lines · Josh Poimboeuf
Quoted 38 lines · Andy Lutomirski
Quoted 22 lines · 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.

---
 include/linux/stackvalidate.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/linux/stackvalidate.h
include/linux/stackvalidate.h new file +6 -0
new file mode 100644
index 0000000..30d4a60
@@ -0,0 +1,38 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#ifndef __ASSEMBLY__
+
+/*

include/linux/stackvalidate.h line 16

Quoted 10 lines
include/linux/stackvalidate.h new file +10 -0
+ * 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) \
+       void __attribute__((section("__stackvalidate_ignore_func,\"ae\"#"))) \
+               *__stackvalidate_ignore_func_##_func = _func
+

static? Otherwise there's some risk that ignoring a static function will cause a duplicate symbol. Alternatively you could generate a more unique name.

Quoted 1 line

Also, should the linker script be updated to discard this section?

Quoted 8 lines

It validates per individual object file, so there shouldn't be a risk of duplicate symbols. The 'e' flag in ,\"ae\"#" tells the linker to discard the section.

(The ,\"ae\"# stuff is a horrible hack, but it's the only way I could figure out how to set the section flags from C code. The '#' is used to comment out gcc's default arguments to the .section directive in favor of the "ae" flags.)

Oh, egads. FWIW, doing it in the linker script would probably be less hackish.

Does the linker discard before noticing duplicate symbols in that section?

Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
include/linux/stackvalidate.h new file +9 -0
+#else /* __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.
Quoted 8 lines

include/linux/stackvalidate.h line 29

Quoted 4 lines
include/linux/stackvalidate.h new file +4 -0
+ */
+.macro STACKVALIDATE_IGNORE_INSN
+       .if CONFIG_STACK_VALIDATION
+               163:

Some day we should come up with a better way to do this than using a random three-digit number. I wonder if something like .Ltemp_\@ could be used for this purpose.

Quoted 1 line

Agreed. And .Ltemp_\@ sounds like a good idea. I'll try it.

:)

--Andy

#13 lore raw

Re: [PATCH v6 4/4] stackvalidate: Add ignore macros

Quoted 46 lines · Andy Lutomirski
Quoted 45 lines · Josh Poimboeuf
Quoted 38 lines · Andy Lutomirski
Quoted 22 lines · 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.

---
 include/linux/stackvalidate.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/linux/stackvalidate.h
include/linux/stackvalidate.h new file +6 -0
new file mode 100644
index 0000000..30d4a60
@@ -0,0 +1,38 @@
+#ifndef _LINUX_STACKVALIDATE_H
+#define _LINUX_STACKVALIDATE_H
+
+#ifndef __ASSEMBLY__
+
+/*

include/linux/stackvalidate.h line 16

Quoted 10 lines
include/linux/stackvalidate.h new file +10 -0
+ * 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) \
+       void __attribute__((section("__stackvalidate_ignore_func,\"ae\"#"))) \
+               *__stackvalidate_ignore_func_##_func = _func
+

static? Otherwise there's some risk that ignoring a static function will cause a duplicate symbol. Alternatively you could generate a more unique name.

Quoted 1 line

Also, should the linker script be updated to discard this section?

It validates per individual object file, so there shouldn't be a risk of duplicate symbols. The 'e' flag in ,\"ae\"#" tells the linker to discard the section.

Quoted 6 lines
Quoted 4 lines

(The ,\"ae\"# stuff is a horrible hack, but it's the only way I could figure out how to set the section flags from C code. The '#' is used to comment out gcc's default arguments to the .section directive in favor of the "ae" flags.)

Oh, egads. FWIW, doing it in the linker script would probably be less hackish.

Yeah. I'll try that.

Quoted 1 line

Does the linker discard before noticing duplicate symbols in that section?

I think it does, but I'll verify. I deliberately made it global so the compiler doesn't discard the symbol for being unused. But I can use the unused attribute to avoid that.

Signature · 1 line
Josh
#14 lore raw
In reply to Josh Poimboeuf
Quoted 2 lines · Josh Poimboeuf

Also posting a listing of the reported warnings in a reply to this email.

These are the currently reported stackvalidate warnings on tip/master with my Fedora-based config. There were over 1400 warnings, in 37 .c files and 18 .S files. A lot of the repeat warnings have been removed to keep it short.

stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_set_key()+0x128: call without frame pointer save/setup ...removed 29 lines...
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_set_key()+0xb9: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_enc()+0xa: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_dec()+0x11: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_enc()+0x57: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_enc()+0x27: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_dec()+0x5f: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_dec()+0x2f: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_cbc_enc()+0x1b: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_cbc_dec()+0x8a: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_cbc_dec()+0x43: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: _aesni_inc_init(): can't find starting instruction
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ctr_enc()+0x15: call without frame pointer save/setup ...removed 6 lines...
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ctr_enc()+0x51: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_xts_crypt8()+0xda: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_xts_crypt8()+0x1f1: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/aesni-intel_asm.o: _aesni_inc_init()+0x24: return instruction outside of a callable function
stackvalidate: arch/x86/crypto/ghash-clmulni-intel_asm.o: clmul_ghash_mul()+0x13: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/ghash-clmulni-intel_asm.o: clmul_ghash_update()+0x28: call without frame pointer save/setup
stackvalidate: arch/x86/crypto/crc32c-pcl-intel-asm_64.o: crc_pcl()+0x11dd: can't decode instruction
stackvalidate: arch/x86/entry/entry_64.o: native_usergs_sysret64()+0x3: context switch from callable instruction
stackvalidate: arch/x86/entry/entry_64.o: .entry.text+0x329: return instruction outside of a callable function
stackvalidate: arch/x86/entry/entry_64.o: .entry.text+0x1fe9: return instruction outside of a callable function
stackvalidate: arch/x86/entry/entry_64.o: .entry.text+0x2015: return instruction outside of a callable function
stackvalidate: arch/x86/entry/entry_64.o: .entry.text+0x2514: return instruction outside of a callable function
stackvalidate: arch/x86/entry/entry_64.o: .entry.text+0x2628: return instruction outside of a callable function
stackvalidate: arch/x86/entry/thunk_64.o: .text+0x53: return instruction outside of a callable function
stackvalidate: arch/x86/entry/entry_64_compat.o: native_usergs_sysret32()+0x3: context switch from callable instruction
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0xd: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x44e: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x455: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x45a: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x45f: jump to outside file from callable instruction
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0xe6: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x88: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x8f: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x10a: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x110: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x47a: jump to outside file from callable instruction
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x13a: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x14a: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x151: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0xcb: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0xd7: jump to dynamic address from callable instruction
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x179: jump to outside file from callable instruction
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x7b: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x1d0: call without frame pointer save/setup
stackvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x5ce: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0x0: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0xbb: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0x2b7: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/alternative.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/tsc.o: unsynchronized_tsc()+0x5f: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/process.o: set_tsc_mode()+0x55: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64()+0x55: jump to dynamic address from callable instruction
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel()+0x6: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel()+0x95: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel()+0x116: jump to outside file from callable instruction
stackvalidate: arch/x86/kernel/apic/apic.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/cpu/common.o: debug_stack_reset()+0x70: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/cpu/common.o: debug_stack_reset()+0x61: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/cpu/common.o: debug_stack_reset()+0x31: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/cpu/amd.o: .text+0x0: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/cpu/perf_event_intel.o: __arch_hweight64()+0x1: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/cpu/mcheck/mce.o: mce_wrmsrl.constprop.30()+0x3f: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/kprobes/core.o: kretprobe_trampoline_holder()+0x1d: duplicate frame pointer save
stackvalidate: arch/x86/kernel/kprobes/core.o: kprobe_exceptions_notify()+0x2d: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/kprobes/core.o: kretprobe_trampoline_holder()+0x53: function has unreachable instruction
stackvalidate: arch/x86/kernel/kprobes/core.o: kretprobe_trampoline_holder()+0x54: function has unreachable instruction
stackvalidate: arch/x86/kernel/reboot.o: machine_real_restart()+0x5d: jump to dynamic address from callable instruction
stackvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0xd6: can't find call dest symbol at offset 0xdb
stackvalidate: arch/x86/kernel/paravirt.o: paravirt_end_context_switch()+0x3f: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/paravirt.o: paravirt_end_context_switch()+0x37: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/vsmp_64.o: .text+0x1d: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/vsmp_64.o: .text+0x3b: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/vsmp_64.o: .text+0x59: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/vsmp_64.o: .text+0x77: return instruction outside of a callable function
stackvalidate: arch/x86/kernel/head_64.o: .text: unexpected end of section
stackvalidate: arch/x86/kernel/head_64.o: start_cpu0()+0x13: context switch from callable instruction
stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler_common()+0xbb: jump to dynamic address from callable instruction ...removed 6 lines...
stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler_common()+0x42: call without frame pointer save/setup
stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x9: function has unreachable instruction ...removed 140 lines...
stackvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x11f: function has unreachable instruction
stackvalidate: arch/x86/mm/tlb.o: leave_mm()+0x5c: call without frame pointer save/setup
stackvalidate: arch/x86/mm/tlb.o: do_flush_tlb_all()+0x25: call without frame pointer save/setup
stackvalidate: arch/x86/mm/tlb.o: do_flush_tlb_all()+0x11: call without frame pointer save/setup
stackvalidate: arch/x86/mm/mmio-mod.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: arch/x86/net/bpf_jit.o: .text+0x18: return instruction outside of a callable function ...removed 7 lines...
stackvalidate: arch/x86/net/bpf_jit.o: .text+0x16d: return instruction outside of a callable function
stackvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call()+0x7c: call without frame pointer save/setup
stackvalidate: arch/x86/platform/uv/tlb_uv.o: ptc_seq_next()+0x1e: call without frame pointer save/setup
stackvalidate: arch/x86/platform/uv/uv_nmi.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: arch/x86/platform/uv/uv_nmi.o: bitmap_weight.constprop.9()+0x4: call without frame pointer save/setup
stackvalidate: arch/x86/xen/enlighten.o: xen_cpuid()+0x41: can't find jump dest instruction at .text+0x108
stackvalidate: arch/x86/xen/mmu.o: .text+0x1d: return instruction outside of a callable function
...removed 6 lines...
stackvalidate: arch/x86/xen/mmu.o: .text+0xef: return instruction outside of a callable function
stackvalidate: arch/x86/xen/irq.o: xen_halt()+0x0: call without frame pointer save/setup
stackvalidate: arch/x86/xen/irq.o: xen_halt()+0x1d: call without frame pointer save/setup
stackvalidate: arch/x86/xen/irq.o: .text+0x1d: return instruction outside of a callable function
stackvalidate: arch/x86/xen/irq.o: .text+0x3b: return instruction outside of a callable function
stackvalidate: arch/x86/xen/irq.o: .text+0x59: return instruction outside of a callable function
stackvalidate: arch/x86/xen/irq.o: .text+0x77: return instruction outside of a callable function
stackvalidate: arch/x86/xen/time.o: xen_vcpuop_set_mode()+0x36: call without frame pointer save/setup
stackvalidate: arch/x86/xen/time.o: xen_vcpuop_set_mode()+0x49: call without frame pointer save/setup
stackvalidate: arch/x86/xen/time.o: xen_vcpuop_set_mode()+0x1d: call without frame pointer save/setup
stackvalidate: arch/x86/xen/time.o: xen_timerop_set_mode()+0x26: call without frame pointer save/setup
stackvalidate: arch/x86/xen/xen-asm.o: .text+0x7f: return instruction outside of a callable function
stackvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall_target()+0xe: jump to outside file from callable instruction
stackvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall32_target()+0xe: jump to outside file from callable instruction
stackvalidate: arch/x86/xen/xen-asm_64.o: xen_sysenter_target()+0xe: jump to outside file from callable instruction
stackvalidate: arch/x86/xen/xen-asm_64.o: .text+0xa: return instruction outside of a callable function
stackvalidate: arch/x86/xen/smp.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: kernel/softirq.o: __tasklet_hi_schedule_first()+0x5: call without frame pointer save/setup
stackvalidate: kernel/pid.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: kernel/bpf/core.o: __bpf_prog_run()+0x5c: jump to dynamic address from callable instruction
stackvalidate: kernel/bpf/core.o: __bpf_prog_run()+0x60: function has unreachable instruction
...removed 1092 lines...
stackvalidate: kernel/bpf/core.o: __bpf_prog_run()+0x12dc: function has unreachable instruction
stackvalidate: kernel/locking/lockdep.o: trace_hardirqs_off_caller()+0x31: call without frame pointer save/setup
stackvalidate: kernel/printk/printk.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: kernel/sched/core.o: sd_degenerate()+0x10: call without frame pointer save/setup
stackvalidate: kernel/sched/core.o: __schedule()+0x3d8: duplicate frame pointer save
stackvalidate: kernel/sched/core.o: __schedule()+0x415: jump to outside file from callable instruction
stackvalidate: kernel/sched/core.o: __schedule()+0x422: call without frame pointer save/setup
stackvalidate: kernel/sched/core.o: __schedule()+0x42a: frame pointer state mismatch
stackvalidate: kernel/sched/core.o: preempt_schedule_irq()+0x17: call without frame pointer save/setup
stackvalidate: kernel/sched/rt.o: set_cpus_allowed_rt()+0x1b: call without frame pointer save/setup
stackvalidate: kernel/irq_work.o: irq_work_run_list()+0x0: call without frame pointer save/setup
stackvalidate: mm/percpu.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: mm/slub.o: check_slab()+0x5: call without frame pointer save/setup
stackvalidate: mm/huge_memory.o: split_huge_page_address()+0x30: call without frame pointer save/setup
stackvalidate: mm/huge_memory.o: split_huge_page_address()+0x6d: call without frame pointer save/setup
stackvalidate: fs/buffer.o: __find_get_block()+0x5: call without frame pointer save/setup
stackvalidate: drivers/acpi/processor_idle.o: hweight_long()+0x1: call without frame pointer save/setup
stackvalidate: drivers/usb/host/xhci.o: xhci_count_num_new_endpoints.isra.23()+0x10: call without frame pointer save/setup
stackvalidate: drivers/xen/sys-hypervisor.o: pagesize_show()+0xf: call without frame pointer save/setup
stackvalidate: drivers/xen/sys-hypervisor.o: minor_show()+0xc: call without frame pointer save/setup
stackvalidate: drivers/xen/sys-hypervisor.o: major_show()+0xc: call without frame pointer save/setup
stackvalidate: arch/x86/power/hibernate_asm_64.o: .text+0x69: return instruction outside of a callable function
stackvalidate: arch/x86/power/hibernate_asm_64.o: .text+0x16d: return instruction outside of a callable function
stackvalidate: lib/percpu_counter.o: __percpu_counter_compare()+0x1e: call without frame pointer save/setup
stackvalidate: arch/x86/lib/msr-reg.o: rdmsr_safe_regs()+0x3c: frame pointer restore without save/setup
stackvalidate: arch/x86/lib/msr-reg.o: wrmsr_safe_regs()+0x3c: frame pointer restore without save/setup
stackvalidate: arch/x86/lib/copy_user_64.o: .fixup+0x2f: jump to outside file from callable instruction ...removed 5 lines...
stackvalidate: arch/x86/lib/copy_user_64.o: .fixup+0x3d: jump to outside file from callable instruction
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed()+0xf: call without frame pointer save/setup
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed()+0xe: call without frame pointer save/setup
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake()+0x12: call without frame pointer save/setup
stackvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake()+0xf: call without frame pointer save/setup

Signature · 1 line
Josh
#15 lore raw
In reply to Josh Poimboeuf
Quoted 6 lines · Josh Poimboeuf

I did some more looking and it turns out that inline assembly doesn't play nicely with frame pointers at all. If the inline asm is at the beginning of the function, gcc sometimes emits the inline asm code before setting up the frame pointer. That can break stack traces when the inline asm has a call instruction.

Quoted 7 lines

That turns out to be a very common problem. Stackvalidate found 37 C object files which break frame pointer rules, thanks to inline asm.

I don't know of a solution to this problem yet. Basically I think we need a way to ensure that gcc emits the frame pointer setup before inserting any inline asm (particularly when the inline asm has a call instruction).

A solution to this problem was posted by Segher Boessenkool in a related thread on the gcc mailing list:

https://gcc.gnu.org/ml/gcc/2015-07/msg00080.html

The suggestion is to use something like:

	register void *sp asm("%sp");
	asm volatile("call func" : "+r"(sp));

I can confirm that it seems to fix the issue. (I had tried something like this before, but I guess I wasn't able to get the incantation just right.)

Thanks to Jiri for the pointer to the thread, and Martin for raising the issue on the gcc list.

Signature · 1 line
Josh