[PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[Daniel Borkmann]

Since commit 0c3772a8db1f ("x86/ftrace: Implement DYNAMIC_FTRACE_WITH_JMP")
ftrace_call_replace() emits a jmp instead of a call when bit 0 of the
target is set, the mark of a direct trampoline that is entered by a jmp.
ftrace_update_ftrace_func() and arch_ftrace_update_trampoline() use it
too, for the call of the ftrace callback in ftrace_caller, ftrace_regs_caller
and the ftrace trampolines, but a callback is a C function and can start
at an odd address. It does with gcc13 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y:
gcc13 has no -fmin-function-alignment, so CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT
is not set, and the -falign-functions=16 used in its place does not apply
to functions optimized for size. Here ftrace_ops_list_func() starts at:

  ffffffff810e0147 T ftrace_ops_list_func

ftrace_regs_call then becomes a jmp to the byte before it:

  ftrace_regs_call ffffffff810374f3: e9 4e 8c 0a 00

That byte is the last one of the preceding function's tail jump, and with
the first two of ftrace_ops_list_func() it decodes as an inc DWORD PTR
[rcx+0x57], which swallows the push %r15 of the prologue. Entered without
a return address and one push short, the list function returns through
the pt_regs that ftrace_regs_caller saved, here to the saved R14:

  BUG: unable to handle page fault for address: ffff888116c29f40
  #PF: supervisor instruction fetch in kernel mode
  RIP: 0010:0xffff888116c29f40
  R13: 0000000000000000 R14: ffff888116c29f40 R15: 0000000000000000
  Call Trace:
   ? bpf_trampoline_6442499918+0x1/0x149
   ? bpf_lsm_file_open+0x5/0x8
   ? bpf_lsm_file_open+0x5/0x8
   ? security_file_open+0x19/0x46
   ? do_dentry_open+0x16c/0x32a

The list function is installed whenever two ftrace_ops are registered,
and a function that two of them trace calls it. A kprobe next to a BPF
trampoline is one such case, and with the single ftrace_ops of the BPF
trampolines every update_ftrace_direct_mod() is another, as it registers
a stub ops for the sites it modifies. Here, detaching one of several BPF
LSM programs from a busy hook crashed the tasks that ran the hook during
the update.

Bit 0 has that meaning only for the direct trampolines that a dyn_ftrace
record can point to, so jmp mode relies on every other target being even.
Select HAVE_DYNAMIC_FTRACE_WITH_JMP only where it is, with
CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT or without CONFIG_CC_OPTIMIZE_FOR_SIZE.
arm64 does the same for HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS, which depends
on function alignment as well, but tests CONFIG_CC_IS_CLANG instead of
CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT, which also covers gcc 14 and its
-fmin-function-alignment. Other builds use the call mode for BPF
trampolines, as they did before that commit and as a trampoline sharing
its site with an IPMODIFY ops still does.

Fixes: 0c3772a8db1f ("x86/ftrace: Implement DYNAMIC_FTRACE_WITH_JMP")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Menglong Dong <dongml2@chinatelecom.cn>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3907f2f3d5b1..56118fcf036b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -232,7 +232,7 @@ config X86
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS	if X86_64
 	select HAVE_FTRACE_REGS_HAVING_PT_REGS	if X86_64
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
-	select HAVE_DYNAMIC_FTRACE_WITH_JMP	if X86_64
+	select HAVE_DYNAMIC_FTRACE_WITH_JMP	if X86_64 && (CC_HAS_SANE_FUNCTION_ALIGNMENT || !CC_OPTIMIZE_FOR_SIZE)
 	select HAVE_SAMPLE_FTRACE_DIRECT	if X86_64
 	select HAVE_SAMPLE_FTRACE_DIRECT_MULTI	if X86_64
 	select HAVE_EBPF_JIT
-- 
2.43.0

[PATCH bpf 2/2] selftests/bpf: Test kprobe and fentry programs on one function

[Daniel Borkmann]

A kprobe next to a BPF trampoline makes the ftrace site of the function
call ftrace_regs_caller and from there the list of ftrace callbacks,
the path that crashed while ftrace_ops_list_func() started at an odd
address. No test covers a function traced by both. Thus, attach an
fentry program and a kprobe to bpf_fentry_test1(), then a second fentry
program, which modifies the trampoline of the shared site, then detach
the kprobe. After each step, check that every attached program ran once
for the call and saw its argument.

Without the previous fix the test crashes the kernel, but only where
ftrace_ops_list_func() is at an odd address (in my test with gcc +
CONFIG_CC_OPTIMIZE_FOR_SIZE=y). With the fix it passes:

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t fentry_kprobe_shared
  [...]
  #147     fentry_kprobe_shared:OK
  Summary: 1/0 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Menglong Dong <dongml2@chinatelecom.cn>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 .../bpf/prog_tests/fentry_kprobe_shared.c     | 54 +++++++++++++++++++
 .../bpf/progs/fentry_kprobe_shared.c          | 40 ++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_kprobe_shared.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_kprobe_shared.c

diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_kprobe_shared.c b/tools/testing/selftests/bpf/prog_tests/fentry_kprobe_shared.c
new file mode 100644
index 000000000000..b53ddb983f6f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_kprobe_shared.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "fentry_kprobe_shared.skel.h"
+
+static void trigger(struct fentry_kprobe_shared *skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err;
+
+	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.fentry1), &topts);
+	ASSERT_OK(err, "test_run");
+}
+
+/*
+ * With a kprobe next to the trampoline, the ftrace site of the function
+ * reaches the trampoline through the list of ftrace callbacks, and so does
+ * the site of a trampoline that is modified to add or remove a program.
+ */
+void test_fentry_kprobe_shared(void)
+{
+	struct fentry_kprobe_shared *skel;
+
+	skel = fentry_kprobe_shared__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	skel->bss->pid = getpid();
+
+	skel->links.fentry1 = bpf_program__attach(skel->progs.fentry1);
+	if (!ASSERT_OK_PTR(skel->links.fentry1, "attach_fentry1"))
+		goto out;
+	skel->links.kprobe1 = bpf_program__attach(skel->progs.kprobe1);
+	if (!ASSERT_OK_PTR(skel->links.kprobe1, "attach_kprobe1"))
+		goto out;
+	trigger(skel);
+	ASSERT_EQ(skel->bss->fentry1_hits, 1, "fentry1_hits");
+	ASSERT_EQ(skel->bss->kprobe_hits, 1, "kprobe_hits");
+
+	skel->links.fentry2 = bpf_program__attach(skel->progs.fentry2);
+	if (!ASSERT_OK_PTR(skel->links.fentry2, "attach_fentry2"))
+		goto out;
+	trigger(skel);
+	ASSERT_EQ(skel->bss->fentry1_hits, 2, "fentry1_hits");
+	ASSERT_EQ(skel->bss->fentry2_hits, 1, "fentry2_hits");
+	ASSERT_EQ(skel->bss->kprobe_hits, 2, "kprobe_hits");
+
+	bpf_link__destroy(skel->links.kprobe1);
+	skel->links.kprobe1 = NULL;
+	trigger(skel);
+	ASSERT_EQ(skel->bss->fentry1_hits, 3, "fentry1_hits");
+	ASSERT_EQ(skel->bss->fentry2_hits, 2, "fentry2_hits");
+	ASSERT_EQ(skel->bss->kprobe_hits, 2, "kprobe_hits");
+out:
+	fentry_kprobe_shared__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/fentry_kprobe_shared.c b/tools/testing/selftests/bpf/progs/fentry_kprobe_shared.c
new file mode 100644
index 000000000000..aae8d9d29d37
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fentry_kprobe_shared.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+int pid;
+int fentry1_hits;
+int fentry2_hits;
+int kprobe_hits;
+
+static bool from_test(int a)
+{
+	return (bpf_get_current_pid_tgid() >> 32) == pid && a == 1;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(fentry1, int a)
+{
+	if (from_test(a))
+		fentry1_hits++;
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(fentry2, int a)
+{
+	if (from_test(a))
+		fentry2_hits++;
+	return 0;
+}
+
+SEC("kprobe/bpf_fentry_test1")
+int BPF_KPROBE(kprobe1, int a)
+{
+	if (from_test(a))
+		kprobe_hits++;
+	return 0;
+}
-- 
2.43.0

Re: [PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[Alexei Starovoitov]

On Sat, Oct 24, 2026 at 12:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> Bit 0 has that meaning only for the direct trampolines that a dyn_ftrace
> record can point to, so jmp mode relies on every other target being even.
> Select HAVE_DYNAMIC_FTRACE_WITH_JMP only where it is, with
> CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT or without CONFIG_CC_OPTIMIZE_FOR_SIZE.

Disabling it for -Os builds is not great.
Can we fix the users of ftrace_call_replace() instead ?

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 17d6edf..654c3f2 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -190,11 +190,11 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	const char *new;

 	ip = (unsigned long)(&ftrace_call);
-	new = ftrace_call_replace(ip, (unsigned long)func);
+	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
 	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);

 	ip = (unsigned long)(&ftrace_regs_call);
-	new = ftrace_call_replace(ip, (unsigned long)func);
+	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
 	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);

 	return 0;
@@ -503,7 +503,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)

 	mutex_lock(&text_mutex);
 	/* Do a safe modify in case the trampoline is executing */
-	new = ftrace_call_replace(ip, (unsigned long)func);
+	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
 	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
 	mutex_unlock(&text_mutex);
 }

Re: [PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[Jiri Olsa]

On Sat, Oct 24, 2026 at 07:10:57PM +0000, Alexei Starovoitov wrote:
> On Sat, Oct 24, 2026 at 12:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > Bit 0 has that meaning only for the direct trampolines that a dyn_ftrace
> > record can point to, so jmp mode relies on every other target being even.
> > Select HAVE_DYNAMIC_FTRACE_WITH_JMP only where it is, with
> > CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT or without CONFIG_CC_OPTIMIZE_FOR_SIZE.
> 
> Disabling it for -Os builds is not great.
> Can we fix the users of ftrace_call_replace() instead ?

looks good, it takes care of the C function targets, the rest
should be aligned ftrace trampolines (static or dynamic) or
direct api targets

jirka

> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 17d6edf..654c3f2 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -190,11 +190,11 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
>  	const char *new;
> 
>  	ip = (unsigned long)(&ftrace_call);
> -	new = ftrace_call_replace(ip, (unsigned long)func);
> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>  	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
> 
>  	ip = (unsigned long)(&ftrace_regs_call);
> -	new = ftrace_call_replace(ip, (unsigned long)func);
> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>  	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
> 
>  	return 0;
> @@ -503,7 +503,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
> 
>  	mutex_lock(&text_mutex);
>  	/* Do a safe modify in case the trampoline is executing */
> -	new = ftrace_call_replace(ip, (unsigned long)func);
> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>  	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
>  	mutex_unlock(&text_mutex);
>  }
> 

Re: [PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[Daniel Borkmann]

On 10/25/26 2:18 PM, Jiri Olsa wrote:
> On Sat, Oct 24, 2026 at 07:10:57PM +0000, Alexei Starovoitov wrote:
>> On Sat, Oct 24, 2026 at 12:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> Bit 0 has that meaning only for the direct trampolines that a dyn_ftrace
>>> record can point to, so jmp mode relies on every other target being even.
>>> Select HAVE_DYNAMIC_FTRACE_WITH_JMP only where it is, with
>>> CONFIG_CC_HAS_SANE_FUNCTION_ALIGNMENT or without CONFIG_CC_OPTIMIZE_FOR_SIZE.
>>
>> Disabling it for -Os builds is not great.
>> Can we fix the users of ftrace_call_replace() instead ?
> 
> looks good, it takes care of the C function targets, the rest
> should be aligned ftrace trampolines (static or dynamic) or
> direct api targets

I had the below first as well, but then saw that there were similar issues
regarding function alignment in the past and they were solved on kconfig
level, some picks:

- b3f11af9b2ce ("arm64: ftrace: forbid CALL_OPS with CC_OPTIMIZE_FOR_SIZE")
- a743f26d03a9 ("arm64: ftrace: Don't forbid CALL_OPS+CC_OPTIMIZE_FOR_SIZE with Clang")
- c41bf4326c7b ("riscv: ftrace: align patchable functions to 4 Byte boundary")
- 9315e22b0c0a ("arm64: ftrace: allow DIRECT_CALLS without CALL_OPS")

But I can also spin a v2 with the code change instead if preferred.

>> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
>> index 17d6edf..654c3f2 100644
>> --- a/arch/x86/kernel/ftrace.c
>> +++ b/arch/x86/kernel/ftrace.c
>> @@ -190,11 +190,11 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
>>   	const char *new;
>>
>>   	ip = (unsigned long)(&ftrace_call);
>> -	new = ftrace_call_replace(ip, (unsigned long)func);
>> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>>   	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
>>
>>   	ip = (unsigned long)(&ftrace_regs_call);
>> -	new = ftrace_call_replace(ip, (unsigned long)func);
>> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>>   	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
>>
>>   	return 0;
>> @@ -503,7 +503,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
>>
>>   	mutex_lock(&text_mutex);
>>   	/* Do a safe modify in case the trampoline is executing */
>> -	new = ftrace_call_replace(ip, (unsigned long)func);
>> +	new = text_gen_insn(CALL_INSN_OPCODE, (void *)ip, func);
>>   	smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
>>   	mutex_unlock(&text_mutex);
>>   }

Re: [PATCH bpf 1/2] x86/ftrace: Use jmp mode only where functions are aligned

[Alexei Starovoitov]

On Sun, Oct 25, 2026 at 08:27 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> I had the below first as well, but then saw that there were similar issues
> regarding function alignment in the past and they were solved on kconfig
> level, some picks:
>
> - b3f11af9b2ce ("arm64: ftrace: forbid CALL_OPS with CC_OPTIMIZE_FOR_SIZE")
> - a743f26d03a9 ("arm64: ftrace: Don't forbid CALL_OPS+CC_OPTIMIZE_FOR_SIZE with Clang")
> - c41bf4326c7b ("riscv: ftrace: align patchable functions to 4 Byte boundary")
> - 9315e22b0c0a ("arm64: ftrace: allow DIRECT_CALLS without CALL_OPS")

I think those are different. They need the traced function to be aligned
by design. CALL_OPS keeps an 8 byte literal in front of it.
Here nothing needs C functions to be aligned. The callback is hit by
the bit 0 check only because it goes through ftrace_call_replace().

> But I can also spin a v2 with the code change instead if preferred.

I don't insist, but imo my approach is cleaner.