Re: [PATCH bpf-next 1/3] ftrace: Fix BPF fexit with livepatch

[Song Liu <songliubraving@meta.com>]

> On Oct 24, 2025, at 4:42 AM, Jiri Olsa <olsajiri@gmail.com> 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: stable@vger.kernel.org # v6.6+
>> Reported-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
>> Closes: https://lore.kernel.org/live-patching/c5058315a39d4615b333e485893345be@crowdstrike.com/ 
>> Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>> Acked-and-tested-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
>> 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