Re: [PATCHv4 bpf-next 09/25] bpf: Add bpf_trampoline_multi_attach/detach functions
Hi Jiri, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Olsa/ftrace-Add-ftrace_hash_count-function/20260326-101836 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20260324081846.2334094-10-jolsa%40kernel.org patch subject: [PATCHv4 bpf-next 09/25] bpf: Add bpf_trampoline_multi_attach/detach functions config: x86_64-randconfig-015-20260327 (https://download.01.org/0day-ci/archive/20260327/[email protected]/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260327/[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 warnings (new ones prefixed by >>): >> kernel/bpf/trampoline.c:100:13: warning: 'trampoline_unlock_all' defined but >> not used [-Wunused-function] 100 | static void trampoline_unlock_all(void) | ^ >> kernel/bpf/trampoline.c:92:13: warning: 'trampoline_lock_all' defined but >> not used [-Wunused-function] 92 | static void trampoline_lock_all(void) | ^~~ vim +/trampoline_unlock_all +100 kernel/bpf/trampoline.c 91 > 92 static void trampoline_lock_all(void) 93 { 94 int i; 95 96 for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++) 97 mutex_lock(&trampoline_locks[i].mutex); 98 } 99 > 100 static void trampoline_unlock_all(void) 101 { 102 int i; 103 104 for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++) 105 mutex_unlock(&trampoline_locks[i].mutex); 106 } 107 #else 108 static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip) 109 { 110 return ops->private; 111 } 112 #endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */ 113 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCHv4 bpf-next 09/25] bpf: Add bpf_trampoline_multi_attach/detach functions
On Tue, Mar 24, 2026 at 08:58:41AM +, [email protected] wrote: > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > > index a9e328c0a1b3..2986e5cac743 100644 > > --- a/kernel/bpf/trampoline.c > > +++ b/kernel/bpf/trampoline.c > > [ ... ] > > > +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; > > + } > > If alloc_ftrace_hash() fails here under memory pressure, the function > returns -ENOMEM without detaching any trampolines or releasing their > refcounts. > > The caller in the next patch (bpf_tracing_multi_link_release) does: > > WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link)); > > This drops the error on the floor. The link struct is then freed by > bpf_tracing_multi_link_dealloc(), but the trampolines remain attached > with elevated refcounts, permanently leaking them and their ftrace > hooks. > > Would it make sense to pre-allocate these hashes during > bpf_trampoline_multi_attach() and store them in the link struct so > that the detach path cannot fail? it's not just hash objects but we will need allocate also entries, I can do follow up with this if others think it's needed > > > + > > + trampoline_lock_all(); > > [ ... ] > > > +rollback_unlink: > > + /* > > +* We can end up in here from 3 points from above code: > > +* > > +* - __bpf_trampoline_link_prog or update_ftrace_direct_add failed and > > +* we have some portion of linked trampolines without ftrace update > > +* > > +* - update_ftrace_direct_mod failed and we have all trampolines linked > > +* plus we already un-attached all new trampolines > > +* > > +* In both cases we need to unlink all trampolines from the new program > > +* and update modified (data.modify) sites, because those have > > previously > > +* some programs attached and the new trampoline needs to get attached. > > +*/ > > This isn't a bug, but the comment says "3 points" and then describes > two scenarios in two bullet points. Maybe say "2 scenarios" or split > the first bullet into two to match? first two points are covered in the first bullets, I think it's ok jirka > > > + ftrace_hash_clear(data.modify); > > [ ... ] > > > --- > 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/23480161822
Re: [PATCHv4 bpf-next 09/25] bpf: Add bpf_trampoline_multi_attach/detach functions
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index a9e328c0a1b3..2986e5cac743 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
[ ... ]
> +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;
> + }
If alloc_ftrace_hash() fails here under memory pressure, the function
returns -ENOMEM without detaching any trampolines or releasing their
refcounts.
The caller in the next patch (bpf_tracing_multi_link_release) does:
WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link));
This drops the error on the floor. The link struct is then freed by
bpf_tracing_multi_link_dealloc(), but the trampolines remain attached
with elevated refcounts, permanently leaking them and their ftrace
hooks.
Would it make sense to pre-allocate these hashes during
bpf_trampoline_multi_attach() and store them in the link struct so
that the detach path cannot fail?
> +
> + trampoline_lock_all();
[ ... ]
> +rollback_unlink:
> + /*
> + * We can end up in here from 3 points from above code:
> + *
> + * - __bpf_trampoline_link_prog or update_ftrace_direct_add failed and
> + * we have some portion of linked trampolines without ftrace update
> + *
> + * - update_ftrace_direct_mod failed and we have all trampolines linked
> + * plus we already un-attached all new trampolines
> + *
> + * In both cases we need to unlink all trampolines from the new program
> + * and update modified (data.modify) sites, because those have
> previously
> + * some programs attached and the new trampoline needs to get attached.
> + */
This isn't a bug, but the comment says "3 points" and then describes
two scenarios in two bullet points. Maybe say "2 scenarios" or split
the first bullet into two to match?
> + ftrace_hash_clear(data.modify);
[ ... ]
---
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/23480161822
[PATCHv4 bpf-next 09/25] 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_unlink_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
It also needs CONFIG_BPF_SYSCALL enabled.
Signed-off-by: Jiri Olsa
---
include/linux/bpf.h | 28 +
kernel/bpf/trampoline.c | 265
2 files changed, 293 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4628f2bf3a5b..113c9eb7a207 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1452,6 +1452,8 @@ static inline int bpf_dynptr_check_off_len(const struct
bpf_dynptr_kern *ptr, u6
return 0;
}
+struct bpf_tracing_multi_link;
+
#ifdef CONFIG_BPF_JIT
int bpf_trampoline_link_prog(struct bpf_tramp_node *node,
struct bpf_trampoline *tr,
@@ -1464,6 +1466,11 @@ 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);
+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
@@ -1573,6 +1580,16 @@ static inline bool bpf_prog_has_trampoline(const struct
bpf_prog *prog)
{
return false;
}
+static inline int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
+ struct bpf_tracing_multi_link
*link)
+{
+ return -ENOTSUPP;
+}
+static inline int bpf_trampoline_multi_detach(struct bpf_prog *prog,
+ struct bpf_tracing_multi_link
*link)
+{
+ return -ENOTSUPP;
+}
#endif
struct bpf_func_info_aux {
@@ -1888,6 +1905,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 a9e328c0a1b3..2986e5cac743 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,255 @@ 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) && \
+defined(CONFIG_BPF_SYSCALL)
+
+struct fentry_multi_data {
+ struct ftrace_hash *unreg;
+ struct ftrace_hash *modify;
+ struct ftrace_ha
