On Tue, Jul 12, 2022 at 12:58 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Tue, Jul 12, 2022 at 10:57:45AM +0800, Hengqi Chen wrote:
> > binary_path is a required non-null parameter for bpf_program__attach_usdt
> > and bpf_program__attach_uprobe_opts. Check it against NULL to prevent
> > coredump on strchr.
>
> binary_path seems to be mandatory so LGTM, cc-ing Alan to be sure ;-)
>
Right, what will happen for attach_uprobe with NULL binary_path is
that it will be passed as zero to perf_event_create() and kernel will
reject it with -EINVAL. So this looks all correct to me, applying to
bpf-next.
Thanks for the fix!
> thanks,
> jirka
>
> >
> > Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> > ---
> > tools/lib/bpf/libbpf.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index cb49408eb298..72548798126b 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -10545,7 +10545,10 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> > ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0);
> > pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
> >
> > - if (binary_path && !strchr(binary_path, '/')) {
> > + if (!binary_path)
> > + return libbpf_err_ptr(-EINVAL);
> > +
> > + if (!strchr(binary_path, '/')) {
> > err = resolve_full_path(binary_path, full_binary_path,
> > sizeof(full_binary_path));
> > if (err) {
> > @@ -10559,11 +10562,6 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> > if (func_name) {
> > long sym_off;
> >
> > - if (!binary_path) {
> > - pr_warn("prog '%s': name-based attach requires binary_path\n",
> > - prog->name);
> > - return libbpf_err_ptr(-EINVAL);
> > - }
> > sym_off = elf_find_func_offset(binary_path, func_name);
> > if (sym_off < 0)
> > return libbpf_err_ptr(sym_off);
> > @@ -10711,6 +10709,9 @@ struct bpf_link *bpf_program__attach_usdt(const struct bpf_program *prog,
> > return libbpf_err_ptr(-EINVAL);
> > }
> >
> > + if (!binary_path)
> > + return libbpf_err_ptr(-EINVAL);
> > +
> > if (!strchr(binary_path, '/')) {
> > err = resolve_full_path(binary_path, resolved_path, sizeof(resolved_path));
> > if (err) {
> > --
> > 2.30.2
