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

[Daniel Borkmann <daniel@iogearbox.net>]

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