All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Two trampoline cleanups
@ 2026-10-16 14:20 Leon Hwang
  2026-10-16 14:20 ` [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get() Leon Hwang
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Leon Hwang @ 2026-10-16 14:20 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, linux-kernel, Leon Hwang

Split these two cleanups for trampoline from the series
"[PATCH bpf-next v2 00/21] bpf: Add tracing_multi link support for bpf progs" [1].

Since the tr->ip is ftrace_location()-ed in bpf_trampoline_lookup(), no
need to call ftrace_location(tr->ip) again in
register/unregister/modify_fentry_multi(). Also no need to call
ftrace_location() for the target addr in register_fentry(), when
initialize ftrace_managed in bpf_trampoline_get().

Links:
[1] https://lore.kernel.org/bpf/20261012154024.29423-1-leon.hwang@linux.dev/

Leon Hwang (2):
  bpf: Initialize ftrace_managed in bpf_trampoline_get()
  bpf: Drop unnecessary ftrace_location() in
    register/unregister/modify_fentry_multi()

 kernel/bpf/trampoline.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get()
  2026-10-16 14:20 [PATCH bpf-next 0/2] bpf: Two trampoline cleanups Leon Hwang
@ 2026-10-16 14:20 ` Leon Hwang
  2026-10-16 14:32   ` sashiko-bot
  2026-10-16 14:20 ` [PATCH bpf-next 2/2] bpf: Drop unnecessary ftrace_location() in register/unregister/modify_fentry_multi() Leon Hwang
  2026-10-16 16:50 ` [PATCH bpf-next 0/2] bpf: Two trampoline cleanups patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Leon Hwang @ 2026-10-16 14:20 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, linux-kernel, Leon Hwang

A trampoline managed by ftrace is determined by its target, that said
ftrace_managed is already known when the trampoline is initialized. so,
set 'ftrace_managed = tr->ip != 0' in bpf_trampoline_get() instead of
relying on the first regular or tracing_multi attachment to set it.

This makes the backend selection independent of attachment order and keeps
the registration callbacks focused on installing the trampoline.

No functional change intended.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/trampoline.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 24edb59238f3..17f2a8133f61 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -475,18 +475,11 @@ static int register_fentry(struct bpf_trampoline *tr, struct bpf_tramp_image *im
 			   void *data __maybe_unused)
 {
 	void *new_addr = im->image;
-	void *ip = tr->func.addr;
-	unsigned long faddr;
 	int ret;
 
-	faddr = ftrace_location((unsigned long)ip);
-	if (faddr) {
+	if (tr->func.ftrace_managed) {
 		if (!tr->fops)
 			return -ENOTSUPP;
-		tr->func.ftrace_managed = true;
-	}
-
-	if (tr->func.ftrace_managed) {
 		ret = direct_ops_add(tr, new_addr);
 	} else {
 		ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
@@ -1286,6 +1279,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
 
 	memcpy(&tr->func.model, &tgt_info->fmodel, sizeof(tgt_info->fmodel));
 	tr->func.addr = (void *)tgt_info->tgt_addr;
+	tr->func.ftrace_managed = tr->ip != 0;
 out:
 	trampoline_unlock(tr);
 	return tr;
@@ -1649,7 +1643,6 @@ static int register_fentry_multi(struct bpf_trampoline *tr, struct bpf_tramp_ima
 	if (bpf_trampoline_use_jmp(tr->flags))
 		addr = ftrace_jmp_set(addr);
 
-	tr->func.ftrace_managed = true;
 	ftrace_hash_add(data->reg, data->entry, ip, addr);
 	tr->cur_image = im;
 	return 0;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] bpf: Drop unnecessary ftrace_location() in register/unregister/modify_fentry_multi()
  2026-10-16 14:20 [PATCH bpf-next 0/2] bpf: Two trampoline cleanups Leon Hwang
  2026-10-16 14:20 ` [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get() Leon Hwang
@ 2026-10-16 14:20 ` Leon Hwang
  2026-10-16 16:50 ` [PATCH bpf-next 0/2] bpf: Two trampoline cleanups patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2026-10-16 14:20 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, linux-kernel, Leon Hwang

Since 'tr->ip' is already a 'ftrace_location()'-ed IP in
bpf_trampoline_lookup(), it is unnecessary to call ftrace_location() again
in register/unregister/modify_fentry_multi().

No functional changes intended.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/trampoline.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 17f2a8133f61..21d460866dfe 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1629,8 +1629,8 @@ static void ftrace_hash_add(struct ftrace_hash *hash, struct ftrace_func_entry *
 static int register_fentry_multi(struct bpf_trampoline *tr, struct bpf_tramp_image *im, void *ptr)
 {
 	unsigned long addr = (unsigned long) im->image;
-	unsigned long ip = ftrace_location(tr->ip);
 	struct bpf_tracing_multi_data *data = ptr;
+	unsigned long ip = tr->ip;
 
 	/*
 	 * Different BTF ids get different trampolines, but they can still
@@ -1651,13 +1651,12 @@ static int register_fentry_multi(struct bpf_trampoline *tr, struct bpf_tramp_ima
 static int unregister_fentry_multi(struct bpf_trampoline *tr, u32 orig_flags, void *ptr)
 {
 	unsigned long addr = (unsigned long) tr->cur_image->image;
-	unsigned long ip = ftrace_location(tr->ip);
 	struct bpf_tracing_multi_data *data = ptr;
 
 	if (bpf_trampoline_use_jmp(tr->flags))
 		addr = ftrace_jmp_set(addr);
 
-	ftrace_hash_add(data->unreg, data->entry, ip, addr);
+	ftrace_hash_add(data->unreg, data->entry, tr->ip, addr);
 	tr->cur_image = NULL;
 	return 0;
 }
@@ -1666,13 +1665,12 @@ static int modify_fentry_multi(struct bpf_trampoline *tr, u32 orig_flags, struct
 			       bool lock_direct_mutex, void *ptr)
 {
 	unsigned long addr = (unsigned long) im->image;
-	unsigned long ip = ftrace_location(tr->ip);
 	struct bpf_tracing_multi_data *data = ptr;
 
 	if (bpf_trampoline_use_jmp(tr->flags))
 		addr = ftrace_jmp_set(addr);
 
-	ftrace_hash_add(data->modify, data->entry, ip, addr);
+	ftrace_hash_add(data->modify, data->entry, tr->ip, addr);
 	tr->cur_image = im;
 	return 0;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get()
  2026-10-16 14:20 ` [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get() Leon Hwang
@ 2026-10-16 14:32   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-16 14:32 UTC (permalink / raw)
  To: Leon Hwang, bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: dummy trampoline causes ftrace_managed text corruption
--

commit df0c3beb4928ce14ffbe4b44e2bc365c2a470352
Author: Leon Hwang <leon.hwang@linux.dev>

bpf: Initialize ftrace_managed in bpf_trampoline_get()

This patch removes the dynamic ftrace_location() check from
register_fentry() and instead initializes ftrace_managed in
bpf_trampoline_get() based on whether tr->ip is non-zero.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 24edb59238f34..17f2a8133f611 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -475,18 +475,11 @@ static int register_fentry(struct bpf_trampoline *tr, struct bpf_tramp_image *im
>  			   void *data __maybe_unused)
>  {
>  	void *new_addr = im->image;
> -	void *ip = tr->func.addr;
> -	unsigned long faddr;
>  	int ret;
>  
> -	faddr = ftrace_location((unsigned long)ip);
> -	if (faddr) {
> +	if (tr->func.ftrace_managed) {
>  		if (!tr->fops)
>  			return -ENOTSUPP;
> -		tr->func.ftrace_managed = true;
> -	}
> -
> -	if (tr->func.ftrace_managed) {
>  		ret = direct_ops_add(tr, new_addr);
>  	} else {
>  		ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);

[Severity: High]
Because ftrace_managed is no longer evaluated dynamically here, is there a
risk of corrupting ftrace-managed text if tr->func.ftrace_managed was
incorrectly cached as false? 

If it evaluates to false, register_fentry() falls back to 
bpf_trampoline_update_fentry() which does raw text poking.

[ ... ]

> @@ -1286,6 +1279,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
>  
>  	memcpy(&tr->func.model, &tgt_info->fmodel, sizeof(tgt_info->fmodel));
>  	tr->func.addr = (void *)tgt_info->tgt_addr;
> +	tr->func.ftrace_managed = tr->ip != 0;
>  out:
>  	trampoline_unlock(tr);
>  	return tr;

[Severity: High]
Can this assignment incorrectly mark a ftrace-managed target as non-managed
if a concurrent operation creates a dummy trampoline?

If bpf_trampoline_unlink_cgroup_shim() is called for a program that was
never successfully linked to a trampoline, it calls
bpf_trampoline_lookup(key, 0). Since the trampoline doesn't exist, a dummy
trampoline with tr->ip = 0 is temporarily created.

If a concurrent bpf_trampoline_get() targets the same hook during this
window, it finds the dummy trampoline. With this change, it will cache
tr->func.ftrace_managed as false because tr->ip is 0, regardless of the
actual target address.

When register_fentry() then runs, it bypasses direct ops and uses raw
bpf_arch_text_poke() on the address. Could this cause kernel text
corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016142014.12420-1-leon.hwang@linux.dev?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 0/2] bpf: Two trampoline cleanups
  2026-10-16 14:20 [PATCH bpf-next 0/2] bpf: Two trampoline cleanups Leon Hwang
  2026-10-16 14:20 ` [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get() Leon Hwang
  2026-10-16 14:20 ` [PATCH bpf-next 2/2] bpf: Drop unnecessary ftrace_location() in register/unregister/modify_fentry_multi() Leon Hwang
@ 2026-10-16 16:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-16 16:50 UTC (permalink / raw)
  To: Leon Hwang
  Cc: bpf, ast, daniel, john.fastabend, andrii, eddyz87, memxor,
	martin.lau, song, yonghong.song, jolsa, emil, ihor.solodrai,
	linux-kernel

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Fri, 16 Oct 2026 22:20:12 +0800 you wrote:
> Split these two cleanups for trampoline from the series
> "[PATCH bpf-next v2 00/21] bpf: Add tracing_multi link support for bpf progs" [1].
> 
> Since the tr->ip is ftrace_location()-ed in bpf_trampoline_lookup(), no
> need to call ftrace_location(tr->ip) again in
> register/unregister/modify_fentry_multi(). Also no need to call
> ftrace_location() for the target addr in register_fentry(), when
> initialize ftrace_managed in bpf_trampoline_get().
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get()
    https://git.kernel.org/bpf/bpf-next/c/09ec92d70723
  - [bpf-next,2/2] bpf: Drop unnecessary ftrace_location() in register/unregister/modify_fentry_multi()
    https://git.kernel.org/bpf/bpf-next/c/b443a4578a0d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-16 16:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 14:20 [PATCH bpf-next 0/2] bpf: Two trampoline cleanups Leon Hwang
2026-10-16 14:20 ` [PATCH bpf-next 1/2] bpf: Initialize ftrace_managed in bpf_trampoline_get() Leon Hwang
2026-10-16 14:32   ` sashiko-bot
2026-10-16 14:20 ` [PATCH bpf-next 2/2] bpf: Drop unnecessary ftrace_location() in register/unregister/modify_fentry_multi() Leon Hwang
2026-10-16 16:50 ` [PATCH bpf-next 0/2] bpf: Two trampoline cleanups patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.