Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230109094247.1464856-1-imagedong@tencent.com> <504cc35a-74a8-751a-5899-186d7a0aff87@meta.com>
In-Reply-To: <504cc35a-74a8-751a-5899-186d7a0aff87@meta.com>
From:   Menglong Dong <menglong8.dong@gmail.com>
Date:   Tue, 10 Jan 2023 11:11:29 +0800
Message-ID: <CADxym3bRciuyM1nYCrbaAwSMRJQvgV=hJFSLeiu9jysejPaTQQ@mail.gmail.com>
Subject: Re: [PATCH] libbpf: resolve kernel function name optimization for kprobe
To:     Yonghong Song <yhs@meta.com>
Cc:     daniel@iogearbox.net, ast@kernel.org, andrii@kernel.org,
        martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
        john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
        haoluo@google.com, jolsa@kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, Menglong Dong <imagedong@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jan 10, 2023 at 4:29 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> On 1/9/23 1:42 AM, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <imagedong@tencent.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 :/ )

Thanks!
Menglong Dong
>
> >
> > Signed-off-by: Menglong Dong <imagedong@tencent.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)