Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1314601 > unrolled thread

[PATCH 00/33] Compile-time stack metadata validation

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/33] Compile-time stack metadata validation Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 25/33] x86/kvm: Set ELF function type for fastop functions Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
      Re: [PATCH 25/33] x86/kvm: Set ELF function type for fastop functions Paolo Bonzini <pbonzini@redhat.com> - 2016-01-22 11:10 +0100
    [PATCH 21/33] x86/uaccess: Add stack frame output operand in get_user inline asm Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 20/33] x86/asm/power: Create stack frames in hibernate_asm_64.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 12/33] x86/asm/crypto: Move jump_table to .rodata section Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 18/33] x86/asm: Create stack frames in rwsem functions Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 32/33] sched: Add __schedule() to stacktool whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 27/33] watchdog/hpwdt: Create stack frame in asminline_call() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 11/33] x86/asm/crypto: Move .Lbswap_mask data to .rodata section Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 29/33] x86/stacktool: Add directory and file whitelists Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 26/33] x86/kvm: Add stack frame dependency to test_cc() inline asm Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
      Re: [PATCH 26/33] x86/kvm: Add stack frame dependency to test_cc()  inline asm Paolo Bonzini <pbonzini@redhat.com> - 2016-01-22 11:10 +0100
        Re: [PATCH 26/33] x86/kvm: Add stack frame dependency to test_cc()  inline asm Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 17:10 +0100
          [PATCH v16.1 26/33] x86/kvm: Make test_cc() always inline Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 17:20 +0100
    [PATCH 28/33] x86/locking: Create stack frame in PV unlock Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 22/33] x86/asm/bpf: Annotate callable functions Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
    [PATCH 07/33] x86/asm/xen: Create stack frames in xen-asm.S Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 13/33] x86/asm/crypto: Simplify stack usage in sha-mb functions Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 02/33] kbuild/stacktool: Add CONFIG_STACK_VALIDATION option Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 04/33] x86/stacktool: Add STACKTOOL_IGNORE_FUNC macro Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 14/33] x86/asm/crypto: Don't use rbp as a scratch register Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 05/33] x86/xen: Add stack frame dependency to hypercall inline asm calls Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    [PATCH 06/33] x86/asm/xen: Set ELF function type for xen_adjust_exception_frame() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:10 +0100
    Re: [PATCH 00/33] Compile-time stack metadata validation Chris J Arges <chris.j.arges@canonical.com> - 2016-01-22 18:50 +0100
      Re: [PATCH 00/33] Compile-time stack metadata validation Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 20:20 +0100
        Re: [PATCH 00/33] Compile-time stack metadata validation Chris J Arges <chris.j.arges@canonical.com> - 2016-01-22 21:50 +0100
          Re: [PATCH 00/33] Compile-time stack metadata validation Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 21:50 +0100
            [PATCH 0/2] A few stacktool warning fixes. Chris J Arges <chris.j.arges@canonical.com> - 2016-01-22 22:50 +0100
              [PATCH 1/2] tools/stacktool: Add __reiserfs_panic to global_noreturns list Chris J Arges <chris.j.arges@canonical.com> - 2016-01-22 22:50 +0100
                Re: [PATCH 1/2] tools/stacktool: Add __reiserfs_panic to  global_noreturns list Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-25 16:10 +0100
              [PATCH 2/2] x86/kvm: Add output operand in vmx_handle_external_intr inline asm Chris J Arges <chris.j.arges@canonical.com> - 2016-01-22 22:50 +0100
                Re: [PATCH 2/2] x86/kvm: Add output operand in  vmx_handle_external_intr inline asm Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-25 16:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1314627 — [PATCH 04/33] x86/stacktool: Add STACKTOOL_IGNORE_FUNC macro

Add a new stacktool ignore macro, STACKTOOL_IGNORE_FUNC, which can be
used to tell stacktool to skip validation of a function.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 7ecbea9..80b26ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10190,6 +10190,7 @@ STACK METADATA VALIDATION
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
 S:	Supported
 F:	tools/stacktool/
+F:	include/linux/stacktool.h
 
 STAGING SUBSYSTEM
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 4f19942..c08c283c 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -333,7 +333,10 @@ SECTIONS
 
 	/* Sections to be discarded */
 	DISCARDS
-	/DISCARD/ : { *(.eh_frame) }
+	/DISCARD/ : {
+		*(.eh_frame)
+		*(__stacktool_ignore_*)
+	}
 }
 
 
diff --git a/include/linux/stacktool.h b/include/linux/stacktool.h
new file mode 100644
index 0000000..0d90db7
--- /dev/null
+++ b/include/linux/stacktool.h
@@ -0,0 +1,23 @@
+#ifndef _LINUX_STACKTOOL_H
+#define _LINUX_STACKTOOL_H
+
+#ifdef CONFIG_STACK_VALIDATION
+/*
+ * This C macro tells stacktool to ignore the function when doing stack
+ * metadata validation.  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 tools/stacktool/Documentation/stack-validation.txt.
+ */
+#define STACKTOOL_IGNORE_FUNC(_func) \
+	static void __used __section(__stacktool_ignore_func) \
+		*__stacktool_ignore_func_##_func = _func
+
+#else /* !CONFIG_STACK_VALIDATION */
+
+#define STACKTOOL_IGNORE_FUNC(_func)
+
+#endif /* CONFIG_STACK_VALIDATION */
+
+#endif /* _LINUX_STACKTOOL_H */
-- 
2.4.3

[toc] | [prev] | [next] | [standalone]

#1314629 — [PATCH 14/33] x86/asm/crypto: Don't use rbp as a scratch register

The frame pointer (rbp) is getting clobbered in
sha1_mb_mgr_submit_avx2() before a function call, which can mess up
stack traces.  Use r12 instead.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S b/arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S
index a5a14c62..c3b9447 100644
--- a/arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S
+++ b/arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S
@@ -86,8 +86,8 @@ job_rax         = %rax
 len             = %rax
 DWORD_len	= %eax
 
-lane            = %rbp
-tmp3            = %rbp
+lane            = %r12
+tmp3            = %r12
 
 tmp             = %r9
 DWORD_tmp	= %r9d
@@ -99,7 +99,7 @@ lane_data       = %r10
 # arg 2 : rdx : job
 ENTRY(sha1_mb_mgr_submit_avx2)
 	push	%rbx
-	push	%rbp
+	push	%r12
 
 	mov     _unused_lanes(state), unused_lanes
 	mov	unused_lanes, lane
@@ -190,7 +190,7 @@ len_is_0:
 	movl    DWORD_tmp, _result_digest+1*16(job_rax)
 
 return:
-	pop	%rbp
+	pop	%r12
 	pop	%rbx
 	ret
 
-- 
2.4.3

[toc] | [prev] | [next] | [standalone]

#1314631 — [PATCH 05/33] x86/xen: Add stack frame dependency to hypercall inline asm calls

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>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.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 3bcdcc8..a12a047 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

[toc] | [prev] | [next] | [standalone]

#1314632 — [PATCH 06/33] x86/asm/xen: Set ELF function type for xen_adjust_exception_frame()

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

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

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/xen-asm_64.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index cc8acc4..c3df431 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -26,6 +26,7 @@ ENTRY(xen_adjust_exception_frame)
 	mov 8+0(%rsp), %rcx
 	mov 8+8(%rsp), %r11
 	ret $16
+ENDPROC(xen_adjust_exception_frame)
 
 hypercall_iret = hypercall_page + __HYPERVISOR_iret * 32
 /*
-- 
2.4.3

[toc] | [prev] | [next] | [standalone]

#1315172

On Thu, Jan 21, 2016 at 04:49:04PM -0600, Josh Poimboeuf wrote:
> This is v16 of the compile-time stack metadata validation patch set,
> along with proposed fixes for most of the warnings it found.  It's based
> on the tip/master branch.
>
Josh,

Looks good, with my config [1] I do still get a few warnings building
linux/linux-next.

Here are the warnings:
$ grep ^stacktool build.log | grep -v staging
stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup
stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch
stacktool: fs/reiserfs/ibalance.o: .text: unexpected end of section
stacktool: fs/reiserfs/tail_conversion.o: .text: unexpected end of section

For vmx_handle_external_intr, I'm wondering if ignoring this function is the
best option.

--

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e2951b6..d19dfb2 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/tboot.h>
 #include <linux/hrtimer.h>
+#include <linux/stacktool.h>
 #include "kvm_cache_regs.h"
 #include "x86.h"
 
@@ -8398,6 +8399,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
        } else
                local_irq_enable();
 }
+STACKTOOL_IGNORE_FUNC(vmx_handle_external_intr);
 
 static bool vmx_has_high_real_mode_segbase(void)
 {

--chris

[1] http://paste.ubuntu.com/14599083/

 
> v15 can be found here:
> 
>   https://lkml.kernel.org/r/cover.1450442274.git.jpoimboe@redhat.com
> 
> For more information about the motivation behind this patch set, and
> more details about what it does, see the first patch changelog and
> tools/stacktool/Documentation/stack-validation.txt.
> 
> Patches 1-4 add stacktool and integrate it into the kernel build.
> 
> Patches 5-28 are some proposed fixes for several of the warnings
> reported by stacktool.  They've been compile-tested and boot-tested in a
> VM, but I haven't attempted any meaningful testing for many of them.
> 
> Patches 29-33 add some directories, files, and functions to the
> stacktool whitelist in order to silence false positive warnings.
> 
> v16:
> - fix all allyesconfig warnings, except for staging
> - get rid of STACKTOOL_IGNORE_INSN which is no longer needed
> - remove several whitelists in favor of automatically whitelisting any
>   function with a special instruction like ljmp, lret, or vmrun
> - split up stacktool patch into 3 parts as suggested by Ingo
> - update the global noreturn function list
> - detect noreturn function fallthroughs
> - skip weak functions in noreturn call detection logic
> - add empty function check to noreturn logic
> - allow non-section rela symbols for __ex_table sections
> - support rare switch table case with jmpq *[addr](%rip)
> - don't warn on frame pointer restore without save
> - rearrange patch order a bit
> 
> v15:
> - restructure code for a new cmdline interface "stacktool check" using
>   the new subcommand framework in tools/lib/subcmd
> - fix 32 bit build fail (put __sp at end) in paravirt_types.h patch 10
>   which was reported by 0day
> 
> v14:
> - make tools/include/linux/list.h self-sufficient
> - create FRAME_OFFSET to allow 32-bit code to be able to access function
>   arguments on the stack
> - add FRAME_OFFSET usage in crypto patch 14/24: "Create stack frames in
>   aesni-intel_asm.S"
> - rename "index" -> "idx" to fix build with some compilers
> 
> v13:
> - LDFLAGS order fix from Chris J Arges
> - new warning fix patches from Chris J Arges
> - "--frame-pointer" -> "--check-frame-pointer"
> 
> v12:
> - rename "stackvalidate" -> "stacktool"
> - move from scripts/ to tools/:
>   - makefile rework
>   - make a copy of the x86 insn code (and warn if the code diverges)
>   - use tools/include/linux/list.h
> - move warning macros to a new warn.h file
> - change wording: "stack validation" -> "stack metadata validation"
> 
> v11:
> - attempt to answer the "why" question better in the documentation and
>   commit message
> - s/FP_SAVE/FRAME_BEGIN/ in documentation
> 
> 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: live-patching@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: Andrew Morton <akpm@linux-foundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> 
> Chris J Arges (1):
>   x86/uaccess: Add stack frame output operand in get_user inline asm
> 
> Josh Poimboeuf (32):
>   x86/stacktool: Compile-time stack metadata validation
>   kbuild/stacktool: Add CONFIG_STACK_VALIDATION option
>   x86/stacktool: Enable stacktool on x86_64
>   x86/stacktool: Add STACKTOOL_IGNORE_FUNC macro
>   x86/xen: Add stack frame dependency to hypercall inline asm calls
>   x86/asm/xen: Set ELF function type for xen_adjust_exception_frame()
>   x86/asm/xen: Create stack frames in xen-asm.S
>   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/asm/crypto: Move .Lbswap_mask data to .rodata section
>   x86/asm/crypto: Move jump_table to .rodata section
>   x86/asm/crypto: Simplify stack usage in sha-mb functions
>   x86/asm/crypto: Don't use rbp as a scratch register
>   x86/asm/crypto: Create stack frames in crypto functions
>   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
>   x86/asm/bpf: Annotate callable functions
>   x86/asm/bpf: Create stack frames in bpf_jit.S
>   x86/kprobes: Get rid of kretprobe_trampoline_holder()
>   x86/kvm: Set ELF function type for fastop functions
>   x86/kvm: Add stack frame dependency to test_cc() inline asm
>   watchdog/hpwdt: Create stack frame in asminline_call()
>   x86/locking: Create stack frame in PV unlock
>   x86/stacktool: Add directory and file whitelists
>   x86/xen: Add xen_cpuid() to stacktool whitelist
>   bpf: Add __bpf_prog_run() to stacktool whitelist
>   sched: Add __schedule() to stacktool whitelist
>   x86/kprobes: Add kretprobe_trampoline() to stacktool whitelist
> 
>  MAINTAINERS                                        |   6 +
>  Makefile                                           |   5 +-
>  arch/Kconfig                                       |   6 +
>  arch/x86/Kconfig                                   |   1 +
>  arch/x86/boot/Makefile                             |   1 +
>  arch/x86/boot/compressed/Makefile                  |   3 +-
>  arch/x86/crypto/aesni-intel_asm.S                  |  75 +-
>  arch/x86/crypto/camellia-aesni-avx-asm_64.S        |  15 +
>  arch/x86/crypto/camellia-aesni-avx2-asm_64.S       |  15 +
>  arch/x86/crypto/cast5-avx-x86_64-asm_64.S          |   9 +
>  arch/x86/crypto/cast6-avx-x86_64-asm_64.S          |  13 +
>  arch/x86/crypto/crc32c-pcl-intel-asm_64.S          |   8 +-
>  arch/x86/crypto/ghash-clmulni-intel_asm.S          |   5 +
>  arch/x86/crypto/serpent-avx-x86_64-asm_64.S        |  13 +
>  arch/x86/crypto/serpent-avx2-asm_64.S              |  13 +
>  arch/x86/crypto/sha-mb/sha1_mb_mgr_flush_avx2.S    |  35 +-
>  arch/x86/crypto/sha-mb/sha1_mb_mgr_submit_avx2.S   |  36 +-
>  arch/x86/crypto/twofish-avx-x86_64-asm_64.S        |  13 +
>  arch/x86/entry/Makefile                            |   4 +
>  arch/x86/entry/thunk_64.S                          |   4 +
>  arch/x86/entry/vdso/Makefile                       |   5 +-
>  arch/x86/include/asm/paravirt.h                    |   9 +-
>  arch/x86/include/asm/paravirt_types.h              |  18 +-
>  arch/x86/include/asm/qspinlock_paravirt.h          |   4 +
>  arch/x86/include/asm/uaccess.h                     |   5 +-
>  arch/x86/include/asm/xen/hypercall.h               |   5 +-
>  arch/x86/kernel/Makefile                           |   5 +
>  arch/x86/kernel/acpi/wakeup_64.S                   |   3 +
>  arch/x86/kernel/cpu/amd.c                          |   5 +-
>  arch/x86/kernel/kprobes/core.c                     |  59 +-
>  arch/x86/kernel/vmlinux.lds.S                      |   5 +-
>  arch/x86/kvm/emulate.c                             |  33 +-
>  arch/x86/lib/rwsem.S                               |  11 +-
>  arch/x86/net/bpf_jit.S                             |  48 +-
>  arch/x86/platform/efi/Makefile                     |   2 +
>  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                           |   3 +-
>  arch/x86/xen/xen-asm.S                             |  10 +-
>  arch/x86/xen/xen-asm_64.S                          |   1 +
>  drivers/firmware/efi/libstub/Makefile              |   1 +
>  drivers/watchdog/hpwdt.c                           |   8 +-
>  include/linux/stacktool.h                          |  23 +
>  kernel/bpf/core.c                                  |   2 +
>  kernel/sched/core.c                                |   2 +
>  lib/Kconfig.debug                                  |  12 +
>  scripts/Makefile.build                             |  38 +-
>  scripts/mod/Makefile                               |   2 +
>  tools/Makefile                                     |  14 +-
>  tools/stacktool/.gitignore                         |   2 +
>  tools/stacktool/Build                              |  13 +
>  tools/stacktool/Documentation/stack-validation.txt | 333 +++++++
>  tools/stacktool/Makefile                           |  60 ++
>  tools/stacktool/arch.h                             |  44 +
>  tools/stacktool/arch/x86/Build                     |  12 +
>  tools/stacktool/arch/x86/decode.c                  | 172 ++++
>  .../stacktool/arch/x86/insn/gen-insn-attr-x86.awk  | 387 ++++++++
>  tools/stacktool/arch/x86/insn/inat.c               |  97 ++
>  tools/stacktool/arch/x86/insn/inat.h               | 221 +++++
>  tools/stacktool/arch/x86/insn/inat_types.h         |  29 +
>  tools/stacktool/arch/x86/insn/insn.c               | 594 ++++++++++++
>  tools/stacktool/arch/x86/insn/insn.h               | 201 +++++
>  tools/stacktool/arch/x86/insn/x86-opcode-map.txt   | 984 ++++++++++++++++++++
>  tools/stacktool/builtin-check.c                    | 991 +++++++++++++++++++++
>  tools/stacktool/builtin.h                          |  22 +
>  tools/stacktool/elf.c                              | 403 +++++++++
>  tools/stacktool/elf.h                              |  79 ++
>  tools/stacktool/special.c                          | 193 ++++
>  tools/stacktool/special.h                          |  42 +
>  tools/stacktool/stacktool.c                        | 134 +++
>  tools/stacktool/warn.h                             |  60 ++
>  74 files changed, 5516 insertions(+), 189 deletions(-)
>  create mode 100644 include/linux/stacktool.h
>  create mode 100644 tools/stacktool/.gitignore
>  create mode 100644 tools/stacktool/Build
>  create mode 100644 tools/stacktool/Documentation/stack-validation.txt
>  create mode 100644 tools/stacktool/Makefile
>  create mode 100644 tools/stacktool/arch.h
>  create mode 100644 tools/stacktool/arch/x86/Build
>  create mode 100644 tools/stacktool/arch/x86/decode.c
>  create mode 100644 tools/stacktool/arch/x86/insn/gen-insn-attr-x86.awk
>  create mode 100644 tools/stacktool/arch/x86/insn/inat.c
>  create mode 100644 tools/stacktool/arch/x86/insn/inat.h
>  create mode 100644 tools/stacktool/arch/x86/insn/inat_types.h
>  create mode 100644 tools/stacktool/arch/x86/insn/insn.c
>  create mode 100644 tools/stacktool/arch/x86/insn/insn.h
>  create mode 100644 tools/stacktool/arch/x86/insn/x86-opcode-map.txt
>  create mode 100644 tools/stacktool/builtin-check.c
>  create mode 100644 tools/stacktool/builtin.h
>  create mode 100644 tools/stacktool/elf.c
>  create mode 100644 tools/stacktool/elf.h
>  create mode 100644 tools/stacktool/special.c
>  create mode 100644 tools/stacktool/special.h
>  create mode 100644 tools/stacktool/stacktool.c
>  create mode 100644 tools/stacktool/warn.h
> 
> -- 
> 2.4.3
> 

[toc] | [prev] | [next] | [standalone]

#1315233

On Fri, Jan 22, 2016 at 11:43:48AM -0600, Chris J Arges wrote:
> On Thu, Jan 21, 2016 at 04:49:04PM -0600, Josh Poimboeuf wrote:
> > This is v16 of the compile-time stack metadata validation patch set,
> > along with proposed fixes for most of the warnings it found.  It's based
> > on the tip/master branch.
> >
> Josh,
> 
> Looks good, with my config [1] I do still get a few warnings building
> linux/linux-next.
> 
> Here are the warnings:
> $ grep ^stacktool build.log | grep -v staging

Thanks for reporting these!

> stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup

This can be fixed by setting the stack pointer as an output operand for
the inline asm call in vmx_handle_external_intr().

Feel free to submit a patch, or I'll get around to it eventually.

> stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
> stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
> stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
> stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
> stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch

These are false positives.  Stacktool is confused by the use of a
"noreturn" function which it doesn't know about (__reiserfs_panic).

Unfortunately the only solution I currently have for dealing with global
noreturn functions is to just hard-code a list of them.  So the short
term fix would be to add "__reiserfs_panic" to the global_noreturns list
in tools/stacktool/builtin-check.c.

I'm still trying to figure out a better way to deal with this type of
issue, as it's a pain to have to keep a hard-coded list of noreturn
functions.  Unfortunately that info isn't available in the ELF.

> stacktool: fs/reiserfs/ibalance.o: .text: unexpected end of section
> stacktool: fs/reiserfs/tail_conversion.o: .text: unexpected end of section

For some reason I'm not able to recreate these warnings...  Can you
share one of the .o files?

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1315262

On Fri, Jan 22, 2016 at 01:14:47PM -0600, Josh Poimboeuf wrote:
> On Fri, Jan 22, 2016 at 11:43:48AM -0600, Chris J Arges wrote:
> > On Thu, Jan 21, 2016 at 04:49:04PM -0600, Josh Poimboeuf wrote:
> > > This is v16 of the compile-time stack metadata validation patch set,
> > > along with proposed fixes for most of the warnings it found.  It's based
> > > on the tip/master branch.
> > >
> > Josh,
> > 
> > Looks good, with my config [1] I do still get a few warnings building
> > linux/linux-next.
> > 
> > Here are the warnings:
> > $ grep ^stacktool build.log | grep -v staging
> 
> Thanks for reporting these!
> 
> > stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup
> 
> This can be fixed by setting the stack pointer as an output operand for
> the inline asm call in vmx_handle_external_intr().
> 
> Feel free to submit a patch, or I'll get around to it eventually.
> 
> > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
> > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
> > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
> > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
> > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch
> 
> These are false positives.  Stacktool is confused by the use of a
> "noreturn" function which it doesn't know about (__reiserfs_panic).
> 
> Unfortunately the only solution I currently have for dealing with global
> noreturn functions is to just hard-code a list of them.  So the short
> term fix would be to add "__reiserfs_panic" to the global_noreturns list
> in tools/stacktool/builtin-check.c.
> 
> I'm still trying to figure out a better way to deal with this type of
> issue, as it's a pain to have to keep a hard-coded list of noreturn
> functions.  Unfortunately that info isn't available in the ELF.
> 

Josh,
Ok I'll hack on the patches above.

> > stacktool: fs/reiserfs/ibalance.o: .text: unexpected end of section
> > stacktool: fs/reiserfs/tail_conversion.o: .text: unexpected end of section
> 
> For some reason I'm not able to recreate these warnings...  Can you
> share one of the .o files?
> 
> -- 
> Josh
> 

Binaries are here:
http://people.canonical.com/~arges/stacktool/

--chris

[toc] | [prev] | [next] | [standalone]

#1315269

On Fri, Jan 22, 2016 at 02:40:35PM -0600, Chris J Arges wrote:
> On Fri, Jan 22, 2016 at 01:14:47PM -0600, Josh Poimboeuf wrote:
> > On Fri, Jan 22, 2016 at 11:43:48AM -0600, Chris J Arges wrote:
> > > On Thu, Jan 21, 2016 at 04:49:04PM -0600, Josh Poimboeuf wrote:
> > > > This is v16 of the compile-time stack metadata validation patch set,
> > > > along with proposed fixes for most of the warnings it found.  It's based
> > > > on the tip/master branch.
> > > >
> > > Josh,
> > > 
> > > Looks good, with my config [1] I do still get a few warnings building
> > > linux/linux-next.
> > > 
> > > Here are the warnings:
> > > $ grep ^stacktool build.log | grep -v staging
> > 
> > Thanks for reporting these!
> > 
> > > stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup
> > 
> > This can be fixed by setting the stack pointer as an output operand for
> > the inline asm call in vmx_handle_external_intr().
> > 
> > Feel free to submit a patch, or I'll get around to it eventually.
> > 
> > > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
> > > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
> > > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
> > > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
> > > stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch
> > 
> > These are false positives.  Stacktool is confused by the use of a
> > "noreturn" function which it doesn't know about (__reiserfs_panic).
> > 
> > Unfortunately the only solution I currently have for dealing with global
> > noreturn functions is to just hard-code a list of them.  So the short
> > term fix would be to add "__reiserfs_panic" to the global_noreturns list
> > in tools/stacktool/builtin-check.c.
> > 
> > I'm still trying to figure out a better way to deal with this type of
> > issue, as it's a pain to have to keep a hard-coded list of noreturn
> > functions.  Unfortunately that info isn't available in the ELF.
> > 
> 
> Josh,
> Ok I'll hack on the patches above.
> 
> > > stacktool: fs/reiserfs/ibalance.o: .text: unexpected end of section
> > > stacktool: fs/reiserfs/tail_conversion.o: .text: unexpected end of section
> > 
> > For some reason I'm not able to recreate these warnings...  Can you
> > share one of the .o files?
> 
> Binaries are here:
> http://people.canonical.com/~arges/stacktool/

Thanks, looks like the same __reiserfs_panic() noreturn fix for those.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1315307 — [PATCH 0/2] A few stacktool warning fixes.

These patches fix a few warnings I saw testing stacktool v16.
I've done light testing on these by booting it on a machine and running
kvm-unit-tests on it.

Chris J Arges (2):
  tools/stacktool: Add __reiserfs_panic to global_noreturns list
  x86/kvm: Add output operand in vmx_handle_external_intr inline asm

 arch/x86/kvm/vmx.c              | 4 +++-
 tools/stacktool/builtin-check.c | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1315308 — [PATCH 1/2] tools/stacktool: Add __reiserfs_panic to global_noreturns list

The following false positives were noticed with stacktool:
  stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
  stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
  stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
  stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
  stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch

These all call into '__reiserfs_panic' which has a noreturn attribute. Add this
to the global list because this particular attribute cannot be determined from
reading the ELF object.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 tools/stacktool/builtin-check.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/stacktool/builtin-check.c b/tools/stacktool/builtin-check.c
index 5b0e91f..23fa93d2 100644
--- a/tools/stacktool/builtin-check.c
+++ b/tools/stacktool/builtin-check.c
@@ -139,6 +139,7 @@ static bool dead_end_function(struct stacktool_file *file, struct symbol *func)
 		"__module_put_and_exit",
 		"complete_and_exit",
 		"kvm_spurious_fault",
+		"__reiserfs_panic",
 	};
 
 	if (func->bind == STB_WEAK)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1316792 — Re: [PATCH 1/2] tools/stacktool: Add __reiserfs_panic to global_noreturns list

On Fri, Jan 22, 2016 at 03:44:37PM -0600, Chris J Arges wrote:
> The following false positives were noticed with stacktool:
>   stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: return without frame pointer restore
>   stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x89: duplicate frame pointer save
>   stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x8a: duplicate frame pointer setup
>   stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x9e: frame pointer state mismatch
>   stacktool: fs/reiserfs/namei.o: set_de_name_and_namelen()+0x0: frame pointer state mismatch
> 
> These all call into '__reiserfs_panic' which has a noreturn attribute. Add this
> to the global list because this particular attribute cannot be determined from
> reading the ELF object.
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

> ---
>  tools/stacktool/builtin-check.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/stacktool/builtin-check.c b/tools/stacktool/builtin-check.c
> index 5b0e91f..23fa93d2 100644
> --- a/tools/stacktool/builtin-check.c
> +++ b/tools/stacktool/builtin-check.c
> @@ -139,6 +139,7 @@ static bool dead_end_function(struct stacktool_file *file, struct symbol *func)
>  		"__module_put_and_exit",
>  		"complete_and_exit",
>  		"kvm_spurious_fault",
> +		"__reiserfs_panic",
>  	};
>  
>  	if (func->bind == STB_WEAK)
> -- 
> 2.5.0
> 

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1315309 — [PATCH 2/2] x86/kvm: Add output operand in vmx_handle_external_intr inline asm

Stacktool generates the following warning:
  stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup

By adding the stackpointer as an output operand, this patch ensures that a
stack frame is created when CONFIG_FRAME_POINTER is enabled for the inline
assmebly statement.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 arch/x86/kvm/vmx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e2951b6..e153522 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8356,6 +8356,7 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
 static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
 {
 	u32 exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
+	register void *__sp asm(_ASM_SP);
 
 	/*
 	 * If external interrupt exists, IF bit is set in rflags/eflags on the
@@ -8388,8 +8389,9 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
 			"call *%[entry]\n\t"
 			:
 #ifdef CONFIG_X86_64
-			[sp]"=&r"(tmp)
+			[sp]"=&r"(tmp),
 #endif
+			"+r"(__sp)
 			:
 			[entry]"r"(entry),
 			[ss]"i"(__KERNEL_DS),
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1316793 — Re: [PATCH 2/2] x86/kvm: Add output operand in vmx_handle_external_intr inline asm

On Fri, Jan 22, 2016 at 03:44:38PM -0600, Chris J Arges wrote:
> Stacktool generates the following warning:
>   stacktool: arch/x86/kvm/vmx.o: vmx_handle_external_intr()+0x67: call without frame pointer save/setup
> 
> By adding the stackpointer as an output operand, this patch ensures that a
> stack frame is created when CONFIG_FRAME_POINTER is enabled for the inline
> assmebly statement.
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

> ---
>  arch/x86/kvm/vmx.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e2951b6..e153522 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8356,6 +8356,7 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
>  static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>  {
>  	u32 exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
> +	register void *__sp asm(_ASM_SP);
>  
>  	/*
>  	 * If external interrupt exists, IF bit is set in rflags/eflags on the
> @@ -8388,8 +8389,9 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
>  			"call *%[entry]\n\t"
>  			:
>  #ifdef CONFIG_X86_64
> -			[sp]"=&r"(tmp)
> +			[sp]"=&r"(tmp),
>  #endif
> +			"+r"(__sp)
>  			:
>  			[entry]"r"(entry),
>  			[ss]"i"(__KERNEL_DS),
> -- 
> 2.5.0
> 

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web