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

[Haiyue Wang]

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@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxx>
> Acked-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
>   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)
>   {