Re: [PATCH v4 04/10] libtracefs: Change tracefs_kprobe_info API

[Tzvetomir Stoyanov <tz.stoyanov@gmail.com>]

On Thu, Nov 4, 2021 at 6:33 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu,  4 Nov 2021 13:10:41 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > +enum tracefs_dynevent_type tracefs_kprobe_info(struct tracefs_dynevent *kprobe,
> > +                                            char **system, char **event,
> > +                                            char **prefix, char **addr, char **format)
> > +{
> > +     char **lv[] = { system, event, prefix, addr, format };
> > +     char **rv[] = { &kprobe->system, &kprobe->event, &kprobe->prefix,
> > +                     &kprobe->address, &kprobe->format };
> > +     int i;
> > +
> > +     if (!kprobe)
> > +             return TRACEFS_DYNEVENT_MAX;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(lv); i++)
> > +             *lv[i] = NULL;
>
> Do we really need to initialize them to NULL here?
>
> Not to mention, if one of the parameters is NULL itself, this will SEGFAULT.
>

I'll add a check for that. They should be set to NULL to ensure that
free() in the error path will not segfault, if the caller passes not
initialised pointers.

> > +
> > +     for (i = 0; i < ARRAY_SIZE(lv); i++) {
>
> Nice use of ARRAY_SIZE() ;-)
>
> > +             if (lv[i]) {
> > +                     if (*rv[i]) {
> > +                             *lv[i] = strdup(*rv[i]);
> > +                             if (!*lv[i])
> > +                                     goto error;
> > +                     } else {
> > +                             *lv[i] = NULL;
> >                       }
>
> The above here sets it to NULL if the parameter is non NULL.
>
> We don't need the initial NULL setting loop.
>
> -- Steve
>
>
> > -                     break;
> >               }
> > -             ret = parse_kprobe(NULL, &saveptr, &ktype, &system, &probe,
> > -                                &kaddr, &kfmt);
> >       }
> > -     free(content);
> > -     return rtype;
> > +
> > +     return kprobe->type;
> > +
> > +error:
> > +     for (i--; i >= 0; i--) {
> > +             if (lv[i])
> > +                     free(*lv[i]);
> > +     }
> > +
> > +     return TRACEFS_DYNEVENT_MAX;
> >  }
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center