Re: [PATCH v19 19/19] bpf: Use ftrace_get_symaddr() in get_entry_ip()

Masami Hiramatsu
2024-12-09T09:30:02Z

On Mon, 11 Nov 2024 00:52:31 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Rewrite get_entry_ip() to use ftrace_get_symaddr() macro.

I found a root problem of this patch. This get_entry_ip() is used not
only for fprobe (kprobe_multi) but also kprobes, but that is wrong.
On x86, both kprobes and ftrace (fentry) have the same restriction,
it should avoid ENDBR. But on arm64, ftrace_get_symaddr() is only for
fprobe, and kp->addr should point the symbol address.

So what I should do is to use `ftrace_get_symaddr()` version for
fprobe (kprobe_multi) and keep this original function for kprobe.

Let me fix that.

Thanks,

>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Changes in v19:
> - Use ftrace_get_symaddr() instead of introducing new arch dependent code.
> - Also, replace x86 code with ftrace_get_symaddr(), which does the same
> thing.
> ---
> kernel/trace/bpf_trace.c | 19 ++-----------------
> 1 file changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1532e9172bf9..e848a782bc8d 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1024,27 +1024,12 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_tracing = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> -#ifdef CONFIG_X86_KERNEL_IBT
> static unsigned long get_entry_ip(unsigned long fentry_ip)
> {
> - u32 instr;
> + unsigned long ret = ftrace_get_symaddr(fentry_ip);
>
> - /* We want to be extra safe in case entry ip is on the page edge,
> - * but otherwise we need to avoid get_kernel_nofault()'s overhead.
> - */
> - if ((fentry_ip & ~PAGE_MASK) < ENDBR_INSN_SIZE) {
> - if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
> - return fentry_ip;
> - } else {
> - instr = *(u32 *)(fentry_ip - ENDBR_INSN_SIZE);
> - }
> - if (is_endbr(instr))
> - fentry_ip -= ENDBR_INSN_SIZE;
> - return fentry_ip;
> + return ret ? : fentry_ip;
> }
> -#else
> -#define get_entry_ip(fentry_ip) fentry_ip
> -#endif
>
> BPF_CALL_1(bpf_get_func_ip_kprobe, struct pt_regs *, regs)
> {
>
>

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>