* [PATCH] perf parse-events: add defensive null check @ 2020-03-25 16:40 Ian Rogers 2020-03-25 19:25 ` Arnaldo Carvalho de Melo 2020-04-04 8:41 ` [tip: perf/urgent] perf parse-events: Add defensive NULL check tip-bot2 for Ian Rogers 0 siblings, 2 replies; 4+ messages in thread From: Ian Rogers @ 2020-03-25 16:40 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry, Jin Yao, Andi Kleen, Kan Liang, linux-kernel, clang-built-linux Cc: Stephane Eranian, Ian Rogers Terms may have a null config in which case a strcmp will segv. This can be reproduced with: perf stat -e '*/event=?,nr/' sleep 1 Add a null check to avoid this. This was caught by LLVM's libfuzzer. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/pmu.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 616fbda7c3fc..ef6a63f3d386 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -984,12 +984,11 @@ static int pmu_resolve_param_term(struct parse_events_term *term, struct parse_events_term *t; list_for_each_entry(t, head_terms, list) { - if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM) { - if (!strcmp(t->config, term->config)) { - t->used = true; - *value = t->val.num; - return 0; - } + if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM && + t->config && !strcmp(t->config, term->config)) { + t->used = true; + *value = t->val.num; + return 0; } } -- 2.25.1.696.g5e7596f4ac-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf parse-events: add defensive null check 2020-03-25 16:40 [PATCH] perf parse-events: add defensive null check Ian Rogers @ 2020-03-25 19:25 ` Arnaldo Carvalho de Melo 2020-03-25 21:35 ` Ian Rogers 2020-04-04 8:41 ` [tip: perf/urgent] perf parse-events: Add defensive NULL check tip-bot2 for Ian Rogers 1 sibling, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-03-25 19:25 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry, Jin Yao, Andi Kleen, Kan Liang, linux-kernel, clang-built-linux, Stephane Eranian Em Wed, Mar 25, 2020 at 09:40:22AM -0700, Ian Rogers escreveu: > Terms may have a null config in which case a strcmp will segv. This can > be reproduced with: > perf stat -e '*/event=?,nr/' sleep 1 > Add a null check to avoid this. This was caught by LLVM's libfuzzer. Adding the NULL check doesn't hurt, I guess, but I coudln't repro it: [root@seventh ~]# perf stat -e '*/event=?,nr/' sleep 1 WARNING: multiple event parsing errors event syntax error: '*/event=?,nr/' \___ 'nr' is not usable in 'perf stat' Initial error: event syntax error: '*/event=?,nr/' \___ Cannot find PMU `*'. Missing kernel support? Run 'perf list' for a list of valid events Usage: perf stat [<options>] [<command>] -e, --event <event> event selector. use 'perf list' to list available events [root@seventh ~]# Does this take place only when libfuzzer is being used? - Arnaldo > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/pmu.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 616fbda7c3fc..ef6a63f3d386 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -984,12 +984,11 @@ static int pmu_resolve_param_term(struct parse_events_term *term, > struct parse_events_term *t; > > list_for_each_entry(t, head_terms, list) { > - if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM) { > - if (!strcmp(t->config, term->config)) { > - t->used = true; > - *value = t->val.num; > - return 0; > - } > + if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM && > + t->config && !strcmp(t->config, term->config)) { > + t->used = true; > + *value = t->val.num; > + return 0; > } > } > > -- > 2.25.1.696.g5e7596f4ac-goog > -- - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf parse-events: add defensive null check 2020-03-25 19:25 ` Arnaldo Carvalho de Melo @ 2020-03-25 21:35 ` Ian Rogers 0 siblings, 0 replies; 4+ messages in thread From: Ian Rogers @ 2020-03-25 21:35 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry, Jin Yao, Andi Kleen, Kan Liang, LKML, clang-built-linux, Stephane Eranian On Wed, Mar 25, 2020 at 12:25 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Wed, Mar 25, 2020 at 09:40:22AM -0700, Ian Rogers escreveu: > > Terms may have a null config in which case a strcmp will segv. This can > > be reproduced with: > > perf stat -e '*/event=?,nr/' sleep 1 > > Add a null check to avoid this. This was caught by LLVM's libfuzzer. > > Adding the NULL check doesn't hurt, I guess, but I coudln't repro it: > > [root@seventh ~]# perf stat -e '*/event=?,nr/' sleep 1 > WARNING: multiple event parsing errors > event syntax error: '*/event=?,nr/' > \___ 'nr' is not usable in 'perf stat' > > Initial error: > event syntax error: '*/event=?,nr/' > \___ Cannot find PMU `*'. Missing kernel support? > Run 'perf list' for a list of valid events > > Usage: perf stat [<options>] [<command>] > > -e, --event <event> event selector. use 'perf list' to list available events > [root@seventh ~]# > > Does this take place only when libfuzzer is being used? Good catch! I was driving the repro through the fuzzer and getting the stack trace below and had assumed this would repro on the command line. I'm now wondering why it won't reproduce :-) I suspect this issue can come up in other scenarios so as you say the null check won't hurt, but the commit message is incorrect. #0 __interceptor_strcmp () at llvm/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:448 #1 0x0000555557345eb2 in pmu_resolve_param_term (term=0x607000081190, head_terms=0x602000007cb0, value=0x7ffff4cc0320) at tools/perf/util/pmu.c:994 #2 0x00005555573389a4 in pmu_config_term (formats=0x6080000012c8, attr=0x7ffff4dd4e20, term=0x607000081190, head_terms=0x602000007cb0, zero=false, err=0x7ffff4c90020) at tools/perf/util/pmu.c:1117 #3 0x0000555557337b4e in perf_pmu__config_terms (formats=0x6080000012c8, attr=0x7ffff4dd4e20, head_terms=0x602000007cb0, zero=false, err=0x7ffff4c90020) at tools/perf/util/pmu.c:1154 #4 0x0000555557338f7e in perf_pmu__config (pmu=0x6080000012a0, attr=0x7ffff4dd4e20, head_terms=0x602000007cb0, err=0x7ffff4c90020) at tools/perf/util/pmu.c:1174 #5 0x0000555557314027 in parse_events_add_pmu (parse_state=0x7ffff4c900a0, list=0x602000007e10, name=0x602000001910 "uncore_cha_1", head_config=0x602000007cb0, auto_merge_stats=true, use_alias=false) at tools/perf/util/parse-events.c:1485 #6 0x0000555556d257f9 in parse_events_parse (_parse_state=0x7ffff4c900a0, scanner=0x611000000cc0) at tools/perf/util/parse-events.y:318 #7 0x000055555731c573 in parse_events__scanner (str=0x7ffff4d85c40 "*/event=?,nr/", parse_state=0x7ffff4c900a0, start_token=258) at tools/perf/util/parse-events.c:2026 #8 0x000055555731cbc3 in parse_events (evlist=0x61e000000080, str=0x7ffff4d85c40 "*/event=?,nr/", err=0x7ffff4c90020) at tools/perf/util/parse-events.c:2066 Thanks, Ian > - Arnaldo > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/pmu.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > > index 616fbda7c3fc..ef6a63f3d386 100644 > > --- a/tools/perf/util/pmu.c > > +++ b/tools/perf/util/pmu.c > > @@ -984,12 +984,11 @@ static int pmu_resolve_param_term(struct parse_events_term *term, > > struct parse_events_term *t; > > > > list_for_each_entry(t, head_terms, list) { > > - if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM) { > > - if (!strcmp(t->config, term->config)) { > > - t->used = true; > > - *value = t->val.num; > > - return 0; > > - } > > + if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM && > > + t->config && !strcmp(t->config, term->config)) { > > + t->used = true; > > + *value = t->val.num; > > + return 0; > > } > > } > > > > -- > > 2.25.1.696.g5e7596f4ac-goog > > > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip: perf/urgent] perf parse-events: Add defensive NULL check 2020-03-25 16:40 [PATCH] perf parse-events: add defensive null check Ian Rogers 2020-03-25 19:25 ` Arnaldo Carvalho de Melo @ 2020-04-04 8:41 ` tip-bot2 for Ian Rogers 1 sibling, 0 replies; 4+ messages in thread From: tip-bot2 for Ian Rogers @ 2020-04-04 8:41 UTC (permalink / raw) To: linux-tip-commits Cc: Ian Rogers, Alexander Shishkin, Andi Kleen, Jin Yao, Jiri Olsa, John Garry, Kan Liang, Mark Rutland, Namhyung Kim, Peter Zijlstra, Stephane Eranian, clang-built-linux, Arnaldo Carvalho de Melo, x86, LKML The following commit has been merged into the perf/urgent branch of tip: Commit-ID: 2a3d252dffe14582f238e21b09923e3772263123 Gitweb: https://git.kernel.org/tip/2a3d252dffe14582f238e21b09923e3772263123 Author: Ian Rogers <irogers@google.com> AuthorDate: Wed, 25 Mar 2020 09:40:22 -07:00 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitterDate: Thu, 26 Mar 2020 11:03:53 -03:00 perf parse-events: Add defensive NULL check Terms may have a NULL config in which case a strcmp will SEGV. This can be reproduced with: perf stat -e '*/event=?,nr/' sleep 1 Add a NULL check to avoid this. This was caught by LLVM's libfuzzer. Signed-off-by: Ian Rogers <irogers@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: Jin Yao <yao.jin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: John Garry <john.garry@huawei.com> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: clang-built-linux@googlegroups.com Link: http://lore.kernel.org/lkml/20200325164022.41385-1-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/pmu.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 616fbda..ef6a63f 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -984,12 +984,11 @@ static int pmu_resolve_param_term(struct parse_events_term *term, struct parse_events_term *t; list_for_each_entry(t, head_terms, list) { - if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM) { - if (!strcmp(t->config, term->config)) { - t->used = true; - *value = t->val.num; - return 0; - } + if (t->type_val == PARSE_EVENTS__TERM_TYPE_NUM && + t->config && !strcmp(t->config, term->config)) { + t->used = true; + *value = t->val.num; + return 0; } } ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-04 8:43 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-25 16:40 [PATCH] perf parse-events: add defensive null check Ian Rogers 2020-03-25 19:25 ` Arnaldo Carvalho de Melo 2020-03-25 21:35 ` Ian Rogers 2020-04-04 8:41 ` [tip: perf/urgent] perf parse-events: Add defensive NULL check tip-bot2 for Ian Rogers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.