Re: [PATCH] perf jevents: Fix suspicious code in fixregex()

From: Arnaldo Carvalho de Melo
Date: Thu Sep 03 2020 - 14:41:34 EST


Em Thu, Sep 03, 2020 at 10:47:39AM -0700, Ian Rogers escreveu:
> On Thu, Sep 3, 2020 at 8:25 AM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> > The new string should have enough space for the original string and
> > the back slashes IMHO.

> > Cc: John Garry <john.garry@xxxxxxxxxx>
> > Cc: Kajol Jain <kjain@xxxxxxxxxxxxx>
> > Cc: Ian Rogers <irogers@xxxxxxxxxx>
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Definitely looks like the right fix. I'm surprised this hasn't shown
> up in sanitizer testing.

Yeap, good catch! Namyung you forgot to add the Fixes tag + Cc the patch
author that introduced that bug, I did it:

Cc: William Cohen <wcohen@xxxxxxxxxx>
Fixes: fbc2844e84038ce3 ("perf vendor events: Use more flexible pattern matching for CPU identification for mapfile.csv"

Please consider doing it next time :-)

Thanks a lot!

- Arnaldo

> Thanks,
> Ian
>
> > ---
> > tools/perf/pmu-events/jevents.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> > index fa86c5f997cc..fc9c158bfa13 100644
> > --- a/tools/perf/pmu-events/jevents.c
> > +++ b/tools/perf/pmu-events/jevents.c
> > @@ -137,7 +137,7 @@ static char *fixregex(char *s)
> > return s;
> >
> > /* allocate space for a new string */
> > - fixed = (char *) malloc(len + 1);
> > + fixed = (char *) malloc(len + esc_count + 1);
> > if (!fixed)
> > return NULL;
> >
> > --
> > 2.28.0.402.g5ffc5be6b7-goog
> >

--

- Arnaldo