lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADxym3adDgLaoQcQZLW=-fwELDi2-HTJ6tvA+HdF97+mKDErsQ@mail.gmail.com>
Date: Wed, 9 Jul 2025 18:33:08 +0800
From: Menglong Dong <menglong8.dong@...il.com>
To: Jiri Olsa <olsajiri@...il.com>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com, 
	andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Menglong Dong <dongml2@...natelecom.cn>
Subject: Re: [PATCH bpf-next v2] bpf: make the attach target more accurate

On Wed, Jul 9, 2025 at 4:43 PM Jiri Olsa <olsajiri@...il.com> 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@mail.gmail.com/

(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@...natelecom.cn>
> > ---
> > 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
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ