Re: [PATCHv2 bpf-next 4/8] bpf: Take module reference on kprobe_multi link

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Oct 21, 2022 at 03:02:30PM -0700, Andrii Nakryiko wrote:

SNIP

> > +       if (err) {
> > +               kprobe_multi_put_modules(args.mods, args.mods_cnt);
> > +               kfree(args.mods);
> > +               return err;
> > +       }
> > +
> > +       /* or number of modules found if everything is ok. */
> > +       *mods = args.mods;
> > +       return args.mods_cnt;
> > +}
> > +
> >  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >  {
> >         struct bpf_kprobe_multi_link *link = NULL;
> > @@ -2773,10 +2850,25 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >                        bpf_kprobe_multi_cookie_cmp,
> >                        bpf_kprobe_multi_cookie_swap,
> >                        link);
> > +       } else {
> > +               /*
> > +                * We need to sort addrs array even if there are no cookies
> > +                * provided, to allow bsearch in get_modules_for_addrs.
> > +                */
> > +               sort(addrs, cnt, sizeof(*addrs),
> > +                      bpf_kprobe_multi_addrs_cmp, NULL);
> > +       }
> > +
> > +       err = get_modules_for_addrs(&link->mods, addrs, cnt);
> > +       if (err < 0) {
> > +               bpf_link_cleanup(&link_primer);
> > +               return err;
> >         }
> > +       link->mods_cnt = err;
> >
> >         err = register_fprobe_ips(&link->fp, addrs, cnt);
> >         if (err) {
> > +               kprobe_multi_put_modules(link->mods, link->mods_cnt);
> 
> I don't think bpf_link_cleanup() will free link->mods, you have to do
> it explicitly here

hum, so bpf_link_cleanup sets link->prog to NULL so bpf_link_free
won't call link->ops->release, but will call link->ops->dealloc,
so it should be fine AFAICS

jirka

> 
> >                 bpf_link_cleanup(&link_primer);
> >                 return err;
> >         }
> > --
> > 2.37.3
> >