On Fri, 06 Nov 2020 16:42:35 -0500 Steven Rostedt <rost...@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rost...@goodmis.org> > > 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. > This looks good to me. Reviewed-by: Masami Hiramatsu <mhira...@kernel.org> Thank you, > Signed-off-by: Steven Rostedt (VMware) <rost...@goodmis.org> > --- > arch/x86/kernel/kprobes/ftrace.c | 3 ++- > 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_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 +- > 11 files changed, 53 insertions(+), 35 deletions(-) > > diff --git a/arch/x86/kernel/kprobes/ftrace.c > b/arch/x86/kernel/kprobes/ftrace.c > index 954d930a7127..373e5fa3ce1f 100644 > --- a/arch/x86/kernel/kprobes/ftrace.c > +++ b/arch/x86/kernel/kprobes/ftrace.c > @@ -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; > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 8dde9c17aaa5..24e1fa52337d 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -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 */ > /* > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 629abaf25681..be73350955e4 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -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 > > diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c > index 875c5dbbdd33..f89f9e7e9b07 100644 > --- a/kernel/livepatch/patch.c > +++ b/kernel/livepatch/patch.c > @@ -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; > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 3db64fb0cce8..67888311784e 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -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); > diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c > index 1b202e28dfaa..a71181655958 100644 > --- a/kernel/trace/trace_event_perf.c > +++ b/kernel/trace/trace_event_perf.c > @@ -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; > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c > index 646eda6c44a5..c5095dd28e20 100644 > --- a/kernel/trace/trace_functions.c > +++ b/kernel/trace/trace_functions.c > @@ -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; > diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c > index 10bbb0f381d5..d06aab4dcbb8 100644 > --- a/kernel/trace/trace_irqsoff.c > +++ b/kernel/trace/trace_irqsoff.c > @@ -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; > diff --git a/kernel/trace/trace_sched_wakeup.c > b/kernel/trace/trace_sched_wakeup.c > index 97b10bb31a1f..c0181066dbe9 100644 > --- a/kernel/trace/trace_sched_wakeup.c > +++ b/kernel/trace/trace_sched_wakeup.c > @@ -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; > diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c > index 8ee3c0bb5d8a..5ed081c6471c 100644 > --- a/kernel/trace/trace_selftest.c > +++ b/kernel/trace/trace_selftest.c > @@ -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; > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c > index 969db526a563..63c285042051 100644 > --- a/kernel/trace/trace_stack.c > +++ b/kernel/trace/trace_stack.c > @@ -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; > > -- > 2.28.0 > > -- Masami Hiramatsu <mhira...@kernel.org>