Commit 71e308a239c098673570d0b417d42262bb535909

Authored by Steven Rostedt
Committed by Steven Rostedt
1 parent eb4a03780d

function-graph: add stack frame test

In case gcc does something funny with the stack frames, or the return
from function code, we would like to detect that.

An arch may implement passing of a variable that is unique to the
function and can be saved on entering a function and can be tested
when exiting the function. Usually the frame pointer can be used for
this purpose.

This patch also implements this for x86. Where it passes in the stack
frame of the parent function, and will test that frame on exit.

There was a case in x86_32 with optimize for size (-Os) where, for a
few functions, gcc would align the stack frame and place a copy of the
return address into it. The function graph tracer modified the copy and
not the actual return address. On return from the funtion, it did not go
to the tracer hook, but returned to the parent. This broke the function
graph tracer, because the return of the parent (where gcc did not do
this funky manipulation) returned to the location that the child function
was suppose to. This caused strange kernel crashes.

This test detected the problem and pointed out where the issue was.

This modifies the parameters of one of the functions that the arch
specific code calls, so it includes changes to arch code to accommodate
the new prototype.

Note, I notice that the parsic arch implements its own push_return_trace.
This is now a generic function and the ftrace_push_return_trace should be
used instead. This patch does not touch that code.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Showing 9 changed files with 53 additions and 9 deletions Side-by-side Diff

arch/powerpc/kernel/ftrace.c
... ... @@ -605,7 +605,7 @@
605 605 return;
606 606 }
607 607  
608   - if (ftrace_push_return_trace(old, self_addr, &trace.depth) == -EBUSY) {
  608 + if (ftrace_push_return_trace(old, self_addr, &trace.depth, 0) == -EBUSY) {
609 609 *parent = old;
610 610 return;
611 611 }
arch/s390/kernel/ftrace.c
... ... @@ -190,7 +190,7 @@
190 190 goto out;
191 191 if (unlikely(atomic_read(&current->tracing_graph_pause)))
192 192 goto out;
193   - if (ftrace_push_return_trace(parent, ip, &trace.depth) == -EBUSY)
  193 + if (ftrace_push_return_trace(parent, ip, &trace.depth, 0) == -EBUSY)
194 194 goto out;
195 195 trace.func = ftrace_mcount_call_adjust(ip) & PSW_ADDR_INSN;
196 196 /* Only trace if the calling function expects to. */
... ... @@ -33,6 +33,7 @@
33 33 select HAVE_DYNAMIC_FTRACE
34 34 select HAVE_FUNCTION_TRACER
35 35 select HAVE_FUNCTION_GRAPH_TRACER
  36 + select HAVE_FUNCTION_GRAPH_FP_TEST
36 37 select HAVE_FUNCTION_TRACE_MCOUNT_TEST
37 38 select HAVE_FTRACE_NMI_ENTER if DYNAMIC_FTRACE
38 39 select HAVE_FTRACE_SYSCALLS
arch/x86/kernel/entry_32.S
... ... @@ -1154,6 +1154,7 @@
1154 1154 pushl %edx
1155 1155 movl 0xc(%esp), %edx
1156 1156 lea 0x4(%ebp), %eax
  1157 + movl (%ebp), %ecx
1157 1158 subl $MCOUNT_INSN_SIZE, %edx
1158 1159 call prepare_ftrace_return
1159 1160 popl %edx
... ... @@ -1168,6 +1169,7 @@
1168 1169 pushl %eax
1169 1170 pushl %ecx
1170 1171 pushl %edx
  1172 + movl %ebp, %eax
1171 1173 call ftrace_return_to_handler
1172 1174 movl %eax, 0xc(%esp)
1173 1175 popl %edx
arch/x86/kernel/entry_64.S
... ... @@ -135,6 +135,7 @@
135 135  
136 136 leaq 8(%rbp), %rdi
137 137 movq 0x38(%rsp), %rsi
  138 + movq (%rbp), %rdx
138 139 subq $MCOUNT_INSN_SIZE, %rsi
139 140  
140 141 call prepare_ftrace_return
... ... @@ -150,6 +151,7 @@
150 151 /* Save the return values */
151 152 movq %rax, (%rsp)
152 153 movq %rdx, 8(%rsp)
  154 + movq %rbp, %rdi
153 155  
154 156 call ftrace_return_to_handler
155 157  
arch/x86/kernel/ftrace.c
... ... @@ -408,7 +408,8 @@
408 408 * Hook the return address and push it in the stack of return addrs
409 409 * in current thread info.
410 410 */
411   -void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
  411 +void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
  412 + unsigned long frame_pointer)
412 413 {
413 414 unsigned long old;
414 415 int faulted;
... ... @@ -453,7 +454,8 @@
453 454 return;
454 455 }
455 456  
456   - if (ftrace_push_return_trace(old, self_addr, &trace.depth) == -EBUSY) {
  457 + if (ftrace_push_return_trace(old, self_addr, &trace.depth,
  458 + frame_pointer) == -EBUSY) {
457 459 *parent = old;
458 460 return;
459 461 }
include/linux/ftrace.h
... ... @@ -362,6 +362,7 @@
362 362 unsigned long func;
363 363 unsigned long long calltime;
364 364 unsigned long long subtime;
  365 + unsigned long fp;
365 366 };
366 367  
367 368 /*
... ... @@ -372,7 +373,8 @@
372 373 extern void return_to_handler(void);
373 374  
374 375 extern int
375   -ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth);
  376 +ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,
  377 + unsigned long frame_pointer);
376 378  
377 379 /*
378 380 * Sometimes we don't want to trace a function with the function
kernel/trace/Kconfig
... ... @@ -18,6 +18,13 @@
18 18 config HAVE_FUNCTION_GRAPH_TRACER
19 19 bool
20 20  
  21 +config HAVE_FUNCTION_GRAPH_FP_TEST
  22 + bool
  23 + help
  24 + An arch may pass in a unique value (frame pointer) to both the
  25 + entering and exiting of a function. On exit, the value is compared
  26 + and if it does not match, then it will panic the kernel.
  27 +
21 28 config HAVE_FUNCTION_TRACE_MCOUNT_TEST
22 29 bool
23 30 help
kernel/trace/trace_functions_graph.c
... ... @@ -57,7 +57,8 @@
57 57  
58 58 /* Add a function return address to the trace stack on thread info.*/
59 59 int
60   -ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth)
  60 +ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,
  61 + unsigned long frame_pointer)
61 62 {
62 63 unsigned long long calltime;
63 64 int index;
... ... @@ -85,6 +86,7 @@
85 86 current->ret_stack[index].func = func;
86 87 current->ret_stack[index].calltime = calltime;
87 88 current->ret_stack[index].subtime = 0;
  89 + current->ret_stack[index].fp = frame_pointer;
88 90 *depth = index;
89 91  
90 92 return 0;
... ... @@ -92,7 +94,8 @@
92 94  
93 95 /* Retrieve a function return address to the trace stack on thread info.*/
94 96 static void
95   -ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret)
  97 +ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
  98 + unsigned long frame_pointer)
96 99 {
97 100 int index;
98 101  
... ... @@ -106,6 +109,31 @@
106 109 return;
107 110 }
108 111  
  112 +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
  113 + /*
  114 + * The arch may choose to record the frame pointer used
  115 + * and check it here to make sure that it is what we expect it
  116 + * to be. If gcc does not set the place holder of the return
  117 + * address in the frame pointer, and does a copy instead, then
  118 + * the function graph trace will fail. This test detects this
  119 + * case.
  120 + *
  121 + * Currently, x86_32 with optimize for size (-Os) makes the latest
  122 + * gcc do the above.
  123 + */
  124 + if (unlikely(current->ret_stack[index].fp != frame_pointer)) {
  125 + ftrace_graph_stop();
  126 + WARN(1, "Bad frame pointer: expected %lx, received %lx\n"
  127 + " from func %pF return to %lx\n",
  128 + current->ret_stack[index].fp,
  129 + frame_pointer,
  130 + (void *)current->ret_stack[index].func,
  131 + current->ret_stack[index].ret);
  132 + *ret = (unsigned long)panic;
  133 + return;
  134 + }
  135 +#endif
  136 +
109 137 *ret = current->ret_stack[index].ret;
110 138 trace->func = current->ret_stack[index].func;
111 139 trace->calltime = current->ret_stack[index].calltime;
112 140  
... ... @@ -117,12 +145,12 @@
117 145 * Send the trace to the ring-buffer.
118 146 * @return the original return address.
119 147 */
120   -unsigned long ftrace_return_to_handler(void)
  148 +unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
121 149 {
122 150 struct ftrace_graph_ret trace;
123 151 unsigned long ret;
124 152  
125   - ftrace_pop_return_trace(&trace, &ret);
  153 + ftrace_pop_return_trace(&trace, &ret, frame_pointer);
126 154 trace.rettime = trace_clock_local();
127 155 ftrace_graph_return(&trace);
128 156 barrier();