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@crowd... 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);
out_unlock: mutex_unlock(&direct_mutex);
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@crowd... 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
seems like it's should be caller responsibility, also you could do that just for (err == -EAGAIN) case to address the use case directly
jirka
out_unlock: mutex_unlock(&direct_mutex); -- 2.47.3
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@crowd... 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
On Fri, Oct 24, 2025 at 03:42:44PM +0000, Song Liu wrote:
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@crowd... 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.
I see, I wonder then we could use free_hash to restore original direct_functions, something like:
if (err) { call_direct_funcs = rcu_assign_pointer(free_hash); free_hash = new_hash; }
we'd need to keep new_hash value
but feel free to ignore, removal is also fine ;-)
thanks, jirka
linux-stable-mirror@lists.linaro.org
-
Jiri Olsa -
Song Liu -
Song Liu