linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf jevents: Fix suspicious code in fixregex()
@ 2020-09-03 15:25 Namhyung Kim
  2020-09-03 17:47 ` Ian Rogers
  0 siblings, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2020-09-03 15:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
	Stephane Eranian, LKML, Andi Kleen, John Garry, Kajol Jain,
	Ian Rogers

The new string should have enough space for the original string and
the back slashes IMHO.

Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf jevents: Fix suspicious code in fixregex()
  2020-09-03 15:25 [PATCH] perf jevents: Fix suspicious code in fixregex() Namhyung Kim
@ 2020-09-03 17:47 ` Ian Rogers
  2020-09-03 18:41   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2020-09-03 17:47 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
	Andi Kleen, John Garry, Kajol Jain

On Thu, Sep 3, 2020 at 8:25 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The new string should have enough space for the original string and
> the back slashes IMHO.
>
> Cc: John Garry <john.garry@huawei.com>
> Cc: Kajol Jain <kjain@linux.ibm.com>
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Definitely looks like the right fix. I'm surprised this hasn't shown
up in sanitizer testing.

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf jevents: Fix suspicious code in fixregex()
  2020-09-03 17:47 ` Ian Rogers
@ 2020-09-03 18:41   ` Arnaldo Carvalho de Melo
  2020-09-04  1:38     ` Namhyung Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-09-03 18:41 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Jiri Olsa, William Cohen, Ingo Molnar,
	Peter Zijlstra, Mark Rutland, Alexander Shishkin,
	Stephane Eranian, LKML, Andi Kleen, John Garry, Kajol Jain

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@kernel.org> wrote:
> > The new string should have enough space for the original string and
> > the back slashes IMHO.

> > Cc: John Garry <john.garry@huawei.com>
> > Cc: Kajol Jain <kjain@linux.ibm.com>
> > Cc: Ian Rogers <irogers@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> 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@redhat.com>
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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf jevents: Fix suspicious code in fixregex()
  2020-09-03 18:41   ` Arnaldo Carvalho de Melo
@ 2020-09-04  1:38     ` Namhyung Kim
  2020-09-04  2:09       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2020-09-04  1:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, William Cohen, Ingo Molnar,
	Peter Zijlstra, Mark Rutland, Alexander Shishkin,
	Stephane Eranian, LKML, Andi Kleen, John Garry, Kajol Jain

Hi Arnaldo and Ian,

On Fri, Sep 4, 2020 at 3:41 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> 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@kernel.org> wrote:
> > > The new string should have enough space for the original string and
> > > the back slashes IMHO.
>
> > > Cc: John Garry <john.garry@huawei.com>
> > > Cc: Kajol Jain <kjain@linux.ibm.com>
> > > Cc: Ian Rogers <irogers@google.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Reviewed-by: Ian Rogers <irogers@google.com>
> >
> > Definitely looks like the right fix. I'm surprised this hasn't shown
> > up in sanitizer testing.

I guess the code didn't run on most arch (including x86) since
they don't have backslashes.

>
> 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@redhat.com>
> Fixes: fbc2844e84038ce3 ("perf vendor events: Use more flexible pattern matching for CPU identification for mapfile.csv"
>
> Please consider doing it next time :-)

Oh, right!  Will do it later..

Thanks
Namhyung

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf jevents: Fix suspicious code in fixregex()
  2020-09-04  1:38     ` Namhyung Kim
@ 2020-09-04  2:09       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-09-04  2:09 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Jiri Olsa, William Cohen, Ingo Molnar,
	Peter Zijlstra, Mark Rutland, Alexander Shishkin,
	Stephane Eranian, LKML, Andi Kleen, John Garry, Kajol Jain

Em Fri, Sep 04, 2020 at 10:38:35AM +0900, Namhyung Kim escreveu:
> On Fri, Sep 4, 2020 at 3:41 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > 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@kernel.org> wrote:
> > > > The new string should have enough space for the original string and
> > > > the back slashes IMHO.
> > > > Cc: John Garry <john.garry@huawei.com>
> > > > Cc: Kajol Jain <kjain@linux.ibm.com>
> > > > Cc: Ian Rogers <irogers@google.com>
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > Definitely looks like the right fix. I'm surprised this hasn't shown
> > > up in sanitizer testing.
 
> I guess the code didn't run on most arch (including x86) since
> they don't have backslashes.

> > 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@redhat.com>
> > Fixes: fbc2844e84038ce3 ("perf vendor events: Use more flexible pattern matching for CPU identification for mapfile.csv"

> > Please consider doing it next time :-)
 
> Oh, right!  Will do it later..

Thanks!

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-04  2:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 15:25 [PATCH] perf jevents: Fix suspicious code in fixregex() Namhyung Kim
2020-09-03 17:47 ` Ian Rogers
2020-09-03 18:41   ` Arnaldo Carvalho de Melo
2020-09-04  1:38     ` Namhyung Kim
2020-09-04  2:09       ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).