Re: [PATCHv3 bpf-next 08/24] bpf: Add bpf_trampoline_multi_attach/detach functions
Hi Jiri, kernel test robot noticed the following build errors: [auto build test ERROR on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Olsa/ftrace-Add-ftrace_hash_count-function/20260316-160117 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20260316075138.465430-9-jolsa%40kernel.org patch subject: [PATCHv3 bpf-next 08/24] bpf: Add bpf_trampoline_multi_attach/detach functions config: x86_64-randconfig-075-20260320 (https://download.01.org/0day-ci/archive/20260320/[email protected]/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260320/[email protected]/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ All errors (new ones prefixed by >>): >> kernel/bpf/trampoline.c:1520:8: error: call to undeclared function >> 'btf_distill_func_proto'; ISO C99 and later do not support implicit function >> declarations [-Wimplicit-function-declaration] 1520 | err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel); | ^ 1 error generated. vim +/btf_distill_func_proto +1520 kernel/bpf/trampoline.c 1498 1499 static int bpf_get_btf_id_target(struct btf *btf, struct bpf_prog *prog, u32 btf_id, 1500 struct bpf_attach_target_info *tgt_info) 1501 { 1502 const struct btf_type *t; 1503 unsigned long addr; 1504 const char *tname; 1505 int err; 1506 1507 if (!btf_id || !btf) 1508 return -EINVAL; 1509 t = btf_type_by_id(btf, btf_id); 1510 if (!t) 1511 return -EINVAL; 1512 tname = btf_name_by_offset(btf, t->name_off); 1513 if (!tname) 1514 return -EINVAL; 1515 if (!btf_type_is_func(t)) 1516 return -EINVAL; 1517 t = btf_type_by_id(btf, t->type); 1518 if (!btf_type_is_func_proto(t)) 1519 return -EINVAL; > 1520 err = btf_distill_func_proto(NULL, btf, t, tname, > &tgt_info->fmodel); 1521 if (err < 0) 1522 return err; 1523 if (btf_is_module(btf)) { 1524 /* The bpf program already holds refference to module. */ 1525 if (WARN_ON_ONCE(!prog->aux->mod)) 1526 return -EINVAL; 1527 addr = find_kallsyms_symbol_value(prog->aux->mod, tname); 1528 } else { 1529 addr = kallsyms_lookup_name(tname); 1530 } 1531 if (!addr || !ftrace_location(addr)) 1532 return -ENOENT; 1533 tgt_info->tgt_addr = addr; 1534 return 0; 1535 } 1536 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCHv3 bpf-next 08/24] bpf: Add bpf_trampoline_multi_attach/detach functions
On Mon, Mar 16, 2026 at 08:35:15AM +, [email protected] wrote: > > The detach works as follows: > > - we lock all the needed trampolines > > - remove the program from each (__bpf_trampoline_unlink_prog) > > - the trampoline_multi_ops passed in __bpf_trampoline_link_prog gathers > > ftrace_hash (ip -> trampoline) objects > > The detach description references __bpf_trampoline_link_prog, but the > detach path calls __bpf_trampoline_unlink_prog. ack > > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > > index d55651b13511..9331cca8c0b4 100644 > > --- a/kernel/bpf/trampoline.c > > +++ b/kernel/bpf/trampoline.c > > [ ... ] > > > +static int modify_fentry_multi(struct bpf_trampoline *tr, u32 orig_flags, > > void *old_addr, > > + void *new_addr, bool lock_direct_mutex, void > > *ptr) > > +{ > > + unsigned long addr = (unsigned long) new_addr; > > + unsigned long ip = ftrace_location(tr->ip); > > + struct fentry_multi_data *data = ptr; > > + > > + if (bpf_trampoline_use_jmp(tr->flags)) > > + addr = ftrace_jmp_set(addr); > > + return add_ftrace_hash_entry_direct(data->modify, ip, addr) ? 0 : > > -ENOMEM; > > +} > > Unlike the standard modify_fentry() which calls direct_ops_mod() -> > update_ftrace_direct_mod() to update ftrace immediately, > modify_fentry_multi() only adds a hash entry. The actual ftrace > update is deferred until after the link loop. > > This matters because bpf_trampoline_update() unconditionally calls > bpf_tramp_image_put() on the old cur_image after a successful > modify_fentry callback: > > bpf_trampoline_update() { > ... > err = ops->modify_fentry(tr, ...); > ... > if (tr->cur_image) > bpf_tramp_image_put(tr->cur_image); > tr->cur_image = im; > } > > With the standard ops, the old image is already unreachable when put. > With multi ops, ftrace still points to the old image when it is put. > bpf_tramp_image_put() text-pokes the old image to skip fexit programs > and starts deferred freeing. > > In the success path, the window between image_put and the batched > update_ftrace_direct_mod is brief (bounded by trampoline_lock_all). > > However, in the rollback path below, ftrace is never updated at all: > > [ ... ] > > > + trampoline_lock_all(); > > + > > + for (i = 0; i < cnt; i++) { > > + mnode = &link->nodes[i]; > > + err = __bpf_trampoline_link_prog(&mnode->node, > > mnode->trampoline, NULL, > > +&trampoline_multi_ops, &data); > > + if (err) > > + goto rollback_unlink; > > + } > > [ ... ] > > > +rollback_unlink: > > + for (j = 0; j < i; j++) { > > + mnode = &link->nodes[j]; > > + WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, > > mnode->trampoline, > > + NULL, &trampoline_multi_ops, &data)); > > + } > > + trampoline_unlock_all(); > > + > > + i = cnt; > > If a trampoline at index 0 already had programs attached through the > standard single-trampoline path, and __bpf_trampoline_link_prog > succeeds for it but fails for a later trampoline, the rollback path > restores tr->cur_image to a correct new image but never calls > update_ftrace_direct_mod. > > At that point, ftrace still routes calls through the old image that > was put during the link phase. bpf_tramp_image_put() has already > patched that old image to skip fexit programs and started the deferred > freeing sequence (percpu_ref_kill). > > Could this leave existing fexit/fmod_ret programs on that trampoline > non-functional? And once the percpu_ref drains during a quiet period, > would the old image be freed while ftrace still references it? good catch, we need the update_ftrace_direct_mod call after the unlink, I changed the rollback test to hit the described issue, will fix > > [ ... ] > > > +int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct > > bpf_tracing_multi_link *link) > > +{ > > + struct bpf_tracing_multi_node *mnode; > > + struct fentry_multi_data data = {}; > > + int i, cnt = link->nodes_cnt; > > + > > + data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS); > > + data.modify = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS); > > + > > + if (!data.unreg || !data.modify) { > > + free_fentry_multi_data(&data); > > + return -ENOMEM; > > + } > > bpf_trampoline_multi_detach can return -ENOMEM here without detaching > anything. In e80a839e3d2c ("bpf: Add support for tracing multi > link"), this is called from bpf_tracing_multi_link_release(): > > WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link)); > > If detach fails, the programs remain linked to the trampolines via > mnode->node.tramp_hlist, but the link (and its embedded nodes array) > is freed by bpf_tracing_multi_link_dealloc(). Would this leave > dangling hlist entries in each trampol
Re: [PATCHv3 bpf-next 08/24] bpf: Add bpf_trampoline_multi_attach/detach functions
> The detach works as follows:
> - we lock all the needed trampolines
> - remove the program from each (__bpf_trampoline_unlink_prog)
> - the trampoline_multi_ops passed in __bpf_trampoline_link_prog gathers
> ftrace_hash (ip -> trampoline) objects
The detach description references __bpf_trampoline_link_prog, but the
detach path calls __bpf_trampoline_unlink_prog.
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index d55651b13511..9331cca8c0b4 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
[ ... ]
> +static int modify_fentry_multi(struct bpf_trampoline *tr, u32 orig_flags,
> void *old_addr,
> +void *new_addr, bool lock_direct_mutex, void
> *ptr)
> +{
> + unsigned long addr = (unsigned long) new_addr;
> + unsigned long ip = ftrace_location(tr->ip);
> + struct fentry_multi_data *data = ptr;
> +
> + if (bpf_trampoline_use_jmp(tr->flags))
> + addr = ftrace_jmp_set(addr);
> + return add_ftrace_hash_entry_direct(data->modify, ip, addr) ? 0 :
> -ENOMEM;
> +}
Unlike the standard modify_fentry() which calls direct_ops_mod() ->
update_ftrace_direct_mod() to update ftrace immediately,
modify_fentry_multi() only adds a hash entry. The actual ftrace
update is deferred until after the link loop.
This matters because bpf_trampoline_update() unconditionally calls
bpf_tramp_image_put() on the old cur_image after a successful
modify_fentry callback:
bpf_trampoline_update() {
...
err = ops->modify_fentry(tr, ...);
...
if (tr->cur_image)
bpf_tramp_image_put(tr->cur_image);
tr->cur_image = im;
}
With the standard ops, the old image is already unreachable when put.
With multi ops, ftrace still points to the old image when it is put.
bpf_tramp_image_put() text-pokes the old image to skip fexit programs
and starts deferred freeing.
In the success path, the window between image_put and the batched
update_ftrace_direct_mod is brief (bounded by trampoline_lock_all).
However, in the rollback path below, ftrace is never updated at all:
[ ... ]
> + trampoline_lock_all();
> +
> + for (i = 0; i < cnt; i++) {
> + mnode = &link->nodes[i];
> + err = __bpf_trampoline_link_prog(&mnode->node,
> mnode->trampoline, NULL,
> + &trampoline_multi_ops, &data);
> + if (err)
> + goto rollback_unlink;
> + }
[ ... ]
> +rollback_unlink:
> + for (j = 0; j < i; j++) {
> + mnode = &link->nodes[j];
> + WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node,
> mnode->trampoline,
> + NULL, &trampoline_multi_ops, &data));
> + }
> + trampoline_unlock_all();
> +
> + i = cnt;
If a trampoline at index 0 already had programs attached through the
standard single-trampoline path, and __bpf_trampoline_link_prog
succeeds for it but fails for a later trampoline, the rollback path
restores tr->cur_image to a correct new image but never calls
update_ftrace_direct_mod.
At that point, ftrace still routes calls through the old image that
was put during the link phase. bpf_tramp_image_put() has already
patched that old image to skip fexit programs and started the deferred
freeing sequence (percpu_ref_kill).
Could this leave existing fexit/fmod_ret programs on that trampoline
non-functional? And once the percpu_ref drains during a quiet period,
would the old image be freed while ftrace still references it?
[ ... ]
> +int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct
> bpf_tracing_multi_link *link)
> +{
> + struct bpf_tracing_multi_node *mnode;
> + struct fentry_multi_data data = {};
> + int i, cnt = link->nodes_cnt;
> +
> + data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> + data.modify = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> +
> + if (!data.unreg || !data.modify) {
> + free_fentry_multi_data(&data);
> + return -ENOMEM;
> + }
bpf_trampoline_multi_detach can return -ENOMEM here without detaching
anything. In e80a839e3d2c ("bpf: Add support for tracing multi
link"), this is called from bpf_tracing_multi_link_release():
WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link));
If detach fails, the programs remain linked to the trampolines via
mnode->node.tramp_hlist, but the link (and its embedded nodes array)
is freed by bpf_tracing_multi_link_dealloc(). Would this leave
dangling hlist entries in each trampoline's progs_hlist?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23133791558
[PATCHv3 bpf-next 08/24] bpf: Add bpf_trampoline_multi_attach/detach functions
Adding bpf_trampoline_multi_attach/detach functions that allows to
attach/detach tracing program to multiple functions/trampolines.
The attachment is defined with bpf_program and array of BTF ids of
functions to attach the bpf program to.
Adding bpf_tracing_multi_link object that holds all the attached
trampolines and is initialized in attach and used in detach.
The attachment allocates or uses currently existing trampoline
for each function to attach and links it with the bpf program.
The attach works as follows:
- we get all the needed trampolines
- lock them and add the bpf program to each (__bpf_trampoline_link_prog)
- the trampoline_multi_ops passed in __bpf_trampoline_link_prog gathers
ftrace_hash (ip -> trampoline) objects
- we call update_ftrace_direct_add/mod to update needed locations
- we unlock all the trampolines
The detach works as follows:
- we lock all the needed trampolines
- remove the program from each (__bpf_trampoline_unlink_prog)
- the trampoline_multi_ops passed in __bpf_trampoline_link_prog gathers
ftrace_hash (ip -> trampoline) objects
- we call update_ftrace_direct_del/mod to update needed locations
- we unlock and put all the trampolines
Adding trampoline_(un)lock_all functions to (un)lock all trampolines
to gate the tracing_multi attachment.
Note this is supported only for archs (x86_64) with ftrace direct and
have single ops support.
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS &&
CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
Signed-off-by: Jiri Olsa
---
include/linux/bpf.h | 17 +++
kernel/bpf/trampoline.c | 243
2 files changed, 260 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c401b308a325..f22b9400a915 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1464,6 +1464,12 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
void bpf_trampoline_put(struct bpf_trampoline *tr);
int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int
num_funcs);
+struct bpf_tracing_multi_link;
+int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
+ struct bpf_tracing_multi_link *link);
+int bpf_trampoline_multi_detach(struct bpf_prog *prog,
+ struct bpf_tracing_multi_link *link);
+
/*
* When the architecture supports STATIC_CALL replace the bpf_dispatcher_fn
* indirection with a direct call to the bpf program. If the architecture does
@@ -1888,6 +1894,17 @@ struct bpf_tracing_link {
struct bpf_prog *tgt_prog;
};
+struct bpf_tracing_multi_node {
+ struct bpf_tramp_node node;
+ struct bpf_trampoline *trampoline;
+};
+
+struct bpf_tracing_multi_link {
+ struct bpf_link link;
+ int nodes_cnt;
+ struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
+};
+
struct bpf_raw_tp_link {
struct bpf_link link;
struct bpf_raw_event_map *btp;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index d55651b13511..9331cca8c0b4 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -88,6 +88,22 @@ static struct bpf_trampoline *direct_ops_ip_lookup(struct
ftrace_ops *ops, unsig
mutex_unlock(&trampoline_mutex);
return tr;
}
+
+static void trampoline_lock_all(void)
+{
+ int i;
+
+ for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
+ mutex_lock(&trampoline_locks[i].mutex);
+}
+
+static void trampoline_unlock_all(void)
+{
+ int i;
+
+ for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
+ mutex_unlock(&trampoline_locks[i].mutex);
+}
#else
static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops,
unsigned long ip)
{
@@ -1423,6 +1439,233 @@ int __weak arch_bpf_trampoline_size(const struct
btf_func_model *m, u32 flags,
return -ENOTSUPP;
}
+#if defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && \
+defined(CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS)
+
+struct fentry_multi_data {
+ struct ftrace_hash *unreg;
+ struct ftrace_hash *modify;
+ struct ftrace_hash *reg;
+};
+
+static void free_fentry_multi_data(struct fentry_multi_data *data)
+{
+ free_ftrace_hash(data->reg);
+ free_ftrace_hash(data->unreg);
+ free_ftrace_hash(data->modify);
+}
+
+static int register_fentry_multi(struct bpf_trampoline *tr, void *new_addr,
void *ptr)
+{
+ unsigned long addr = (unsigned long) new_addr;
+ unsigned long ip = ftrace_location(tr->ip);
+ struct fentry_multi_data *data = ptr;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+ return add_ftrace_hash_entry_direct(data->reg, ip, addr) ? 0 : -ENOMEM;
+}
+
+static int unregister_fentry_multi(struct bpf_trampoline *tr, u32 orig_flags,
void *old_addr,
+ void *ptr)
+{
+ unsigned long addr = (unsigned long) old_addr;
+ unsigned long ip = ftrace_location(tr->ip);
+ struc
