On Mon, Dec 18, 2023 at 10:16:23PM +0900, Masami Hiramatsu (Google) wrote:

SNIP

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 84e8a0f6e4e0..d3f8745d8ead 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2503,7 +2503,7 @@ static int __init bpf_event_init(void)
>  fs_initcall(bpf_event_init);
>  #endif /* CONFIG_MODULES */
>  
> -#ifdef CONFIG_FPROBE
> +#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
>  struct bpf_kprobe_multi_link {
>       struct bpf_link link;
>       struct fprobe fp;
> @@ -2733,10 +2733,14 @@ kprobe_multi_link_prog_run(struct 
> bpf_kprobe_multi_link *link,
>  
>  static int
>  kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> -                       unsigned long ret_ip, struct pt_regs *regs,
> +                       unsigned long ret_ip, struct ftrace_regs *fregs,
>                         void *data)
>  {
>       struct bpf_kprobe_multi_link *link;
> +     struct pt_regs *regs = ftrace_get_regs(fregs);
> +
> +     if (!regs)
> +             return 0;
>  
>       link = container_of(fp, struct bpf_kprobe_multi_link, fp);
>       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> @@ -3008,7 +3012,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr 
> *attr, struct bpf_prog *pr
>       kvfree(cookies);
>       return err;
>  }
> -#else /* !CONFIG_FPROBE */
> +#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog 
> *prog)
>  {
>       return -EOPNOTSUPP;
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 6cd2a4e3afb8..f12569494d8a 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -46,7 +46,7 @@ static inline void __fprobe_handler(unsigned long ip, 
> unsigned long parent_ip,
>       }
>  
>       if (fp->entry_handler)
> -             ret = fp->entry_handler(fp, ip, parent_ip, 
> ftrace_get_regs(fregs), entry_data);
> +             ret = fp->entry_handler(fp, ip, parent_ip, fregs, entry_data);
>  
>       /* If entry_handler returns !0, nmissed is not counted. */
>       if (rh) {
> @@ -182,7 +182,7 @@ static void fprobe_init(struct fprobe *fp)
>               fp->ops.func = fprobe_kprobe_handler;
>       else
>               fp->ops.func = fprobe_handler;
> -     fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
> +     fp->ops.flags |= FTRACE_OPS_FL_SAVE_ARGS;

so with this change you move to ftrace_caller trampoline,
but we need ftrace_regs_caller right?

otherwise the (!regs) check in kprobe_multi_link_handler
will be allways true IIUC

jirka

>  }
>  
>  static int fprobe_init_rethook(struct fprobe *fp, int num)
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 7d2ddbcfa377..ef6b36fd05ae 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -320,12 +320,16 @@ NOKPROBE_SYMBOL(fexit_perf_func);
>  #endif       /* CONFIG_PERF_EVENTS */
>  
>  static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
> -                          unsigned long ret_ip, struct pt_regs *regs,
> +                          unsigned long ret_ip, struct ftrace_regs *fregs,
>                            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);
>  #ifdef CONFIG_PERF_EVENTS
> diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
> index 24de0e5ff859..ff607babba18 100644
> --- a/lib/test_fprobe.c
> +++ b/lib/test_fprobe.c
> @@ -40,7 +40,7 @@ static noinline u32 fprobe_selftest_nest_target(u32 value, 
> u32 (*nest)(u32))
>  
>  static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
>                                   unsigned long ret_ip,
> -                                 struct pt_regs *regs, void *data)
> +                                 struct ftrace_regs *fregs, void *data)
>  {
>       KUNIT_EXPECT_FALSE(current_test, preemptible());
>       /* This can be called on the fprobe_selftest_target and the 
> fprobe_selftest_target2 */
> @@ -81,7 +81,7 @@ static notrace void fp_exit_handler(struct fprobe *fp, 
> unsigned long ip,
>  
>  static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
>                                     unsigned long ret_ip,
> -                                   struct pt_regs *regs, void *data)
> +                                   struct ftrace_regs *fregs, void *data)
>  {
>       KUNIT_EXPECT_FALSE(current_test, preemptible());
>       return 0;
> diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
> index 64e715e7ed11..1545a1aac616 100644
> --- a/samples/fprobe/fprobe_example.c
> +++ b/samples/fprobe/fprobe_example.c
> @@ -50,7 +50,7 @@ static void show_backtrace(void)
>  
>  static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
>                               unsigned long ret_ip,
> -                             struct pt_regs *regs, void *data)
> +                             struct ftrace_regs *fregs, void *data)
>  {
>       if (use_trace)
>               /*
> 

Reply via email to