ftrace: Add access to function arguments for all callbacks
Steven Rostedt
maintainerv5 cover ftrace: Add access to function arguments for all callbacks
This is something I wanted to implement a long time ago, but held off until there was a good reason to do so. Now it appears that having access to the arguments of the function by default is very useful. As a bonus, because arguments must be saved regardless before calling a callback, because they need to be restored before returning back to the start of the traced function, there's not much work to do to have them always be there for normal function callbacks.
The basic idea is that if CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS is set, then all callbacks registered to ftrace can use the regs parameter for the stack and arguments (kernel_stack_pointer(regs), regs_get_kernel_argument(regs, n)), without the need to set REGS that causes overhead by saving all registers as REGS simulates a breakpoint.
This could be extended to move the REGS portion to kprobes itself, and remove the SAVE_REGS flag completely, but for now, kprobes still uses the full SAVE_REGS support.
The last patch extends the WITH_ARGS to allow default function tracing to modify the instruction pointer, where livepatching for x86 no longer needs to save all registers.
The idea of this approach is to give enough information to a callback that it could retrieve all arguments, which includes the stack pointer and instruction pointer.
This can also be extended to modify the function graph tracer to use the function tracer instead of having a separate trampoline.
Changes since v4:
- Fixed compile error for !CONFIG_FUNCTION_TRACER
- Fixed prototype of pstore ftrace callback function.
This also passed all my tests, and I will probably add this to my for-next soon if there's no complaints.
Steven Rostedt (VMware) (3):
ftrace: Have the callbacks receive a struct ftrace_regs instead of pt_regs ftrace/x86: Allow for arguments to be passed in to ftrace_regs by default
livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
----
arch/powerpc/include/asm/livepatch.h | 4 +++- arch/s390/include/asm/livepatch.h | 5 ++++- arch/x86/Kconfig | 1 + arch/x86/include/asm/ftrace.h | 18 ++++++++++++++++++ arch/x86/include/asm/livepatch.h | 4 ++-- arch/x86/kernel/ftrace_64.S | 15 +++++++++++++-- arch/x86/kernel/kprobes/ftrace.c | 3 ++- fs/pstore/ftrace.c | 2 +- include/linux/ftrace.h | 28 ++++++++++++++++++++++++++-- include/linux/kprobes.h | 2 +- kernel/livepatch/Kconfig | 2 +- kernel/livepatch/patch.c | 10 ++++++---- kernel/trace/Kconfig | 9 +++++++++ kernel/trace/ftrace.c | 27 +++++++++++++++------------ kernel/trace/trace_event_perf.c | 2 +- kernel/trace/trace_events.c | 2 +- kernel/trace/trace_functions.c | 9 ++++----- kernel/trace/trace_irqsoff.c | 2 +- kernel/trace/trace_sched_wakeup.c | 2 +- kernel/trace/trace_selftest.c | 20 +++++++++++--------- kernel/trace/trace_stack.c | 2 +- 21 files changed, 122 insertions(+), 47 deletions(-)
Steven Rostedt
maintainerPATCH v5 1/3 ftrace: Have the callbacks receive a struct ftrace_regs instead of pt_regs
From: "Steven Rostedt (VMware)" <[email protected]>
In preparation to have arguments of a function passed to callbacks attached to functions as default, change the default callback prototype to receive a struct ftrace_regs as the forth parameter instead of a pt_regs.
For callbacks that set the FL_SAVE_REGS flag in their ftrace_ops flags, they will now need to get the pt_regs via a ftrace_get_regs() helper call. If this is called by a callback that their ftrace_ops did not have a FL_SAVE_REGS flag set, it that helper function will return NULL.
This will allow the ftrace_regs to hold enough just to get the parameters and stack pointer, but without the worry that callbacks may have a pt_regs that is not completely filled.
--- arch/x86/kernel/kprobes/ftrace.c | 3 ++- fs/pstore/ftrace.c | 2 +- include/linux/ftrace.h | 16 ++++++++++++++-- include/linux/kprobes.h | 2 +- kernel/livepatch/patch.c | 3 ++- kernel/trace/ftrace.c | 27 +++++++++++++++------------ kernel/trace/trace_event_perf.c | 2 +- kernel/trace/trace_events.c | 2 +- kernel/trace/trace_functions.c | 9 ++++----- kernel/trace/trace_irqsoff.c | 2 +- kernel/trace/trace_sched_wakeup.c | 2 +- kernel/trace/trace_selftest.c | 20 +++++++++++--------- kernel/trace/trace_stack.c | 2 +- 13 files changed, 55 insertions(+), 37 deletions(-)
arch/x86/kernel/kprobes/ftrace.c +2 -1
@@ -14,8 +14,9 @@/* Ftrace callback handler for kprobes -- called under preepmt disabed */ void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *regs) +struct ftrace_ops *ops,struct ftrace_regs *fregs) { +struct pt_regs *regs =ftrace_get_regs (fregs);struct kprobe *p;struct kprobe_ctlblk *kcb;int bit;
fs/pstore/ftrace.c +1 -1
@@ -26,7 +26,7 @@ static u64 pstore_ftrace_stamp; static void notrace pstore_ftrace_call(unsigned long ip,unsigned long parent_ip,struct ftrace_ops *op, -struct pt_regs *regs) +struct ftrace_regs *fregs) {int bit;unsigned long flags;
include/linux/ftrace.h +14 -2
@@ -90,8 +90,20 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,struct ftrace_ops ; +struct ftrace_regs { +struct pt_regs regs; +}; + +static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs) +{ +if (!fregs) +return NULL ; + +return &fregs->regs; +} +typedef void (*ftrace_func_t )(unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *regs); +struct ftrace_ops *op,struct ftrace_regs *fregs);ftrace_func_t ftrace_ops_get_func (struct ftrace_ops *ops);
@@ -259,7 +271,7 @@ int register_ftrace_function(struct ftrace_ops *ops);int unregister_ftrace_function (struct ftrace_ops *ops);extern void ftrace_stub (unsigned long a0,unsigned long a1, -struct ftrace_ops *op,struct pt_regs *regs); +struct ftrace_ops *op,struct ftrace_regs *fregs);#else /* !CONFIG_FUNCTION_TRACER */ /*
include/linux/kprobes.h +1 -1
@@ -345,7 +345,7 @@ static inline void wait_for_kprobe_optimizer(void) { }#endif /* CONFIG_OPTPROBES */ #ifdef CONFIG_KPROBES_ON_FTRACE extern void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *regs); +struct ftrace_ops *ops,struct ftrace_regs *fregs);extern int arch_prepare_kprobe_ftrace (struct kprobe *p);#endif
kernel/livepatch/patch.c +2 -1
@@ -40,8 +40,9 @@ struct klp_ops *klp_find_ops(void *old_func) static void notrace klp_ftrace_handler(unsigned long ip,unsigned long parent_ip,struct ftrace_ops *fops, -struct pt_regs *regs) +struct ftrace_regs *fregs) { +struct pt_regs *regs =ftrace_get_regs (fregs);struct klp_ops *ops;struct klp_func *func;int patch_state;
kernel/trace/ftrace.c +15 -12
@@ -121,7 +121,7 @@ struct ftrace_ops global_ops;#if ARCH_SUPPORTS_FTRACE_OPS static void ftrace_ops_list_func (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *regs); +struct ftrace_ops *op,struct ftrace_regs *fregs);#else /* See comment below, where ftrace_ops_list_func is defined */ static void ftrace_ops_no_ops (unsigned long ip,unsigned long parent_ip);
@@ -140,7 +140,7 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops) }static void ftrace_pid_func (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct trace_array *tr = op->private;int pid;
@@ -154,7 +154,7 @@ static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip,return ; } - op->saved_func (ip, parent_ip, op, regs); + op->saved_func (ip, parent_ip, op, fregs); }static void ftrace_sync_ipi (void *data)
@@ -754,7 +754,7 @@ ftrace_profile_alloc(struct ftrace_profile_stat *stat, unsigned long ip)static void function_profile_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *regs) +struct ftrace_ops *ops,struct ftrace_regs *fregs) {struct ftrace_profile_stat *stat;struct ftrace_profile *rec;
@@ -2143,6 +2143,7 @@ static int ftrace_check_record(struct dyn_ftrace *rec, bool enable, bool update)else rec->flags &= ~FTRACE_FL_TRAMP_EN ; } +if (flag &FTRACE_FL_DIRECT ) {/* * If there's only one user (direct_ops helper)
@@ -2368,8 +2369,9 @@ unsigned long ftrace_find_rec_direct(unsigned long ip) }static void call_direct_funcs (unsigned long ip,unsigned long pip, -struct ftrace_ops *ops,struct pt_regs *regs) +struct ftrace_ops *ops,struct ftrace_regs *fregs) { +struct pt_regs *regs =ftrace_get_regs (fregs);unsigned long addr; addr =ftrace_find_rec_direct (ip);
@@ -4292,7 +4294,7 @@ static int __init ftrace_mod_cmd_init(void)core_initcall (ftrace_mod_cmd_init);static void function_trace_probe_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct ftrace_probe_ops *probe_ops;struct ftrace_func_probe *probe;
@@ -6911,8 +6913,9 @@ void ftrace_reset_array_ops(struct trace_array *tr) static nokprobe_inline void__ftrace_ops_list_func (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ignored,struct pt_regs *regs) +struct ftrace_ops *ignored,struct ftrace_regs *fregs) { +struct pt_regs *regs =ftrace_get_regs (fregs);struct ftrace_ops *op;int bit;
@@ -6945,7 +6948,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,pr_warn ("op=%p %pS\n" , op, op); goto out; } - op->func (ip, parent_ip, op, regs); + op->func (ip, parent_ip, op, fregs); } }while_for_each_ftrace_op (op); out:
@@ -6968,9 +6971,9 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,*/ #if ARCH_SUPPORTS_FTRACE_OPS static void ftrace_ops_list_func (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) { -__ftrace_ops_list_func (ip, parent_ip,NULL , regs); +__ftrace_ops_list_func (ip, parent_ip,NULL , fregs); }NOKPROBE_SYMBOL (ftrace_ops_list_func);#else
@@ -6987,7 +6990,7 @@ NOKPROBE_SYMBOL(ftrace_ops_no_ops);* this function will be called by the mcount trampoline. */ static void ftrace_ops_assist_func (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {int bit;
@@ -6998,7 +7001,7 @@ static void ftrace_ops_assist_func(unsigned long ip, unsigned long parent_ip,preempt_disable_notrace ();if (!(op->flags &FTRACE_OPS_FL_RCU ) ||rcu_is_watching ()) - op->func (ip, parent_ip, op, regs); + op->func (ip, parent_ip, op, fregs);preempt_enable_notrace ();trace_clear_recursion (bit);
kernel/trace/trace_event_perf.c +1 -1
@@ -432,7 +432,7 @@ NOKPROBE_SYMBOL(perf_trace_buf_update);#ifdef CONFIG_FUNCTION_TRACER static void perf_ftrace_function_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *pt_regs) +struct ftrace_ops *ops,struct ftrace_regs *fregs) {struct ftrace_entry *entry;struct perf_event *event;
kernel/trace/trace_events.c +1 -1
@@ -3673,7 +3673,7 @@ static struct trace_event_file event_trace_file __initdata;static void __init function_test_events_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *regs) {struct trace_buffer *buffer;struct ring_buffer_event *event;
kernel/trace/trace_functions.c +4 -5
@@ -23,10 +23,10 @@ static void tracing_start_function_trace(struct trace_array *tr);static void tracing_stop_function_trace (struct trace_array *tr);static void function_trace_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs); +struct ftrace_ops *op,struct ftrace_regs *fregs);static void function_stack_trace_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs); +struct ftrace_ops *op,struct ftrace_regs *fregs);static struct tracer_flags func_flags;/* Our option */
@@ -89,7 +89,6 @@ void ftrace_destroy_function_files(struct trace_array *tr)static int function_trace_init (struct trace_array *tr) {ftrace_func_t func; -/* * Instance trace_arrays get their ops allocated * at instance creation. Unless it failed
@@ -129,7 +128,7 @@ static void function_trace_start(struct trace_array *tr)static void function_trace_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct trace_array *tr = op->private;struct trace_array_cpu *data;
@@ -178,7 +177,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,static void function_stack_trace_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct trace_array *tr = op->private;struct trace_array_cpu *data;
kernel/trace/trace_irqsoff.c +1 -1
@@ -138,7 +138,7 @@ static int func_prolog_dec(struct trace_array *tr,*/ static void irqsoff_tracer_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct trace_array *tr = irqsoff_trace;struct trace_array_cpu *data;
kernel/trace/trace_sched_wakeup.c +1 -1
@@ -212,7 +212,7 @@ static void wakeup_print_header(struct seq_file *s)*/ static void wakeup_tracer_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {struct trace_array *tr = wakeup_trace;struct trace_array_cpu *data;
kernel/trace/trace_selftest.c +11 -9
@@ -107,7 +107,7 @@ static int trace_selftest_test_probe1_cnt;static void trace_selftest_test_probe1_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { trace_selftest_test_probe1_cnt++; }
@@ -116,7 +116,7 @@ static int trace_selftest_test_probe2_cnt;static void trace_selftest_test_probe2_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { trace_selftest_test_probe2_cnt++; }
@@ -125,7 +125,7 @@ static int trace_selftest_test_probe3_cnt;static void trace_selftest_test_probe3_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { trace_selftest_test_probe3_cnt++; }
@@ -134,7 +134,7 @@ static int trace_selftest_test_global_cnt;static void trace_selftest_test_global_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { trace_selftest_test_global_cnt++; }
@@ -143,7 +143,7 @@ static int trace_selftest_test_dyn_cnt;static void trace_selftest_test_dyn_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { trace_selftest_test_dyn_cnt++; }
@@ -414,7 +414,7 @@ static int trace_selftest_recursion_cnt;static void trace_selftest_test_recursion_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) {/* * This function is registered without the recursion safe flag.
@@ -429,7 +429,7 @@ static void trace_selftest_test_recursion_func(unsigned long ip,static void trace_selftest_test_recursion_safe_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) {/* * We said we would provide our own recursion. By calling
@@ -548,9 +548,11 @@ static enum {static void trace_selftest_test_regs_func (unsigned long ip,unsigned long pip,struct ftrace_ops *op, -struct pt_regs *pt_regs) +struct ftrace_regs *fregs) { -if (pt_regs) +struct pt_regs *regs =ftrace_get_regs (fregs); + +if (regs) trace_selftest_regs_stat =TRACE_SELFTEST_REGS_FOUND ;else trace_selftest_regs_stat =TRACE_SELFTEST_REGS_NOT_FOUND ;
kernel/trace/trace_stack.c +1 -1
@@ -290,7 +290,7 @@ static void check_stack(unsigned long ip, unsigned long *stack)static void stack_trace_call (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *op,struct pt_regs *pt_regs) +struct ftrace_ops *op,struct ftrace_regs *fregs) {unsigned long stack;
Signature · 1 line
2.28.0
Re: [PATCH 1/3 v5] ftrace: Have the callbacks receive a struct ftrace_regs instead of pt_regs
Hi Steven,
Quoted 33 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
In preparation to have arguments of a function passed to callbacks attached to functions as default, change the default callback prototype to receive a struct
ftrace_regsas the forth parameter instead of apt_regs.For callbacks that set the
FL_SAVE_REGSflag in theirftrace_opsflags, they will now need to get thept_regsvia aftrace_get_regs()helper call. If this is called by a callback that theirftrace_opsdid not have aFL_SAVE_REGSflag set, it that helper function will return NULL.This will allow the
ftrace_regsto hold enough just to get the parameters and stack pointer, but without the worry that callbacks may have apt_regsthat is not completely filled.Reviewed-by Masami Hiramatsu <[email protected]>Signed-off-by Steven Rostedt (VMware) <[email protected]>--- arch/x86/kernel/kprobes/ftrace.c | 3 ++- fs/pstore/ftrace.c | 2 +- include/linux/ftrace.h | 16 ++++++++++++++-- include/linux/kprobes.h | 2 +- kernel/livepatch/patch.c | 3 ++- kernel/trace/ftrace.c | 27 +++++++++++++++------------ kernel/trace/trace_event_perf.c | 2 +- kernel/trace/trace_events.c | 2 +- kernel/trace/trace_functions.c | 9 ++++----- kernel/trace/trace_irqsoff.c | 2 +- kernel/trace/trace_sched_wakeup.c | 2 +- kernel/trace/trace_selftest.c | 20 +++++++++++--------- kernel/trace/trace_stack.c | 2 +- 13 files changed, 55 insertions(+), 37 deletions(-)
<snip>
include/linux/kprobes.h static inline void wait_for_kprobe_optimizer(void) · line 349
Quoted 11 lines
include/linux/kprobes.h +1
-1@@ -345,7 +345,7 @@ static inline void wait_for_kprobe_optimizer(void) { }#endif /* CONFIG_OPTPROBES */ #ifdef CONFIG_KPROBES_ON_FTRACE extern void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *regs); +struct ftrace_ops *ops,struct ftrace_regs *fregs);
This breaks the build on non-x86 architectures that support KPROBES_ON_FTRACE.
- Naveen
Steven Rostedt
maintainerRe: [PATCH 1/3 v5] ftrace: Have the callbacks receive a struct ftrace_regs instead of pt_regs
Quoted 6 lines · On Thu, 12 Nov 2020 22:38:53 +0530 "Naveen N. Rao"…
Quoted 6 lines
include/linux/kprobes.h +0
-0@@ -345,7 +345,7 @@ static inline void wait_for_kprobe_optimizer(void) { }
Quoted 8 lines
include/linux/kprobes.h static inline void wait_for_kprobe_optimizer(void) { } · line 348
Quoted 5 lines
include/linux/kprobes.h +1
-1#endif /* CONFIG_OPTPROBES */ #ifdef CONFIG_KPROBES_ON_FTRACE extern void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, -struct ftrace_ops *ops,struct pt_regs *regs); +struct ftrace_ops *ops,struct ftrace_regs *fregs);This breaks the build on non-x86 architectures that support
KPROBES_ON_FTRACE.
I thought I ran this on my cross tests build. :-/
But it seems this only updated x86. I thought I updated all the other archs, but it appears that I didn't. I think I may have forgotten to do a git commit properly.
Let me go and fix that (I have to update this series anyway).
Thanks for reporting.
-- Steve
Steven Rostedt
maintainerPATCH v5 2/3 ftrace/x86: Allow for arguments to be passed in to ftrace_regs by default
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
Currently, the only way to get access to the registers of a function via a ftrace callback is to set the "FL_SAVE_REGS" bit in the ftrace_ops. But as this saves all regs as if a breakpoint were to trigger (for use with kprobes), it is expensive.
The regs are already saved on the stack for the default ftrace callbacks, as that is required otherwise a function being traced will get the wrong arguments and possibly crash. And on x86, the arguments are already stored where they would be on a pt_regs structure to use that code for both the regs version of a callback, it makes sense to pass that information always to all functions.
If an architecture does this (as x86_64 now does), it is to set HAVE_DYNAMIC_FTRACE_WITH_ARGS, and this will let the generic code that it could have access to arguments without having to set the flags.
This also includes having the stack pointer being saved, which could be used for accessing arguments on the stack, as well as having the function graph tracer not require its own trampoline!
--- arch/x86/Kconfig | 1 + arch/x86/include/asm/ftrace.h | 15 +++++++++++++++ arch/x86/kernel/ftrace_64.S | 11 +++++++++-- include/linux/ftrace.h | 7 ++++++- kernel/trace/Kconfig | 9 +++++++++ 5 files changed, 40 insertions(+), 3 deletions(-)
arch/x86/Kconfig +1 -0
@@ -167,6 +167,7 @@ config X86 select HAVE_DMA_CONTIGUOUS select HAVE_DYNAMIC_FTRACE select HAVE_DYNAMIC_FTRACE_WITH_REGS + select HAVE_DYNAMIC_FTRACE_WITH_ARGS if X86_64 select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select HAVE_EBPF_JIT select HAVE_EFFICIENT_UNALIGNED_ACCESS
arch/x86/include/asm/ftrace.h +15 -0
@@ -41,6 +41,21 @@ static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned regs->orig_ax = addr; } +#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS +struct ftrace_regs { +struct pt_regs regs; +}; + +static __always_inline struct pt_regs * +arch_ftrace_get_regs (struct ftrace_regs *fregs) +{ +/* Only when FL_SAVE_REGS is set, cs will be non zero */ +if (!fregs->regs.cs) +return NULL ; +return &fregs->regs; +} +#endif +#ifdef CONFIG_DYNAMIC_FTRACE struct dyn_arch_ftrace {
arch/x86/kernel/ftrace_64.S +9 -2
@@ -140,12 +140,19 @@ SYM_FUNC_START(ftrace_caller) /* save_mcount_regs fills in first two parameters */ save_mcount_regs + /* Stack - skipping return address of ftrace_caller */ + leaq MCOUNT_REG_SIZE+8(%rsp), %rcx + movq %rcx, RSP(%rsp) + SYM_INNER_LABEL(ftrace_caller_op_ptr, SYM_L_GLOBAL) /* Load the ftrace_ops into the 3rd parameter */ movq function_trace_op(%rip), %rdx - /* regs go into 4th parameter (but make it NULL) */ - movq $0, %rcx + /* regs go into 4th parameter */ + leaq (%rsp), %rcx + + /* Only ops with REGS flag set should have CS register set */ + movq $0, CS(%rsp) SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL) call ftrace_stub
include/linux/ftrace.h +6 -1
@@ -90,16 +90,21 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,struct ftrace_ops ; +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS +struct ftrace_regs {struct pt_regs regs; }; +#define arch_ftrace_get_regs (fregs) (&(fregs)->regs) + +#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs) {if (!fregs)return NULL ; -return &fregs->regs; +return arch_ftrace_get_regs (fregs); } typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
kernel/trace/Kconfig +9 -0
@@ -31,6 +31,15 @@ config HAVE_DYNAMIC_FTRACE_WITH_REGS config HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS bool +config HAVE_DYNAMIC_FTRACE_WITH_ARGS + bool + help + If this is set, then arguments and stack can be found from + the pt_regs passed into the function callback regs parameter + by default, even without setting the REGS flag in the ftrace_ops. + This allows for use of regs_get_kernel_argument() and + kernel_stack_pointer(). + config HAVE_FTRACE_MCOUNT_RECORD bool help
Signature · 1 line
2.28.0
Steven Rostedt
maintainerPATCH v5 3/3 livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
When CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS is available, the ftrace call will be able to set the ip of the calling function. This will improve the performance of live kernel patching where it does not need all the regs to be stored just to change the instruction pointer.
If all archs that support live kernel patching also support HAVE_DYNAMIC_FTRACE_WITH_ARGS, then the architecture specific function klp_arch_set_pc() could be made generic.
It is possible that an arch can support HAVE_DYNAMIC_FTRACE_WITH_ARGS but not HAVE_DYNAMIC_FTRACE_WITH_REGS and then have access to live patching.
--- arch/powerpc/include/asm/livepatch.h | 4 +++- arch/s390/include/asm/livepatch.h | 5 ++++- arch/x86/include/asm/ftrace.h | 3 +++ arch/x86/include/asm/livepatch.h | 4 ++-- arch/x86/kernel/ftrace_64.S | 4 ++++ include/linux/ftrace.h | 7 +++++++ kernel/livepatch/Kconfig | 2 +- kernel/livepatch/patch.c | 9 +++++---- 8 files changed, 29 insertions(+), 9 deletions(-)
arch/powerpc/include/asm/livepatch.h +3 -1
@@ -12,8 +12,10 @@#include <linux/sched/task_stack.h> #ifdef CONFIG_LIVEPATCH -static inline void klp_arch_set_pc (struct pt_regs *regs,unsigned long ip) +static inline void klp_arch_set_pc (struct ftrace_regs *fregs,unsigned long ip) { +struct pt_regs *regs =ftrace_get_regs (fregs); + regs->nip = ip; }
arch/s390/include/asm/livepatch.h +4 -1
@@ -11,10 +11,13 @@#ifndef ASM_LIVEPATCH_H #define ASM_LIVEPATCH_H +#include <linux/ftrace.h> #include <asm/ptrace.h> -static inline void klp_arch_set_pc (struct pt_regs *regs,unsigned long ip) +static inline void klp_arch_set_pc (struct ftrace_regs *fregs,unsigned long ip) { +struct pt_regs *regs =ftrace_get_regs (fregs); + regs->psw.addr = ip; }
arch/x86/include/asm/ftrace.h +3 -0
@@ -54,6 +54,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)return NULL ; return &fregs->regs; } + +#define ftrace_regs_set_ip (fregs, _ip) \ + do { (fregs)->regs.ip = (_ip); } while (0)#endif #ifdef CONFIG_DYNAMIC_FTRACE
arch/x86/include/asm/livepatch.h +2 -2
@@ -12,9 +12,9 @@#include <asm/setup.h> #include <linux/ftrace.h> -static inline void klp_arch_set_pc (struct pt_regs *regs,unsigned long ip) +static inline void klp_arch_set_pc (struct ftrace_regs *fregs,unsigned long ip) { - regs->ip = ip; +ftrace_regs_set_ip (fregs, ip); }#endif /* _ASM_X86_LIVEPATCH_H */
arch/x86/kernel/ftrace_64.S +4 -0
@@ -157,6 +157,10 @@ SYM_INNER_LABEL(ftrace_caller_op_ptr, SYM_L_GLOBAL) SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL) call ftrace_stub + /* Handlers can change the RIP */ + movq RIP(%rsp), %rax + movq %rax, MCOUNT_REG_SIZE(%rsp) + restore_mcount_regs /*
include/linux/ftrace.h +7 -0
@@ -97,6 +97,13 @@ struct ftrace_regs { };#define arch_ftrace_get_regs (fregs) (&(fregs)->regs) +/* +* ftrace_regs_set_ip() is to be defined by the architecture if +* to allow setting of the instruction pointer from the ftrace_regs +* when HAVE_DYNAMIC_FTRACE_WITH_ARGS is set and it supports +* live kernel patching. +*/ +#define ftrace_regs_set_ip (fregs, ip) do { } while (0)#endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs)
kernel/livepatch/Kconfig +1 -1
@@ -6,7 +6,7 @@ config HAVE_LIVEPATCH config LIVEPATCH bool "Kernel Live Patching" - depends on DYNAMIC_FTRACE_WITH_REGS + depends on DYNAMIC_FTRACE_WITH_REGS || DYNAMIC_FTRACE_WITH_ARGS depends on MODULES depends on SYSFS depends on KALLSYMS_ALL
kernel/livepatch/patch.c +5 -4
@@ -42,7 +42,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,struct ftrace_ops *fops, struct ftrace_regs *fregs) { - struct pt_regs *regs = ftrace_get_regs(fregs); struct klp_ops *ops;struct klp_func *func;int patch_state;
@@ -118,7 +117,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,if (func->nop) goto unlock; -klp_arch_set_pc (regs, (unsigned long )func->new_func); +klp_arch_set_pc (fregs, (unsigned long )func->new_func); unlock:preempt_enable_notrace ();
@@ -200,8 +199,10 @@ static int klp_patch_func(struct klp_func *func)return -ENOMEM ; ops->fops.func = klp_ftrace_handler; - ops->fops.flags =FTRACE_OPS_FL_SAVE_REGS | -FTRACE_OPS_FL_DYNAMIC | + ops->fops.flags = FTRACE_OPS_FL_DYNAMIC | +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS + FTRACE_OPS_FL_SAVE_REGS | +#endif FTRACE_OPS_FL_IPMODIFY |FTRACE_OPS_FL_PERMANENT ;
Signature · 1 line
2.28.0
Re: [PATCH 3/3 v5] livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
Quoted 19 lines · Steven Rostedt
arch/x86/include/asm/ftrace.h +3
-0@@ -54,6 +54,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)return NULL ; return &fregs->regs; } + +#define ftrace_regs_set_ip (fregs, _ip) \ + do { (fregs)->regs.ip = (_ip); } while (0)#endif #ifdef CONFIG_DYNAMIC_FTRACE arch/x86/include/asm/livepatch.h +0
-0
arch/x86/include/asm/livepatch.h line 19
Quoted 10 lines
arch/x86/include/asm/livepatch.h +2
-2@@ -12,9 +12,9 @@#include <asm/setup.h> #include <linux/ftrace.h> -static inline void klp_arch_set_pc (struct pt_regs *regs,unsigned long ip) +static inline void klp_arch_set_pc (struct ftrace_regs *fregs,unsigned long ip) { - regs->ip = ip; +ftrace_regs_set_ip (fregs, ip); }
The normal variant is called instruction_pointer_set(), should this be called ftrace_instruction_pointer_set() ?
(and yes, I hate the long name too).
Also, do you want something like:
unsigned long ftrace_regs_get_register(struct ftrace_regs *regs, unsigned int offset)
{
switch (offset / sizeof(long)) {
case 4: /* RBP */
case 8: /* R9 */
case 9: /* R8 */
case 10: /* RAX */
case 11: /* RCX */
case 12: /* RDX */
case 13: /* RSI */
case 14: /* RDI */
case 15: /* ORIG_RAX */
case 16: /* RIP */
return *(unsigned long *)regs->regs + offset;
default:
WARN_ON_ONCE(1);
}
return 0;
}Re: [PATCH 3/3 v5] livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
Quoted 4 lines · Peter Zijlstra
Also, do you want something like:
unsigned long
ftrace_regs_get_register(struct ftrace_regs *regs, unsigned int offset){
I forgot the full regs case:
if (regs->regs.cs) return regs_get_register(regs->regs, offset);
Quoted 19 lines
switch (offset / sizeof(long)) {
case 4: /* RBP */
case 8: /* R9 */
case 9: /* R8 */
case 10: /* RAX */
case 11: /* RCX */
case 12: /* RDX */
case 13: /* RSI */
case 14: /* RDI */
case 15: /* ORIG_RAX */
case 16: /* RIP */
return *(unsigned long *)regs->regs + offset;
default:
WARN_ON_ONCE(1);
}
return 0;
}Steven Rostedt
maintainerRe: [PATCH 3/3 v5] livepatch: Use the default ftrace_ops instead of REGS when ARGS is available
In reply to Peter ZijlstraQuoted 24 lines · On Thu, 12 Nov 2020 09:21:44 +0100 Peter Zijlstra <[email protected]> wrote:
Quoted 22 lines · Steven Rostedt
arch/x86/include/asm/ftrace.h +3
-0@@ -54,6 +54,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)return NULL ; return &fregs->regs; } + +#define ftrace_regs_set_ip (fregs, _ip) \ + do { (fregs)->regs.ip = (_ip); } while (0)#endif #ifdef CONFIG_DYNAMIC_FTRACE arch/x86/include/asm/livepatch.h +0
-0@@ -12,9 +12,9 @@#include <asm/setup.h> #include <linux/ftrace.h>
Quoted 10 lines
arch/x86/include/asm/livepatch.h line 19
Quoted 7 lines
arch/x86/include/asm/livepatch.h +2
-2-static inline void klp_arch_set_pc (struct pt_regs *regs,unsigned long ip) +static inline void klp_arch_set_pc (struct ftrace_regs *fregs,unsigned long ip) { - regs->ip = ip; +ftrace_regs_set_ip (fregs, ip); }The normal variant is called
instruction_pointer_set(), should this be calledftrace_instruction_pointer_set()?
Sure, I can change that.
Quoted 2 lines
(and yes, I hate the long name too).
ftrace_regs_ip_set()? ;-)
Quoted 5 lines
Also, do you want something like:
unsigned long
ftrace_regs_get_register(struct ftrace_regs *regs, unsigned int offset){
I haven't gotten this far yet. I'm looking at generic use cases on how to get args across archs. Each arch will have its own method.
Quoted 8 lines
switch (offset / sizeof(long)) {
case 4: /* RBP */
case 8: /* R9 */
case 9: /* R8 */
case 10: /* RAX */
case 11: /* RCX */
case 12: /* RDX */Quoted 8 lines
case 13: /* RSI */ case 14: /* RDI */ case 15: /* ORIG_RAX */ case 16: /* RIP */ return *(unsigned long *)regs->regs + offset; default: WARN_ON_ONCE(1);
Not sure we even want to warn. Perhaps have this as:
bool ftrace_regs_get_register(struct ftrace_regs *regs,
unsigned int offset, unsigned long *val)
{
if (regs->cs) {
*val = regs_get_register(regs->regs, offset);
return true;
}
switch (offset / sizeof(long)) {
case ...:
*val = *(unsigned long *)regs->regs + offset;
return true;
default;
return false;Quoted 1 line
}
-- Steve