* [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch
2025-10-26 20:54 [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Song Liu
@ 2025-10-26 20:54 ` Song Liu
2025-10-27 8:48 ` Jiri Olsa
2025-10-27 17:01 ` Steven Rostedt
2025-10-26 20:54 ` [PATCH v3 bpf 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct() Song Liu
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Song Liu @ 2025-10-26 20:54 UTC (permalink / raw)
To: bpf, linux-trace-kernel, live-patching
Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
kernel-team, olsajiri, Song Liu, stable
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.
Also, move the code that resets ops->func and ops->trampoline to
the error path of register_ftrace_direct().
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/bpf/trampoline.c | 5 -----
kernel/trace/ftrace.c | 6 ++++++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 5949095e51c3..f2cb0b097093 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -479,11 +479,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
* BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
* trampoline again, and retry register.
*/
- /* reset fops->func and fops->trampoline for re-register */
- tr->fops->func = NULL;
- tr->fops->trampoline = 0;
-
- /* free im memory and reallocate later */
bpf_tramp_image_free(im);
goto again;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a82..725c224fb4e6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
ops->direct_call = addr;
err = register_ftrace_function_nolock(ops);
+ if (err) {
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ remove_direct_functions_hash(hash, addr);
+ }
out_unlock:
mutex_unlock(&direct_mutex);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch
2025-10-26 20:54 ` [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch Song Liu
@ 2025-10-27 8:48 ` Jiri Olsa
2025-10-27 17:10 ` Song Liu
2025-10-27 17:01 ` Steven Rostedt
1 sibling, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2025-10-27 8:48 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
rostedt, andrey.grodzovsky, mhiramat, kernel-team, olsajiri,
stable
On Sun, Oct 26, 2025 at 01:54:43PM -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.
>
> Also, move the code that resets ops->func and ops->trampoline to
> the error path of register_ftrace_direct().
>
> 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/bpf/trampoline.c | 5 -----
> kernel/trace/ftrace.c | 6 ++++++
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5949095e51c3..f2cb0b097093 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -479,11 +479,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
> * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
> * trampoline again, and retry register.
> */
> - /* reset fops->func and fops->trampoline for re-register */
> - tr->fops->func = NULL;
> - tr->fops->trampoline = 0;
> -
> - /* free im memory and reallocate later */
> bpf_tramp_image_free(im);
> goto again;
> }
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 42bd2ba68a82..725c224fb4e6 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> ops->direct_call = addr;
>
> err = register_ftrace_function_nolock(ops);
> + if (err) {
> + /* cleanup for possible another register call */
> + ops->func = NULL;
> + ops->trampoline = 0;
nit, we could cleanup also flags and direct_call just to be complete,
but at the same time it does not seem to affect anything
jirka
> + remove_direct_functions_hash(hash, addr);
> + }
>
> out_unlock:
> mutex_unlock(&direct_mutex);
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch
2025-10-27 8:48 ` Jiri Olsa
@ 2025-10-27 17:10 ` Song Liu
0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2025-10-27 17:10 UTC (permalink / raw)
To: Jiri Olsa
Cc: Song Liu, bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
live-patching@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, rostedt@goodmis.org,
andrey.grodzovsky@crowdstrike.com, mhiramat@kernel.org,
Kernel Team, stable@vger.kernel.org
> On Oct 27, 2025, at 1:48 AM, Jiri Olsa <olsajiri@gmail.com> wrote:
[…]
>
>>
>>
>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
>> index 5949095e51c3..f2cb0b097093 100644
>> --- a/kernel/bpf/trampoline.c
>> +++ b/kernel/bpf/trampoline.c
>> @@ -479,11 +479,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>> * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
>> * trampoline again, and retry register.
>> */
>> - /* reset fops->func and fops->trampoline for re-register */
>> - tr->fops->func = NULL;
>> - tr->fops->trampoline = 0;
>> -
>> - /* free im memory and reallocate later */
>> bpf_tramp_image_free(im);
>> goto again;
>> }
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 42bd2ba68a82..725c224fb4e6 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>> ops->direct_call = addr;
>>
>> err = register_ftrace_function_nolock(ops);
>> + if (err) {
>> + /* cleanup for possible another register call */
>> + ops->func = NULL;
>> + ops->trampoline = 0;
>
> nit, we could cleanup also flags and direct_call just to be complete,
> but at the same time it does not seem to affect anything
I actually thought about this and decided to use the same
logic as unregister_ftrace_direct().
Thanks,
Song
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch
2025-10-26 20:54 ` [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch Song Liu
2025-10-27 8:48 ` Jiri Olsa
@ 2025-10-27 17:01 ` Steven Rostedt
2025-10-27 17:11 ` Song Liu
1 sibling, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2025-10-27 17:01 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
andrey.grodzovsky, mhiramat, kernel-team, olsajiri, stable
On Sun, 26 Oct 2025 13:54:43 -0700
Song Liu <song@kernel.org> wrote:
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> ops->direct_call = addr;
>
> err = register_ftrace_function_nolock(ops);
> + if (err) {
> + /* cleanup for possible another register call */
> + ops->func = NULL;
> + ops->trampoline = 0;
> + remove_direct_functions_hash(hash, addr);
> + }
>
As you AI bot noticed that it was missing what unregister_ftrace_direct()
does, instead, can we make a helper function that both use? This way it
will not get out of sync again.
static void reset_direct(struct ftrace_ops *ops, unsigned long addr)
{
struct ftrace_hash *hash = ops->func_hash->filter_hash;
ops->func = NULL;
ops->trampoline = 0;
remove_direct_functions_hash(hash, addr);
}
Then we could have:
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0c91247a95ab..51c3f5d46fde 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6062,7 +6062,7 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
err = register_ftrace_function_nolock(ops);
if (err)
- remove_direct_functions_hash(hash, addr);
+ reset_direct(ops, addr);
out_unlock:
mutex_unlock(&direct_mutex);
@@ -6095,7 +6095,6 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct);
int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
bool free_filters)
{
- struct ftrace_hash *hash = ops->func_hash->filter_hash;
int err;
if (check_direct_multi(ops))
@@ -6105,13 +6104,9 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
mutex_lock(&direct_mutex);
err = unregister_ftrace_function(ops);
- remove_direct_functions_hash(hash, addr);
+ reset_direct(ops, addr);
mutex_unlock(&direct_mutex);
- /* cleanup for possible another register call */
- ops->func = NULL;
- ops->trampoline = 0;
-
if (free_filters)
ftrace_free_filter(ops);
return err;
-- Steve
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch
2025-10-27 17:01 ` Steven Rostedt
@ 2025-10-27 17:11 ` Song Liu
0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2025-10-27 17:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Song Liu, bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
live-patching@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org,
andrey.grodzovsky@crowdstrike.com, mhiramat@kernel.org,
Kernel Team, olsajiri@gmail.com, stable@vger.kernel.org
> On Oct 27, 2025, at 10:01 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sun, 26 Oct 2025 13:54:43 -0700
> Song Liu <song@kernel.org> wrote:
>
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>> ops->direct_call = addr;
>>
>> err = register_ftrace_function_nolock(ops);
>> + if (err) {
>> + /* cleanup for possible another register call */
>> + ops->func = NULL;
>> + ops->trampoline = 0;
>> + remove_direct_functions_hash(hash, addr);
>> + }
>>
>
> As you AI bot noticed that it was missing what unregister_ftrace_direct()
> does, instead, can we make a helper function that both use? This way it
> will not get out of sync again.
>
> static void reset_direct(struct ftrace_ops *ops, unsigned long addr)
> {
> struct ftrace_hash *hash = ops->func_hash->filter_hash;
>
> ops->func = NULL;
> ops->trampoline = 0;
> remove_direct_functions_hash(hash, addr);
> }
>
> Then we could have:
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 0c91247a95ab..51c3f5d46fde 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6062,7 +6062,7 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>
> err = register_ftrace_function_nolock(ops);
> if (err)
> - remove_direct_functions_hash(hash, addr);
> + reset_direct(ops, addr);
>
> out_unlock:
> mutex_unlock(&direct_mutex);
> @@ -6095,7 +6095,6 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct);
> int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
> bool free_filters)
> {
> - struct ftrace_hash *hash = ops->func_hash->filter_hash;
> int err;
>
> if (check_direct_multi(ops))
> @@ -6105,13 +6104,9 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
>
> mutex_lock(&direct_mutex);
> err = unregister_ftrace_function(ops);
> - remove_direct_functions_hash(hash, addr);
> + reset_direct(ops, addr);
> mutex_unlock(&direct_mutex);
>
> - /* cleanup for possible another register call */
> - ops->func = NULL;
> - ops->trampoline = 0;
> -
> if (free_filters)
> ftrace_free_filter(ops);
> return err;
Make sense. I will use this in v4.
Thanks,
Song
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 bpf 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
2025-10-26 20:54 [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Song Liu
2025-10-26 20:54 ` [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch Song Liu
@ 2025-10-26 20:54 ` Song Liu
2025-10-26 20:54 ` [PATCH v3 bpf 3/3] selftests/bpf: Add tests for livepatch + bpf trampoline Song Liu
2025-10-27 8:47 ` [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Jiri Olsa
3 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2025-10-26 20:54 UTC (permalink / raw)
To: bpf, linux-trace-kernel, live-patching
Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
kernel-team, olsajiri, Song Liu
ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
the same kernel function. When needed, ftrace_hash_ipmodify_enable()
calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
share the same function as the IPMODIFY ftrace (livepatch).
ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
but not called in modify_ftrace_direct() path. As a result, the following
operations will break livepatch:
1. Load livepatch to a kernel function;
2. Attach fentry program to the kernel function;
3. Attach fexit program to the kernel function.
After 3, the kernel function being used will not be the livepatched
version, but the original version.
Fix this by adding __ftrace_hash_update_ipmodify() to
__modify_ftrace_direct() and adjust some logic around the call.
Signed-off-by: Song Liu <song@kernel.org>
---
kernel/trace/ftrace.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 725c224fb4e6..e977538fc0ca 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1971,7 +1971,8 @@ static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops)
*/
static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
struct ftrace_hash *old_hash,
- struct ftrace_hash *new_hash)
+ struct ftrace_hash *new_hash,
+ bool update_target)
{
struct ftrace_page *pg;
struct dyn_ftrace *rec, *end = NULL;
@@ -2006,10 +2007,13 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (rec->flags & FTRACE_FL_DISABLED)
continue;
- /* We need to update only differences of filter_hash */
+ /*
+ * Unless we are updating the target of a direct function,
+ * we only need to update differences of filter_hash
+ */
in_old = !!ftrace_lookup_ip(old_hash, rec->ip);
in_new = !!ftrace_lookup_ip(new_hash, rec->ip);
- if (in_old == in_new)
+ if (!update_target && (in_old == in_new))
continue;
if (in_new) {
@@ -2020,7 +2024,16 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (is_ipmodify)
goto rollback;
- FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
+ /*
+ * If this is called by __modify_ftrace_direct()
+ * then it is only chaning where the direct
+ * pointer is jumping to, and the record already
+ * points to a direct trampoline. If it isn't
+ * then it is a bug to update ipmodify on a direct
+ * caller.
+ */
+ FTRACE_WARN_ON(!update_target &&
+ (rec->flags & FTRACE_FL_DIRECT));
/*
* Another ops with IPMODIFY is already
@@ -2076,7 +2089,7 @@ static int ftrace_hash_ipmodify_enable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash);
+ return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash, false);
}
/* Disabling always succeeds */
@@ -2087,7 +2100,7 @@ static void ftrace_hash_ipmodify_disable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH);
+ __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH, false);
}
static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
@@ -2101,7 +2114,7 @@ static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
if (ftrace_hash_empty(new_hash))
new_hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash);
+ return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash, false);
}
static void print_ip_ins(const char *fmt, const unsigned char *p)
@@ -6112,7 +6125,7 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_direct);
static int
__modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
{
- struct ftrace_hash *hash;
+ struct ftrace_hash *hash = ops->func_hash->filter_hash;
struct ftrace_func_entry *entry, *iter;
static struct ftrace_ops tmp_ops = {
.func = ftrace_stub,
@@ -6132,13 +6145,21 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
if (err)
return err;
+ /*
+ * Call __ftrace_hash_update_ipmodify() here, so that we can call
+ * ops->ops_func for the ops. This is needed because the above
+ * register_ftrace_function_nolock() worked on tmp_ops.
+ */
+ err = __ftrace_hash_update_ipmodify(ops, hash, hash, true);
+ if (err)
+ goto out;
+
/*
* Now the ftrace_ops_list_func() is called to do the direct callers.
* We can safely change the direct functions attached to each entry.
*/
mutex_lock(&ftrace_lock);
- hash = ops->func_hash->filter_hash;
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
@@ -6153,6 +6174,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
mutex_unlock(&ftrace_lock);
+out:
/* Removing the tmp_ops will add the updated direct callers to the functions */
unregister_ftrace_function(&tmp_ops);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 bpf 3/3] selftests/bpf: Add tests for livepatch + bpf trampoline
2025-10-26 20:54 [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Song Liu
2025-10-26 20:54 ` [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch Song Liu
2025-10-26 20:54 ` [PATCH v3 bpf 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct() Song Liu
@ 2025-10-26 20:54 ` Song Liu
2025-10-27 8:47 ` [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Jiri Olsa
3 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2025-10-26 20:54 UTC (permalink / raw)
To: bpf, linux-trace-kernel, live-patching
Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
kernel-team, olsajiri, Song Liu
Both livepatch and BPF trampoline use ftrace. Special attention is needed
when livepatch and fexit program touch the same function at the same
time, because livepatch updates a kernel function and the BPF trampoline
need to call into the right version of the kernel function.
Use samples/livepatch/livepatch-sample.ko for the test.
The test covers two cases:
1) When a fentry program is loaded first. This exercises the
modify_ftrace_direct code path.
2) When a fentry program is loaded first. This exercises the
register_ftrace_direct code path.
Signed-off-by: Song Liu <song@kernel.org>
---
tools/testing/selftests/bpf/config | 3 +
.../bpf/prog_tests/livepatch_trampoline.c | 107 ++++++++++++++++++
.../bpf/progs/livepatch_trampoline.c | 30 +++++
3 files changed, 140 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
create mode 100644 tools/testing/selftests/bpf/progs/livepatch_trampoline.c
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 70b28c1e653e..f2a2fd236ca8 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -50,6 +50,7 @@ CONFIG_IPV6_SIT=y
CONFIG_IPV6_TUNNEL=y
CONFIG_KEYS=y
CONFIG_LIRC=y
+CONFIG_LIVEPATCH=y
CONFIG_LWTUNNEL=y
CONFIG_MODULE_SIG=y
CONFIG_MODULE_SRCVERSION_ALL=y
@@ -111,6 +112,8 @@ CONFIG_IP6_NF_FILTER=y
CONFIG_NF_NAT=y
CONFIG_PACKET=y
CONFIG_RC_CORE=y
+CONFIG_SAMPLES=y
+CONFIG_SAMPLE_LIVEPATCH=m
CONFIG_SECURITY=y
CONFIG_SECURITYFS=y
CONFIG_SYN_COOKIES=y
diff --git a/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
new file mode 100644
index 000000000000..72aa5376c30e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <test_progs.h>
+#include "testing_helpers.h"
+#include "livepatch_trampoline.skel.h"
+
+static int load_livepatch(void)
+{
+ char path[4096];
+
+ /* CI will set KBUILD_OUTPUT */
+ snprintf(path, sizeof(path), "%s/samples/livepatch/livepatch-sample.ko",
+ getenv("KBUILD_OUTPUT") ? : "../../../..");
+
+ return load_module(path, env_verbosity > VERBOSE_NONE);
+}
+
+static void unload_livepatch(void)
+{
+ /* Disable the livepatch before unloading the module */
+ system("echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled");
+
+ unload_module("livepatch_sample", env_verbosity > VERBOSE_NONE);
+}
+
+static void read_proc_cmdline(void)
+{
+ char buf[4096];
+ int fd, ret;
+
+ fd = open("/proc/cmdline", O_RDONLY);
+ if (!ASSERT_OK_FD(fd, "open /proc/cmdline"))
+ return;
+
+ ret = read(fd, buf, sizeof(buf));
+ if (!ASSERT_GT(ret, 0, "read /proc/cmdline"))
+ goto out;
+
+ ASSERT_OK(strncmp(buf, "this has been live patched", 26), "strncmp");
+
+out:
+ close(fd);
+}
+
+static void __test_livepatch_trampoline(bool fexit_first)
+{
+ struct livepatch_trampoline *skel = NULL;
+ int err;
+
+ skel = livepatch_trampoline__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+ goto out;
+
+ skel->bss->my_pid = getpid();
+
+ if (!fexit_first) {
+ /* fentry program is loaded first by default */
+ err = livepatch_trampoline__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto out;
+ } else {
+ /* Manually load fexit program first. */
+ skel->links.fexit_cmdline = bpf_program__attach(skel->progs.fexit_cmdline);
+ if (!ASSERT_OK_PTR(skel->links.fexit_cmdline, "attach_fexit"))
+ goto out;
+
+ skel->links.fentry_cmdline = bpf_program__attach(skel->progs.fentry_cmdline);
+ if (!ASSERT_OK_PTR(skel->links.fentry_cmdline, "attach_fentry"))
+ goto out;
+ }
+
+ read_proc_cmdline();
+
+ ASSERT_EQ(skel->bss->fentry_hit, 1, "fentry_hit");
+ ASSERT_EQ(skel->bss->fexit_hit, 1, "fexit_hit");
+out:
+ livepatch_trampoline__destroy(skel);
+}
+
+void test_livepatch_trampoline(void)
+{
+ int retry_cnt = 0;
+
+retry:
+ if (load_livepatch()) {
+ if (retry_cnt) {
+ ASSERT_OK(1, "load_livepatch");
+ goto out;
+ }
+ /*
+ * Something else (previous run of the same test?) loaded
+ * the KLP module. Unload the KLP module and retry.
+ */
+ unload_livepatch();
+ retry_cnt++;
+ goto retry;
+ }
+
+ if (test__start_subtest("fentry_first"))
+ __test_livepatch_trampoline(false);
+
+ if (test__start_subtest("fexit_first"))
+ __test_livepatch_trampoline(true);
+out:
+ unload_livepatch();
+}
diff --git a/tools/testing/selftests/bpf/progs/livepatch_trampoline.c b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
new file mode 100644
index 000000000000..15579d5bcd91
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+int fentry_hit;
+int fexit_hit;
+int my_pid;
+
+SEC("fentry/cmdline_proc_show")
+int BPF_PROG(fentry_cmdline)
+{
+ if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+ return 0;
+
+ fentry_hit = 1;
+ return 0;
+}
+
+SEC("fexit/cmdline_proc_show")
+int BPF_PROG(fexit_cmdline)
+{
+ if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+ return 0;
+
+ fexit_hit = 1;
+ return 0;
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs
2025-10-26 20:54 [PATCH v3 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs Song Liu
` (2 preceding siblings ...)
2025-10-26 20:54 ` [PATCH v3 bpf 3/3] selftests/bpf: Add tests for livepatch + bpf trampoline Song Liu
@ 2025-10-27 8:47 ` Jiri Olsa
3 siblings, 0 replies; 9+ messages in thread
From: Jiri Olsa @ 2025-10-27 8:47 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
rostedt, andrey.grodzovsky, mhiramat, kernel-team, olsajiri
On Sun, Oct 26, 2025 at 01:54:42PM -0700, Song Liu wrote:
> livepatch and BPF trampoline are two special users of ftrace. livepatch
> uses ftrace with IPMODIFY flag and BPF trampoline uses ftrace direct
> functions. When livepatch and BPF trampoline with fexit programs attach to
> the same kernel function, BPF trampoline needs to call into the patched
> version of the kernel function.
>
> 1/3 and 2/3 of this patchset fix two issues with livepatch + fexit cases,
> one in the register_ftrace_direct path, the other in the
> modify_ftrace_direct path.
>
> 3/3 adds selftests for both cases.
>
> ---
>
> Changes v2 => v3:
> 1. Incorporate feedback by AI, which also fixes build error reported by
> Steven and kernel test robot.
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
>
> v2: https://lore.kernel.org/bpf/20251024182901.3247573-1-song@kernel.org/
>
> Changes v1 => v2:
> 1. Target bpf tree. (Alexei)
> 2. Bring back the FTRACE_WARN_ON in __ftrace_hash_update_ipmodify
> for valid code paths. (Steven)
> 3. Update selftests with cleaner way to find livepatch-sample.ko.
> (offlline discussion with Ihor)
>
> v1: https://lore.kernel.org/bpf/20251024071257.3956031-1-song@kernel.org/
>
> Song Liu (3):
> ftrace: Fix BPF fexit with livepatch
> ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
> selftests/bpf: Add tests for livepatch + bpf trampoline
>
> kernel/bpf/trampoline.c | 5 -
> kernel/trace/ftrace.c | 46 ++++++--
> tools/testing/selftests/bpf/config | 3 +
> .../bpf/prog_tests/livepatch_trampoline.c | 107 ++++++++++++++++++
> .../bpf/progs/livepatch_trampoline.c | 30 +++++
> 5 files changed, 177 insertions(+), 14 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
> create mode 100644 tools/testing/selftests/bpf/progs/livepatch_trampoline.c
>
> --
> 2.47.3
^ permalink raw reply [flat|nested] 9+ messages in thread