Skip to content

Commit

Permalink
x86/ftrace: Fix ORC unwinding from ftrace handlers
Browse files Browse the repository at this point in the history
Steven Rostedt discovered that the ftrace stack tracer is broken when
it's used with the ORC unwinder.  The problem is that objtool is
instructed by the Makefile to ignore the ftrace_64.S code, so it doesn't
generate any ORC data for it.

Fix it by making the asm code objtool-friendly:

- Objtool doesn't like the fact that save_mcount_regs pushes RBP at the
  beginning, but it's never restored (directly, at least).  So just skip
  the original RBP push, which is only needed for frame pointers anyway.

- Annotate some functions as normal callable functions with
  ENTRY/ENDPROC.

- Add an empty unwind hint to return_to_handler().  The return address
  isn't on the stack, so there's nothing ORC can do there.  It will just
  punt in the unlikely case it tries to unwind from that code.

With all that fixed, remove the OBJECT_FILES_NON_STANDARD Makefile
annotation so objtool can read the file.

Link: http://lkml.kernel.org/r/20180123040746.ih4ep3tk4pbjvg7c@treble

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
  • Loading branch information
jpoimboe authored and rostedt committed Jan 23, 2018
1 parent 0c5b9b5 commit e2ac83d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
5 changes: 4 additions & 1 deletion arch/x86/kernel/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ KASAN_SANITIZE_stacktrace.o := n
KASAN_SANITIZE_paravirt.o := n

OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_test_nx.o := y
OBJECT_FILES_NON_STANDARD_paravirt_patch_$(BITS).o := y

ifdef CONFIG_FRAME_POINTER
OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o := y
endif

# If instrumentation of this dir is enabled, boot hangs during first second.
# Probably could be more selective here, but note that files related to irqs,
# boot, dumpstack/stacktrace, etc are either non-interesting or can lead to
Expand Down
24 changes: 15 additions & 9 deletions arch/x86/kernel/ftrace_64.S
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <asm/ftrace.h>
#include <asm/export.h>
#include <asm/nospec-branch.h>
#include <asm/unwind_hints.h>

.code64
.section .entry.text, "ax"
Expand All @@ -20,7 +21,6 @@ EXPORT_SYMBOL(__fentry__)
EXPORT_SYMBOL(mcount)
#endif

/* All cases save the original rbp (8 bytes) */
#ifdef CONFIG_FRAME_POINTER
# ifdef CC_USING_FENTRY
/* Save parent and function stack frames (rip and rbp) */
Expand All @@ -31,7 +31,7 @@ EXPORT_SYMBOL(mcount)
# endif
#else
/* No need to save a stack frame */
# define MCOUNT_FRAME_SIZE 8
# define MCOUNT_FRAME_SIZE 0
#endif /* CONFIG_FRAME_POINTER */

/* Size of stack used to save mcount regs in save_mcount_regs */
Expand Down Expand Up @@ -64,10 +64,10 @@ EXPORT_SYMBOL(mcount)
*/
.macro save_mcount_regs added=0

/* Always save the original rbp */
#ifdef CONFIG_FRAME_POINTER
/* Save the original rbp */
pushq %rbp

#ifdef CONFIG_FRAME_POINTER
/*
* Stack traces will stop at the ftrace trampoline if the frame pointer
* is not set up properly. If fentry is used, we need to save a frame
Expand Down Expand Up @@ -105,7 +105,11 @@ EXPORT_SYMBOL(mcount)
* Save the original RBP. Even though the mcount ABI does not
* require this, it helps out callers.
*/
#ifdef CONFIG_FRAME_POINTER
movq MCOUNT_REG_SIZE-8(%rsp), %rdx
#else
movq %rbp, %rdx
#endif
movq %rdx, RBP(%rsp)

/* Copy the parent address into %rsi (second parameter) */
Expand Down Expand Up @@ -148,7 +152,7 @@ EXPORT_SYMBOL(mcount)

ENTRY(function_hook)
retq
END(function_hook)
ENDPROC(function_hook)

ENTRY(ftrace_caller)
/* save_mcount_regs fills in first two parameters */
Expand Down Expand Up @@ -184,7 +188,7 @@ GLOBAL(ftrace_graph_call)
/* This is weak to keep gas from relaxing the jumps */
WEAK(ftrace_stub)
retq
END(ftrace_caller)
ENDPROC(ftrace_caller)

ENTRY(ftrace_regs_caller)
/* Save the current flags before any operations that can change them */
Expand Down Expand Up @@ -255,7 +259,7 @@ GLOBAL(ftrace_regs_caller_end)

jmp ftrace_epilogue

END(ftrace_regs_caller)
ENDPROC(ftrace_regs_caller)


#else /* ! CONFIG_DYNAMIC_FTRACE */
Expand Down Expand Up @@ -313,9 +317,10 @@ ENTRY(ftrace_graph_caller)
restore_mcount_regs

retq
END(ftrace_graph_caller)
ENDPROC(ftrace_graph_caller)

GLOBAL(return_to_handler)
ENTRY(return_to_handler)
UNWIND_HINT_EMPTY
subq $24, %rsp

/* Save the return values */
Expand All @@ -330,4 +335,5 @@ GLOBAL(return_to_handler)
movq (%rsp), %rax
addq $24, %rsp
JMP_NOSPEC %rdi
END(return_to_handler)
#endif

0 comments on commit e2ac83d

Please sign in to comment.