| Message ID | 169280379741.282662.12221517584561036597.stgit@devnote2 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: fprobe: rethook: Use ftrace_regs instead of pt_regs | expand |
On Wed, Aug 23, 2023 at 5:16 PM Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c > index c60d0d9f1a95..90ad28260a9f 100644 > --- a/kernel/trace/trace_fprobe.c > +++ b/kernel/trace/trace_fprobe.c > +#else /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ > + > +/* Since fprobe handlers can be nested, pt_regs buffer need to be a stack */ > +#define PERF_FPROBE_REGS_MAX 4 > + > +struct pt_regs_stack { > + struct pt_regs regs[PERF_FPROBE_REGS_MAX]; > + int idx; > +}; > + > +static DEFINE_PER_CPU(struct pt_regs_stack, perf_fprobe_regs); > + > +static __always_inline > +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) > +{ > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > + struct pt_regs *regs; > + > + if (stack->idx < PERF_FPROBE_REGS_MAX) { > + regs = stack->regs[stack->idx++]; This is missing an &: regs = &stack->regs[stack->idx++]; > + return ftrace_partial_regs(fregs, regs); I think this is incorrect on arm64 and will likely cause very subtle failure modes down the line on other architectures too. The problem on arm64 is that Perf calls "user_mode(regs)" somewhere down the line, that macro tries to read the "pstate" register, which is not populated in ftrace_regs, so it's not copied into a "partial" pt_regs either and Perf can take wrong decisions based on that. I already mentioned this problem in the past: - in the third answer block of: https://lore.kernel.org/all/CABRcYmJjtVq-330ktqTAUiNO1=yG_aHd0xz=c550O5C7QP++UA@mail.gmail.com/ - in the fourth answer block of: https://lore.kernel.org/all/CABRcYm+esb8J2O1v6=C+h+HSa5NxraPUgo63w7-iZj0CXbpusg@mail.gmail.com/ It is quite possible that other architectures at some point introduce a light ftrace "args" trampoline that misses one of the registers expected by Perf because they don't realize that this trampoline calls fprobe which calls Perf which has specific registers expectations. We got the green light from Alexei to use ftrace_partial_regs for "BPF mutli_kprobe" because these BPF programs can gracefully deal with sparse pt_regs but I think a similar conversation needs to happen with the Perf folks. ---- On a side-note, a subtle difference between ftrace_partial_regs with and without HAVE_PT_REGS_TO_FTRACE_REGS_CAST is that one does a copy and the other does not. If a subsystem receives a partial regs under HAVE_PT_REGS_TO_FTRACE_REGS_CAST, it can modify register fields and the modified values will be restored by the ftrace trampoline. Without HAVE_PT_REGS_TO_FTRACE_REGS_CAST, only the copy will be modified and ftrace won't restore them. I think the least we can do is to document thoroughly the guarantees of the ftrace_partial_regs API: users shouldn't rely on modifying the resulting regs because depending on the architecture this could do different things. People shouldn't rely on any register that isn't covered by one of the ftrace_regs_get_* helpers because it can be unpopulated on some architectures. I believe this is the case for BPF multi_kprobe but not for Perf. > + } > + return NULL; > +} > + > +static __always_inline void perf_fprobe_return_regs(struct pt_regs *regs) > +{ > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > + > + if (WARN_ON_ONCE(regs != stack->regs[stack->idx])) This is missing an & too: if (WARN_ON_ONCE(regs != &stack->regs[stack->idx])) > + return; > + > + --stack->idx; > +} > + > +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */
(Cc: Peter) On Fri, 25 Aug 2023 18:12:07 +0200 Florent Revest <revest@chromium.org> wrote: > On Wed, Aug 23, 2023 at 5:16 PM Masami Hiramatsu (Google) > <mhiramat@kernel.org> wrote: > > > > diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c > > index c60d0d9f1a95..90ad28260a9f 100644 > > --- a/kernel/trace/trace_fprobe.c > > +++ b/kernel/trace/trace_fprobe.c > > +#else /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ > > + > > +/* Since fprobe handlers can be nested, pt_regs buffer need to be a stack */ > > +#define PERF_FPROBE_REGS_MAX 4 > > + > > +struct pt_regs_stack { > > + struct pt_regs regs[PERF_FPROBE_REGS_MAX]; > > + int idx; > > +}; > > + > > +static DEFINE_PER_CPU(struct pt_regs_stack, perf_fprobe_regs); > > + > > +static __always_inline > > +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) > > +{ > > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > > + struct pt_regs *regs; > > + > > + if (stack->idx < PERF_FPROBE_REGS_MAX) { > > + regs = stack->regs[stack->idx++]; > > This is missing an &: > regs = &stack->regs[stack->idx++]; Oops, good point. I'm curious it didin't cause compile error... (I thought I built it on arm64) > > > + return ftrace_partial_regs(fregs, regs); > > I think this is incorrect on arm64 and will likely cause very subtle > failure modes down the line on other architectures too. The problem on > arm64 is that Perf calls "user_mode(regs)" somewhere down the line, > that macro tries to read the "pstate" register, which is not populated > in ftrace_regs, so it's not copied into a "partial" pt_regs either and > Perf can take wrong decisions based on that. I think we can assure the ftrace_regs is always !user_mode() so in that case ftrace_partial_regs() should fill the 'pstate' register as kernel mode. > > I already mentioned this problem in the past: > - in the third answer block of: > https://lore.kernel.org/all/CABRcYmJjtVq-330ktqTAUiNO1=yG_aHd0xz=c550O5C7QP++UA@mail.gmail.com/ > - in the fourth answer block of: > https://lore.kernel.org/all/CABRcYm+esb8J2O1v6=C+h+HSa5NxraPUgo63w7-iZj0CXbpusg@mail.gmail.com/ > Oops, sorry I missed that. And I basically agreed that we need a special care for perf. Let me reply it. > It is quite possible that other architectures at some point introduce > a light ftrace "args" trampoline that misses one of the registers > expected by Perf because they don't realize that this trampoline calls > fprobe which calls Perf which has specific registers expectations. Agreed. > > We got the green light from Alexei to use ftrace_partial_regs for "BPF > mutli_kprobe" because these BPF programs can gracefully deal with > sparse pt_regs but I think a similar conversation needs to happen with > the Perf folks. Indeed. Who is the best person to involve, Peterz? (but I think we need arm64 PMU part maintainer to talk) > > ---- > > On a side-note, a subtle difference between ftrace_partial_regs with > and without HAVE_PT_REGS_TO_FTRACE_REGS_CAST is that one does a copy > and the other does not. If a subsystem receives a partial regs under > HAVE_PT_REGS_TO_FTRACE_REGS_CAST, it can modify register fields and > the modified values will be restored by the ftrace trampoline. Without > HAVE_PT_REGS_TO_FTRACE_REGS_CAST, only the copy will be modified and > ftrace won't restore them. I think the least we can do is to document > thoroughly the guarantees of the ftrace_partial_regs API: users > shouldn't rely on modifying the resulting regs because depending on > the architecture this could do different things. People shouldn't rely > on any register that isn't covered by one of the ftrace_regs_get_* > helpers because it can be unpopulated on some architectures. I believe > this is the case for BPF multi_kprobe but not for Perf. I agree with the documentation requirement, but since the fprobe official interface becomes ftrace_regs, user naturally expects it is not pt_regs. The problem is that the perf's case. Since the perf is natively only support pt_regs (and there is no reason to support ftrace_regs, yes). Hmm, I will recheck how the perf events on trace-event is implementd. Thank you, > > > + } > > + return NULL; > > +} > > + > > +static __always_inline void perf_fprobe_return_regs(struct pt_regs *regs) > > +{ > > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > > + > > + if (WARN_ON_ONCE(regs != stack->regs[stack->idx])) > > This is missing an & too: > if (WARN_ON_ONCE(regs != &stack->regs[stack->idx])) > > > > > > + return; > > + > > + --stack->idx; > > +} > > + > > +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */
On Thu, 24 Aug 2023 00:16:37 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > +#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \ > + defined(CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST) > + > +static __always_inline > +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) > +{ > + /* See include/linux/ftrace.h, this returns &fregs->regs */ > + return ftrace_partial_regs(fregs, NULL); > +} > + > +#define perf_fprobe_return_regs(regs) do {} while (0) > + > +#else /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ > + > +/* Since fprobe handlers can be nested, pt_regs buffer need to be a stack */ > +#define PERF_FPROBE_REGS_MAX 4 > + > +struct pt_regs_stack { > + struct pt_regs regs[PERF_FPROBE_REGS_MAX]; > + int idx; > +}; > + > +static DEFINE_PER_CPU(struct pt_regs_stack, perf_fprobe_regs); > + > +static __always_inline > +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) > +{ > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > + struct pt_regs *regs; > + > + if (stack->idx < PERF_FPROBE_REGS_MAX) { > + regs = stack->regs[stack->idx++]; > + return ftrace_partial_regs(fregs, regs); > + } > + return NULL; > +} > + > +static __always_inline void perf_fprobe_return_regs(struct pt_regs *regs) > +{ > + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); > + > + if (WARN_ON_ONCE(regs != stack->regs[stack->idx])) > + return; > + > + --stack->idx; > +} Ah, I found that the perf_trace_buf_alloc() does the same thing. So perf_trace_buf_alloc(size, &pt_regs, &rctx); will give us the pt_regs at that point. Trace event does that so I think it is OK to do that here. Thank you, > + > +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ > + > static int fentry_perf_func(struct trace_fprobe *tf, unsigned long entry_ip, > - struct pt_regs *regs) > + struct ftrace_regs *fregs) > { > struct trace_event_call *call = trace_probe_event_call(&tf->tp); > struct fentry_trace_entry_head *entry; > struct hlist_head *head; > int size, __size, dsize; > + struct pt_regs *regs; > int rctx; > > + regs = perf_fprobe_partial_regs(fregs); > + if (!regs) > + return -EINVAL; > + > head = this_cpu_ptr(call->perf_events); > if (hlist_empty(head)) > - return 0; > + goto out; > > - dsize = __get_data_size(&tf->tp, regs); > + dsize = __get_data_size(&tf->tp, fregs); > __size = sizeof(*entry) + tf->tp.size + dsize; > size = ALIGN(__size + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > entry = perf_trace_buf_alloc(size, NULL, &rctx); Here, we can borrow the pt_regs. > if (!entry) > - return 0; > + goto out; > > entry->ip = entry_ip; > memset(&entry[1], 0, dsize); > - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize); > + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize); > perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, > head, NULL); > +out: > + perf_fprobe_return_regs(regs); > return 0; > } > NOKPROBE_SYMBOL(fentry_perf_func); > > static void > fexit_perf_func(struct trace_fprobe *tf, unsigned long entry_ip, > - unsigned long ret_ip, struct pt_regs *regs) > + unsigned long ret_ip, struct ftrace_regs *fregs) > { > struct trace_event_call *call = trace_probe_event_call(&tf->tp); > struct fexit_trace_entry_head *entry; > struct hlist_head *head; > int size, __size, dsize; > + struct pt_regs *regs; > int rctx; > > + regs = perf_fprobe_partial_regs(fregs); > + if (!regs) > + return; > + > head = this_cpu_ptr(call->perf_events); > if (hlist_empty(head)) > - return; > + goto out; > > - dsize = __get_data_size(&tf->tp, regs); > + dsize = __get_data_size(&tf->tp, fregs); > __size = sizeof(*entry) + tf->tp.size + dsize; > size = ALIGN(__size + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > entry = perf_trace_buf_alloc(size, NULL, &rctx); Ditto. Thanks,
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index a6ed2aa71efc..fb0f87d19d35 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -194,6 +194,21 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) return ftrace_get_regs(fregs) != NULL; } +#ifdef CONFIG_HAVE_REGS_AND_STACK_ACCESS_API +static __always_inline unsigned long +ftrace_regs_get_kernel_stack_nth(struct ftrace_regs *fregs, unsigned int nth) +{ + unsigned long *stackp; + + stackp = (unsigned long *)ftrace_regs_get_stack_pointer(fregs); + if (((unsigned long)(stackp + nth) & ~(THREAD_SIZE - 1)) == + ((unsigned long)stackp & ~(THREAD_SIZE - 1))) + return *(stackp + nth); + + return 0; +} +#endif /* CONFIG_HAVE_REGS_AND_STACK_ACCESS_API */ + #ifdef CONFIG_FUNCTION_TRACER extern int ftrace_enabled; diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index d56304276318..6fb4ecf8767d 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -679,7 +679,6 @@ config FPROBE_EVENTS select TRACING select PROBE_EVENTS select DYNAMIC_EVENTS - depends on DYNAMIC_FTRACE_WITH_REGS default y help This allows user to add tracing events on the function entry and diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index c60d0d9f1a95..90ad28260a9f 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -132,7 +132,7 @@ static int process_fetch_insn(struct fetch_insn *code, void *rec, void *dest, void *base) { - struct pt_regs *regs = rec; + struct ftrace_regs *fregs = rec; unsigned long val; int ret; @@ -140,17 +140,17 @@ process_fetch_insn(struct fetch_insn *code, void *rec, void *dest, /* 1st stage: get value from context */ switch (code->op) { case FETCH_OP_STACK: - val = regs_get_kernel_stack_nth(regs, code->param); + val = ftrace_regs_get_kernel_stack_nth(fregs, code->param); break; case FETCH_OP_STACKP: - val = kernel_stack_pointer(regs); + val = ftrace_regs_get_stack_pointer(fregs); break; case FETCH_OP_RETVAL: - val = regs_return_value(regs); + val = ftrace_regs_return_value(fregs); break; #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API case FETCH_OP_ARG: - val = regs_get_kernel_argument(regs, code->param); + val = ftrace_regs_get_argument(fregs, code->param); break; #endif case FETCH_NOP_SYMBOL: /* Ignore a place holder */ @@ -170,7 +170,7 @@ NOKPROBE_SYMBOL(process_fetch_insn) /* function entry handler */ static nokprobe_inline void __fentry_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, - struct pt_regs *regs, + struct ftrace_regs *fregs, struct trace_event_file *trace_file) { struct fentry_trace_entry_head *entry; @@ -184,36 +184,36 @@ __fentry_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, if (trace_trigger_soft_disabled(trace_file)) return; - dsize = __get_data_size(&tf->tp, regs); + dsize = __get_data_size(&tf->tp, fregs); entry = trace_event_buffer_reserve(&fbuffer, trace_file, sizeof(*entry) + tf->tp.size + dsize); if (!entry) return; - fbuffer.regs = regs; + fbuffer.regs = ftrace_get_regs(fregs); entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event); entry->ip = entry_ip; - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize); + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize); trace_event_buffer_commit(&fbuffer); } static void fentry_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, - struct pt_regs *regs) + struct ftrace_regs *fregs) { struct event_file_link *link; trace_probe_for_each_link_rcu(link, &tf->tp) - __fentry_trace_func(tf, entry_ip, regs, link->file); + __fentry_trace_func(tf, entry_ip, fregs, link->file); } NOKPROBE_SYMBOL(fentry_trace_func); /* Kretprobe handler */ static nokprobe_inline void __fexit_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, - unsigned long ret_ip, struct pt_regs *regs, + unsigned long ret_ip, struct ftrace_regs *fregs, struct trace_event_file *trace_file) { struct fexit_trace_entry_head *entry; @@ -227,94 +227,157 @@ __fexit_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, if (trace_trigger_soft_disabled(trace_file)) return; - dsize = __get_data_size(&tf->tp, regs); + dsize = __get_data_size(&tf->tp, fregs); entry = trace_event_buffer_reserve(&fbuffer, trace_file, sizeof(*entry) + tf->tp.size + dsize); if (!entry) return; - fbuffer.regs = regs; + fbuffer.regs = ftrace_get_regs(fregs); entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event); entry->func = entry_ip; entry->ret_ip = ret_ip; - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize); + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize); trace_event_buffer_commit(&fbuffer); } static void fexit_trace_func(struct trace_fprobe *tf, unsigned long entry_ip, - unsigned long ret_ip, struct pt_regs *regs) + unsigned long ret_ip, struct ftrace_regs *fregs) { struct event_file_link *link; trace_probe_for_each_link_rcu(link, &tf->tp) - __fexit_trace_func(tf, entry_ip, ret_ip, regs, link->file); + __fexit_trace_func(tf, entry_ip, ret_ip, fregs, link->file); } NOKPROBE_SYMBOL(fexit_trace_func); #ifdef CONFIG_PERF_EVENTS +#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \ + defined(CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST) + +static __always_inline +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) +{ + /* See include/linux/ftrace.h, this returns &fregs->regs */ + return ftrace_partial_regs(fregs, NULL); +} + +#define perf_fprobe_return_regs(regs) do {} while (0) + +#else /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ + +/* Since fprobe handlers can be nested, pt_regs buffer need to be a stack */ +#define PERF_FPROBE_REGS_MAX 4 + +struct pt_regs_stack { + struct pt_regs regs[PERF_FPROBE_REGS_MAX]; + int idx; +}; + +static DEFINE_PER_CPU(struct pt_regs_stack, perf_fprobe_regs); + +static __always_inline +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs) +{ + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); + struct pt_regs *regs; + + if (stack->idx < PERF_FPROBE_REGS_MAX) { + regs = stack->regs[stack->idx++]; + return ftrace_partial_regs(fregs, regs); + } + return NULL; +} + +static __always_inline void perf_fprobe_return_regs(struct pt_regs *regs) +{ + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs); + + if (WARN_ON_ONCE(regs != stack->regs[stack->idx])) + return; + + --stack->idx; +} + +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */ + static int fentry_perf_func(struct trace_fprobe *tf, unsigned long entry_ip, - struct pt_regs *regs) + struct ftrace_regs *fregs) { struct trace_event_call *call = trace_probe_event_call(&tf->tp); struct fentry_trace_entry_head *entry; struct hlist_head *head; int size, __size, dsize; + struct pt_regs *regs; int rctx; + regs = perf_fprobe_partial_regs(fregs); + if (!regs) + return -EINVAL; + head = this_cpu_ptr(call->perf_events); if (hlist_empty(head)) - return 0; + goto out; - dsize = __get_data_size(&tf->tp, regs); + dsize = __get_data_size(&tf->tp, fregs); __size = sizeof(*entry) + tf->tp.size + dsize; size = ALIGN(__size + sizeof(u32), sizeof(u64)); size -= sizeof(u32); entry = perf_trace_buf_alloc(size, NULL, &rctx); if (!entry) - return 0; + goto out; entry->ip = entry_ip; memset(&entry[1], 0, dsize); - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize); + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize); perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, head, NULL); +out: + perf_fprobe_return_regs(regs); return 0; } NOKPROBE_SYMBOL(fentry_perf_func); static void fexit_perf_func(struct trace_fprobe *tf, unsigned long entry_ip, - unsigned long ret_ip, struct pt_regs *regs) + unsigned long ret_ip, struct ftrace_regs *fregs) { struct trace_event_call *call = trace_probe_event_call(&tf->tp); struct fexit_trace_entry_head *entry; struct hlist_head *head; int size, __size, dsize; + struct pt_regs *regs; int rctx; + regs = perf_fprobe_partial_regs(fregs); + if (!regs) + return; + head = this_cpu_ptr(call->perf_events); if (hlist_empty(head)) - return; + goto out; - dsize = __get_data_size(&tf->tp, regs); + dsize = __get_data_size(&tf->tp, fregs); __size = sizeof(*entry) + tf->tp.size + dsize; size = ALIGN(__size + sizeof(u32), sizeof(u64)); size -= sizeof(u32); entry = perf_trace_buf_alloc(size, NULL, &rctx); if (!entry) - return; + goto out; entry->func = entry_ip; entry->ret_ip = ret_ip; - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize); + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize); perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, head, NULL); +out: + perf_fprobe_return_regs(regs); } NOKPROBE_SYMBOL(fexit_perf_func); #endif /* CONFIG_PERF_EVENTS */ @@ -324,17 +387,14 @@ static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip, void *entry_data) { struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp); - struct pt_regs *regs = ftrace_get_regs(fregs); int ret = 0; - if (!regs) - return 0; - if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE)) - fentry_trace_func(tf, entry_ip, regs); + fentry_trace_func(tf, entry_ip, fregs); + #ifdef CONFIG_PERF_EVENTS if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE)) - ret = fentry_perf_func(tf, entry_ip, regs); + ret = fentry_perf_func(tf, entry_ip, fregs); #endif return ret; } @@ -345,16 +405,13 @@ static void fexit_dispatcher(struct fprobe *fp, unsigned long entry_ip, void *entry_data) { struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp); - struct pt_regs *regs = ftrace_get_regs(fregs); - - if (!regs) - return; if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE)) - fexit_trace_func(tf, entry_ip, ret_ip, regs); + fexit_trace_func(tf, entry_ip, ret_ip, fregs); + #ifdef CONFIG_PERF_EVENTS if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE)) - fexit_perf_func(tf, entry_ip, ret_ip, regs); + fexit_perf_func(tf, entry_ip, ret_ip, fregs); #endif } NOKPROBE_SYMBOL(fexit_dispatcher); diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h index 3935b347f874..05445a745a07 100644 --- a/kernel/trace/trace_probe_tmpl.h +++ b/kernel/trace/trace_probe_tmpl.h @@ -232,7 +232,7 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val, /* Sum up total data length for dynamic arrays (strings) */ static nokprobe_inline int -__get_data_size(struct trace_probe *tp, struct pt_regs *regs) +__get_data_size(struct trace_probe *tp, void *regs) { struct probe_arg *arg; int i, len, ret = 0;