> On Oct 24, 2025, at 4:42 AM, Jiri Olsa <[email protected]> wrote: > > On Fri, Oct 24, 2025 at 12:12:55AM -0700, Song Liu wrote: >> When livepatch is attached to the same function as bpf trampoline with >> a fexit program, bpf trampoline code calls register_ftrace_direct() >> twice. The first time will fail with -EAGAIN, and the second time it >> will succeed. This requires register_ftrace_direct() to unregister >> the address on the first attempt. Otherwise, the bpf trampoline cannot >> attach. Here is an easy way to reproduce this issue: >> >> insmod samples/livepatch/livepatch-sample.ko >> bpftrace -e 'fexit:cmdline_proc_show {}' >> ERROR: Unable to attach probe: fexit:vmlinux:cmdline_proc_show... >> >> Fix this by cleaning up the hash when register_ftrace_function_nolock hits >> errors. >> >> Fixes: d05cb470663a ("ftrace: Fix modification of direct_function hash while >> in use") >> Cc: [email protected] # v6.6+ >> Reported-by: Andrey Grodzovsky <[email protected]> >> Closes: >> https://lore.kernel.org/live-patching/[email protected]/ >> >> Cc: Steven Rostedt (Google) <[email protected]> >> Cc: Masami Hiramatsu (Google) <[email protected]> >> Acked-and-tested-by: Andrey Grodzovsky <[email protected]> >> Signed-off-by: Song Liu <[email protected]> >> --- >> kernel/trace/ftrace.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c >> index 42bd2ba68a82..7f432775a6b5 100644 >> --- a/kernel/trace/ftrace.c >> +++ b/kernel/trace/ftrace.c >> @@ -6048,6 +6048,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, >> unsigned long addr) >> ops->direct_call = addr; >> >> err = register_ftrace_function_nolock(ops); >> + if (err) >> + remove_direct_functions_hash(hash, addr); > > should this be handled by the caller of the register_ftrace_direct? > fops->hash is updated by ftrace_set_filter_ip in register_fentry
We need to clean up here. This is because register_ftrace_direct added the new entries to direct_functions. It need to clean these entries for the caller so that the next call of register_ftrace_direct can work. > seems like it's should be caller responsibility, also you could do that > just for (err == -EAGAIN) case to address the use case directly The cleanup is valid for any error cases, as we need to remove unused entries from direct_functions. Thanks, Song
