Re: [PATCH] ftrace: x86: Fix a compile error about get_kernel_nofault()

Haiyue Wang
2025-02-06T13:26:56Z

On 2025/2/6 20:33, Peter Zijlstra wrote:
> On Thu, Feb 06, 2025 at 01:13:28PM +0100, Peter Zijlstra wrote:
>
>> No, it just cleans up this utter trainwreck. I also noticed this is a
>> second (new) copy of this garbage. Clearly I didn't yell loud enough
>> last time and people didn't think to vomit when doing the copy/paste :/
>>
>> Function will look something like:
>>
>> unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
>> {
>> if (is_endbr(fentry_ip - ENDBR_INSN_SIZE))
>> fentry_op -= ENDBR_INSN_SIZE;
>> return fentry_ip;
>> }
>>
>> Let me finish local build test before I push out.
>
> Bah, still waiting for a LLVM build, but patch should be here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/ibt
>
> I was supposed to merge this last cycle, but then akpm shat all over
> arch/x86/kernel/alternative.c and we had to clean that up :/
>
> Notably, this patch, not sure it applies out of order.
>
> ---
>
> Subject: x86/ibt: Clean up is_endbr()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon Nov 27 09:58:06 CET 2023
>
> Pretty much every caller of is_endbr() actually wants to test something at an
> address and ends up doing get_kernel_nofault(). Fold the lot into a more
> convenient helper.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> arch/x86/events/core.c | 2 +-
> arch/x86/include/asm/ftrace.h | 17 +++--------------
> arch/x86/include/asm/ibt.h | 5 +++--
> arch/x86/kernel/alternative.c | 20 ++++++++++++++------
> arch/x86/kernel/kprobes/core.c | 11 +----------
> arch/x86/net/bpf_jit_comp.c | 4 ++--
> kernel/trace/bpf_trace.c | 20 +++-----------------
> 7 files changed, 27 insertions(+), 52 deletions(-)
>
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2857,7 +2857,7 @@ static bool is_uprobe_at_func_entry(stru
> return true;
>
> /* endbr64 (64-bit only) */
> - if (user_64bit_mode(regs) && is_endbr(*(u32 *)auprobe->insn))
> + if (user_64bit_mode(regs) && is_endbr((u32 *)auprobe->insn))
> return true;
>
> return false;
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -2,6 +2,7 @@
> #ifndef _ASM_X86_FTRACE_H
> #define _ASM_X86_FTRACE_H
>
> +#include "asm/ibt.h"

Then
https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/tree/arch/x86/include/asm/ftrace.h?h=x86/ibt#n17

# include <asm/ibt.h> --> This line can be removed ?

> #include <asm/ptrace.h>
>
> #ifdef CONFIG_FUNCTION_TRACER
> @@ -36,21 +37,9 @@ static inline unsigned long ftrace_call_
>
> static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
> {