linux-kernel - Re: [PATCH] libbpf: resolve kernel function name optimization for kprobe

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: <CAEf4Bzb7iK8z-gGV_snM0LuSPEJXXDUZgvg1WeXu_TXu7ZHEMg@mail.gmail.com>
Date:   Wed, 11 Jan 2023 17:11:15 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Menglong Dong <menglong8.dong@...il.com>
Cc:     Yonghong Song <yhs@...a.com>, daniel@...earbox.net, ast@...nel.org,
        andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
        yhs@...com, john.fastabend@...il.com, kpsingh@...nel.org,
        sdf@...gle.com, haoluo@...gle.com, jolsa@...nel.org,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        Menglong Dong <imagedong@...cent.com>
Subject: Re: [PATCH] libbpf: resolve kernel function name optimization for kprobe

On Mon, Jan 9, 2023 at 7:11 PM Menglong Dong <menglong8.dong@...il.com> wrote:
>
> On Tue, Jan 10, 2023 at 4:29 AM Yonghong Song <yhs@...a.com> wrote:
> >
> >
> >
> > On 1/9/23 1:42 AM, menglong8.dong@...il.com wrote:
> > > From: Menglong Dong <imagedong@...cent.com>
> > >
> > > The function name in kernel may be changed by the compiler. For example,
> > > the function 'ip_rcv_core' can be compiled to 'ip_rcv_core.isra.0'.
> > >
> > > This kind optimization can happen in any kernel function. Therefor, we
> > > should conside this case.
> > >
> > > If we failed to attach kprobe with a '-ENOENT', then we can lookup the
> > > kallsyms and check if there is a similar function end with '.xxx', and
> > > retry.
> >
> > This might produce incorrect result, so this approach won't work
> > for all .isra.0 cases. When a function name is changed from
> > <func> to <func>.isra.<num>, it is possible that compiler may have
> > make some changes to the arguments, e.g., removing one argument,
> > chaning a semantics of argument, etc. if bpf program still
> > uses the original function signature, the bpf program may
> > produce unexpected result.
>
> Oops, I wasn't aware of this part. Can we make this function disabled
> by default and offer an option to users to enable it? Such as:
>
>     bpf_object_adapt_sym(struct bpf_object *obj)
>
> In my case, kernel function rename is common, and I have to
> check all functions and do such adaptation before attaching
> my kprobe programs, which makes me can't use auto-attach.
>
> What's more, I haven't seen the arguments change so far, and
> maybe it's not a common case?
>
> (Please just ignore this reply if it doesn't work :/ )
>

libbpf can't just assume that in SEC("kprobe/abc") abc is meant to be
a prefix. We'd need some more explicit marker to turn this search in
kallsyms by prefix on. But also, at that point it becomes a
multi-kprobe, really, because abc* pattern can match multiple
functions.

So it's not clear how to even integrate that into a singular kprobe
program cleanly.

Can you use kprobe.multi, which already supports what you want?

> Thanks!
> Menglong Dong
> >
> > >
> > > Signed-off-by: Menglong Dong <imagedong@...cent.com>
> > > ---
> > >   tools/lib/bpf/libbpf.c | 37 ++++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 36 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index a5c67a3c93c5..fdfb1ca34ced 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -10375,12 +10375,30 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > >       return libbpf_err_ptr(err);
> > >   }
> > >
> > > +struct kprobe_resolve {
> > > +     char pattern[128];
> > > +     char name[128];
> > > +};
> > > +
> > > +static int kprobe_kallsyms_cb(unsigned long long sym_addr, char sym_type,
> > > +                           const char *sym_name, void *ctx)
> > > +{
> > > +     struct kprobe_resolve *res = ctx;
> > > +
> > > +     if (!glob_match(sym_name, res->pattern))
> > > +             return 0;
> > > +     strcpy(res->name, sym_name);
> > > +     return 1;
> > > +}
> > > +
> > >   static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > >   {
> > >       DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
> > > +     struct kprobe_resolve res = {};
> > >       unsigned long offset = 0;
> > >       const char *func_name;
> > >       char *func;
> > > +     int err;
> > >       int n;
> > >
> > >       *link = NULL;
> > > @@ -10408,8 +10426,25 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
> > >
> > >       opts.offset = offset;
> > >       *link = bpf_program__attach_kprobe_opts(prog, func, &opts);
> > > +     err = libbpf_get_error(*link);
> > > +
> > > +     if (!err || err != -ENOENT)
> > > +             goto out;
> > > +
> > > +     sprintf(res.pattern, "%s.*", func);
> > > +     if (!libbpf_kallsyms_parse(kprobe_kallsyms_cb, &res))
> > > +             goto out;
> > > +
> > > +     pr_warn("prog '%s': trying to create %s '%s+0x%zx' perf event instead\n",
> > > +             prog->name, opts.retprobe ? "kretprobe" : "kprobe",
> > > +             res.name, offset);
> > > +
> > > +     *link = bpf_program__attach_kprobe_opts(prog, res.name, &opts);
> > > +     err = libbpf_get_error(*link);
> > > +
> > > +out:
> > >       free(func);
> > > -     return libbpf_get_error(*link);
> > > +     return err;
> > >   }
> > >
> > >   static int attach_ksyscall(const struct bpf_program *prog, long cookie, struct bpf_link **link)