Skip to content

Commit fee86a4

Browse files
mrutland-armrostedt
authored andcommitted
ftrace: selftest: remove broken trace_direct_tramp
The ftrace selftest code has a trace_direct_tramp() function which it uses as a direct call trampoline. This happens to work on x86, since the direct call's return address is in the usual place, and can be returned to via a RET, but in general the calling convention for direct calls is different from regular function calls, and requires a trampoline written in assembly. On s390, regular function calls place the return address in %r14, and an ftrace patch-site in an instrumented function places the trampoline's return address (which is within the instrumented function) in %r0, preserving the original %r14 value in-place. As a regular C function will return to the address in %r14, using a C function as the trampoline results in the trampoline returning to the caller of the instrumented function, skipping the body of the instrumented function. Note that the s390 issue is not detcted by the ftrace selftest code, as the instrumented function is trivial, and returning back into the caller happens to be equivalent. On arm64, regular function calls place the return address in x30, and an ftrace patch-site in an instrumented function saves this into r9 and places the trampoline's return address (within the instrumented function) in x30. A regular C function will return to the address in x30, but will not restore x9 into x30. Consequently, using a C function as the trampoline results in returning to the trampoline's return address having corrupted x30, such that when the instrumented function returns, it will return back into itself. To avoid future issues in this area, remove the trace_direct_tramp() function, and require that each architecture with direct calls provides a stub trampoline, named ftrace_stub_direct_tramp. This can be written to handle the architecture's trampoline calling convention, and in future could be used elsewhere (e.g. in the ftrace ops sample, to measure the overhead of direct calls), so we may as well always build it in. Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Mark Rutland <[email protected]> Cc: Li Huafei <[email protected]> Cc: Xu Kuohai <[email protected]> Signed-off-by: Florent Revest <[email protected]> Acked-by: Jiri Olsa <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 60c8971 commit fee86a4

File tree

5 files changed

+18
-10
lines changed

5 files changed

+18
-10
lines changed

arch/s390/kernel/mcount.S

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ ENTRY(ftrace_stub)
3232
BR_EX %r14
3333
ENDPROC(ftrace_stub)
3434

35+
SYM_CODE_START(ftrace_stub_direct_tramp)
36+
lgr %r1, %r0
37+
BR_EX %r1
38+
SYM_CODE_END(ftrace_stub_direct_tramp)
39+
3540
.macro ftrace_regs_entry, allregs=0
3641
stg %r14,(__SF_GPRS+8*8)(%r15) # save traced function caller
3742

arch/x86/kernel/ftrace_32.S

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
163163
jmp .Lftrace_ret
164164
SYM_CODE_END(ftrace_regs_caller)
165165

166+
SYM_FUNC_START(ftrace_stub_direct_tramp)
167+
CALL_DEPTH_ACCOUNT
168+
RET
169+
SYM_FUNC_END(ftrace_stub_direct_tramp)
170+
166171
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
167172
SYM_CODE_START(ftrace_graph_caller)
168173
pushl %eax

arch/x86/kernel/ftrace_64.S

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,10 @@ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
309309
SYM_FUNC_END(ftrace_regs_caller)
310310
STACK_FRAME_NON_STANDARD_FP(ftrace_regs_caller)
311311

312+
SYM_FUNC_START(ftrace_stub_direct_tramp)
313+
CALL_DEPTH_ACCOUNT
314+
RET
315+
SYM_FUNC_END(ftrace_stub_direct_tramp)
312316

313317
#else /* ! CONFIG_DYNAMIC_FTRACE */
314318

include/linux/ftrace.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,8 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
413413
int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
414414
int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
415415

416+
void ftrace_stub_direct_tramp(void);
417+
416418
#else
417419
struct ftrace_ops;
418420
# define ftrace_direct_func_count 0

kernel/trace/trace_selftest.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -786,14 +786,6 @@ static struct fgraph_ops fgraph_ops __initdata = {
786786

787787
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
788788
static struct ftrace_ops direct;
789-
#ifndef CALL_DEPTH_ACCOUNT
790-
#define CALL_DEPTH_ACCOUNT ""
791-
#endif
792-
793-
noinline __noclone static void trace_direct_tramp(void)
794-
{
795-
asm(CALL_DEPTH_ACCOUNT);
796-
}
797789
#endif
798790

799791
/*
@@ -873,7 +865,7 @@ trace_selftest_startup_function_graph(struct tracer *trace,
873865
*/
874866
ftrace_set_filter_ip(&direct, (unsigned long)DYN_FTRACE_TEST_NAME, 0, 0);
875867
ret = register_ftrace_direct(&direct,
876-
(unsigned long)trace_direct_tramp);
868+
(unsigned long)ftrace_stub_direct_tramp);
877869
if (ret)
878870
goto out;
879871

@@ -894,7 +886,7 @@ trace_selftest_startup_function_graph(struct tracer *trace,
894886
unregister_ftrace_graph(&fgraph_ops);
895887

896888
ret = unregister_ftrace_direct(&direct,
897-
(unsigned long) trace_direct_tramp,
889+
(unsigned long)ftrace_stub_direct_tramp,
898890
true);
899891
if (ret)
900892
goto out;

0 commit comments

Comments
 (0)