linux-kernel - Re: [PATCH] x86/ibt: make is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250922065248.GO3245006@noisy.programming.kicks-ass.net>
Date: Mon, 22 Sep 2025 08:52:48 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Menglong Dong <menglong8.dong@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
	Jiri Olsa <jolsa@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, X86 ML <x86@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>, Kees Cook <kees@...nel.org>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	Mike Rapoport <rppt@...nel.org>, Andy Lutomirski <luto@...nel.org>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Alexei Starovoitov <ast@...nel.org>,
	Andrii Nakryiko <andrii@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH] x86/ibt: make is_endbr() notrace

On Fri, Sep 19, 2025 at 09:13:15AM +0800, Menglong Dong wrote:

> Ok, let me describe the problem in deetail.
> 
> First of all, it has nothing to do with kprobe. The bpf program of type
> kprobe-multi based on fprobe, and fprobe base on fgraph. So it's all
> about the ftrace, which means __fentry__.

Well, that's not confusing at all. Something called kprobe-multi not
being related to kprobes :-(

> Second, let me explain the recur detection of the kprobe-multi. Let's
> take the is_endbr() for example. When it is hooked by the bpf program
> of type kretprobe-multi, following calling chain will happen:
> 
>   is_endbr -> __ftrace_return_to_handler -> fprobe_return ->
>   kprobe_multi_link_exit_handler -> ftrace_get_entry_ip ->
>   arch_ftrace_get_symaddr -> is_endbr
> 
> Look, is_endbr() is called again during the ftrace handler, so it will
> trigger the ftrace handler(__ftrace_return_to_handler) again, which
> causes recurrence.

Right.

> Such recurrence can be detected. In kprobe_multi_link_prog_run(),
> the percpu various "bpf_prog_active" will be increased by 1 before we
> run the bpf progs, and decrease by 1 after the bpf progs finish. If the
> kprobe_multi_link_prog_run() is triggered again during bpf progs run,
> it will check if bpf_prog_active is zero, and return directly if it is not.
> Therefore, recurrence can't happen within the "bpf_prog_active" protection.

As I think Masami already said, the problem is the layer. You're trying
to fix an ftrace problem at the bpf layer.

> However, the calling to is_endbr() is not within that scope, which makes
> the recurrence happen.

Sorta, I'm still sketchy on the whole kprobe-multi thing.

Anyway, I don't mind making is_endbr() invisible to tracing, that might
just have security benefits too. But I think first the ftrace folks need
to figure out how to best kill that recursion, because I don't think
is_endbr is particularly special here.

It is just one more function that can emit a __fentry__ site.

Anyway, something like the below would do:

Note that without making __is_endbr() __always_inline, you run the risk
of the compiler being retarded (they often are in the face of
KASAN/UBSAN like) and deciding to out-of-line that function, resulting
in yet another __fentry__ site.

An added advantage of noinstr is that it is validated by objtool to
never call to !noinstr code. As such, you can be sure there is no
instrumentation in it.

(the below hasn't been near a compiler)

---
 arch/x86/include/asm/ibt.h    | 2 +-
 arch/x86/kernel/alternative.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/ibt.h b/arch/x86/include/asm/ibt.h
index 5e45d6424722..54937a527042 100644
--- a/arch/x86/include/asm/ibt.h
+++ b/arch/x86/include/asm/ibt.h
@@ -65,7 +65,7 @@ static __always_inline __attribute_const__ u32 gen_endbr_poison(void)
 	return 0xd6401f0f; /* nopl -42(%rax) */
 }
 
-static inline bool __is_endbr(u32 val)
+static __always_inline bool __is_endbr(u32 val)
 {
 	if (val == gen_endbr_poison())
 		return true;
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 69fb818df2ee..f791e7abd466 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1108,7 +1108,7 @@ void __init_or_module noinline apply_returns(s32 *start, s32 *end) { }
 
 #ifdef CONFIG_X86_KERNEL_IBT
 
-__noendbr bool is_endbr(u32 *val)
+__noendbr noinstr bool is_endbr(u32 *val)
 {
 	u32 endbr;