On Wed, Jul 9, 2025 at 4:43 PM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Tue, Jul 08, 2025 at 03:21:40PM +0800, Menglong Dong wrote:
> > For now, we lookup the address of the attach target in
> > bpf_check_attach_target() with find_kallsyms_symbol_value or
> > kallsyms_lookup_name, which is not accurate in some cases.
> >
> > For example, we want to attach to the target "t_next", but there are
> > multiple symbols with the name "t_next" exist in the kallsyms. The one
> > that kallsyms_lookup_name() returned may have no ftrace record, which
> > makes the attach target not available. So we want the one that has ftrace
> > record to be returned.
> >
> > Meanwhile, there may be multiple symbols with the name "t_next" in ftrace
> > record. In this case, the attach target is ambiguous, so the attach should
> > fail.
>
> could you reproduce this somehow (bpftrace/selftest) for some symbol?
> I'd think pahole now filters all such symbols out of BTF and you need
> BTF func record to load the program in the first place
Hi, what's the version of pahole that does such filtering? I have
compiled the latest pahole, and such symbols exist. The version
of the pahole is v1.30
pahole --version
v1.30
It can be reproduced easily, just try to attach to the symbol t_next.
The "t_next" has multiple definition:
bpftrace -e 'fentry:t_next {printf("1");}'
Attaching 1 probe...
ERROR: Error attaching probe: fentry:vmlinux:t_next
This is the symbol information of t_next:
cat /proc/kallsyms | grep ' t_next'
ffffffff8142d9c0 t t_next
ffffffff81440e80 t t_next
ffffffff8144f1f0 t t_next
ffffffff8145ae90 t t_next
ffffffff81735b30 t t_next
cat /tracing/available_filter_functions | grep '^t_next'
t_next
The related patch is here:
https://lore.kernel.org/bpf/CADxym3Y-Jbzp0FupUgBDJB99GhsbDHyuV71Q6m9xyTpFze4ESg@xxxxxxxxxxxxxx/
(I just distclean and rebuild the kernel, the problem still exists)
Thanks!
Menglong Dong
>
> jirka
>
>
> >
> > Introduce the function bpf_lookup_attach_addr() to do the address lookup,
> > which is able to solve this problem.
> >
> > Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> > ---
> > v2:
> > - Lookup both vmlinux and modules symbols when mod is NULL, just like
> > kallsyms_lookup_name().
> >
> > If the btf is not a modules, shouldn't we lookup on the vmlinux only?
> > I'm not sure if we should keep the same logic with
> > kallsyms_lookup_name().
> >
> > - Return the kernel symbol that don't have ftrace location if the symbols
> > with ftrace location are not available
> > ---
> > kernel/bpf/verifier.c | 77 ++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 72 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 53007182b46b..4bacd0abf207 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -23476,6 +23476,73 @@ static int check_non_sleepable_error_inject(u32 btf_id)
> > return btf_id_set_contains(&btf_non_sleepable_error_inject, btf_id);
> > }
> >
> > +struct symbol_lookup_ctx {
> > + const char *name;
> > + unsigned long addr;
> > + bool ftrace_addr;
> > +};
> > +
> > +static int symbol_callback(void *data, unsigned long addr)
> > +{
> > + struct symbol_lookup_ctx *ctx = data;
> > +
> > + ctx->addr = addr;
> > + if (!ftrace_location(addr))
> > + return 0;
> > +
> > + if (ctx->ftrace_addr)
> > + return -EADDRNOTAVAIL;
> > + ctx->ftrace_addr = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int symbol_mod_callback(void *data, const char *name, unsigned long addr)
> > +{
> > + if (strcmp(((struct symbol_lookup_ctx *)data)->name, name) != 0)
> > + return 0;
> > +
> > + return symbol_callback(data, addr);
> > +}
> > +
> > +/**
> > + * bpf_lookup_attach_addr: Lookup address for a symbol
> > + *
> > + * @mod: kernel module to lookup the symbol, NULL means to lookup both vmlinux
> > + * and modules symbols
> > + * @sym: the symbol to resolve
> > + * @addr: pointer to store the result
> > + *
> > + * Lookup the address of the symbol @sym. If multiple symbols with the name
> > + * @sym exist, the one that has ftrace location is preferred. If more
> > + * than 1 has ftrace location, -EADDRNOTAVAIL will be returned.
> > + *
> > + * Returns: 0 on success, -errno otherwise.
> > + */
> > +static int bpf_lookup_attach_addr(const struct module *mod, const char *sym,
> > + unsigned long *addr)
> > +{
> > + struct symbol_lookup_ctx ctx = { .addr = 0, .name = sym };
> > + const char *mod_name = NULL;
> > + int err = 0;
> > +
> > +#ifdef CONFIG_MODULES
> > + mod_name = mod ? mod->name : NULL;
> > +#endif
> > + if (!mod_name)
> > + err = kallsyms_on_each_match_symbol(symbol_callback, sym, &ctx);
> > +
> > + if (!err && !ctx.addr)
> > + err = module_kallsyms_on_each_symbol(mod_name, symbol_mod_callback,
> > + &ctx);
> > +
> > + if (!ctx.addr)
> > + err = -ENOENT;
> > + *addr = err ? 0 : ctx.addr;
> > +
> > + return err;
> > +}
> > +
> > int bpf_check_attach_target(struct bpf_verifier_log *log,
> > const struct bpf_prog *prog,
> > const struct bpf_prog *tgt_prog,
> > @@ -23729,18 +23796,18 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > if (btf_is_module(btf)) {
> > mod = btf_try_get_module(btf);
> > if (mod)
> > - addr = find_kallsyms_symbol_value(mod, tname);
> > + ret = bpf_lookup_attach_addr(mod, tname, &addr);
> > else
> > - addr = 0;
> > + ret = -ENOENT;
> > } else {
> > - addr = kallsyms_lookup_name(tname);
> > + ret = bpf_lookup_attach_addr(NULL, tname, &addr);
> > }
> > - if (!addr) {
> > + if (ret) {
> > module_put(mod);
> > bpf_log(log,
> > "The address of function %s cannot be found\n",
> > tname);
> > - return -ENOENT;
> > + return ret;
> > }
> > }
> >
> > --
> > 2.39.5
> >
