| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230831071421.2201358-4-irogers@google.com>
Date: Thu, 31 Aug 2023 00:14:21 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Rob Herring <robh@...nel.org>,
James Clark <james.clark@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v1 3/3] perf parse-events: Fix propagation of term's no_value
when cloning
The no_value field in parse_events_term indicates that the val
variable isn't used, the case for an event name. Cloning wasn't
propagating this, making cloned event name terms appearing to have a
constant assinged to them. Working around the bug would check for a
value of 1 assigned to value, but then this meant a user value of 1
couldn't be differentiated causing the value to be lost in debug
printing and perf list.
The change fixes the cloning and updates the "val.num ==/!= 1" tests
to use no_value instead. To better check the no_value is set
appropriately parameter comments are added for constant values. This
found that no_value wasn't set correctly in
parse_events_multi_pmu_add, which matters now that no_value is used to
indicate an event name.
Fixes: 7a6e91644708 ("perf parse-events: Make common term list to strbuf helper")
Fixes: 99e7138eb789 ("perf tools: Fail on using multiple bits long terms without value")
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/util/parse-events.c | 29 +++++++++++++----------------
tools/perf/util/parse-events.y | 9 +++++----
tools/perf/util/pmu.c | 2 +-
3 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 51b73207e9f4..68fe2c4ff49f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1510,8 +1510,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
if (parse_events_term__num(&term,
PARSE_EVENTS__TERM_TYPE_USER,
- config, 1, false, NULL,
- NULL) < 0) {
+ config, /*num=*/1, /*novalue=*/true,
+ loc, /*loc_val=*/NULL) < 0) {
zfree(&config);
goto out_err;
}
@@ -2482,7 +2482,7 @@ int parse_events_term__num(struct parse_events_term **term,
.err_val = loc_val ? loc_val->first_column : 0,
};
- return new_term(term, &temp, NULL, num);
+ return new_term(term, &temp, /*str=*/NULL, num);
}
int parse_events_term__str(struct parse_events_term **term,
@@ -2501,7 +2501,7 @@ int parse_events_term__str(struct parse_events_term **term,
.err_val = loc_val ? loc_val->first_column : 0,
};
- return new_term(term, &temp, str, 0);
+ return new_term(term, &temp, str, /*num=*/0);
}
int parse_events_term__term(struct parse_events_term **term,
@@ -2518,26 +2518,21 @@ int parse_events_term__clone(struct parse_events_term **new,
struct parse_events_term *term)
{
char *str;
- struct parse_events_term temp = {
- .type_val = term->type_val,
- .type_term = term->type_term,
- .config = NULL,
- .err_term = term->err_term,
- .err_val = term->err_val,
- };
+ struct parse_events_term temp = *term;
+ temp.used = false;
if (term->config) {
temp.config = strdup(term->config);
if (!temp.config)
return -ENOMEM;
}
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
- return new_term(new, &temp, NULL, term->val.num);
+ return new_term(new, &temp, /*str=*/NULL, term->val.num);
str = strdup(term->val.str);
if (!str)
return -ENOMEM;
- return new_term(new, &temp, str, 0);
+ return new_term(new, &temp, str, /*num=*/0);
}
void parse_events_term__delete(struct parse_events_term *term)
@@ -2611,20 +2606,22 @@ int parse_events_term__to_strbuf(struct list_head *term_list, struct strbuf *sb)
first = false;
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
- if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER && term->val.num == 1)
+ if (term->no_value) {
+ assert(term->type_term == PARSE_EVENTS__TERM_TYPE_USER);
ret = strbuf_addf(sb, "%s", term->config);
- else
+ } else
ret = strbuf_addf(sb, "%s=%#"PRIx64, term->config, term->val.num);
else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
if (term->config) {
ret = strbuf_addf(sb, "%s=", term->config);
if (ret < 0)
return ret;
- } else if (term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
+ } else if ((unsigned int)term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
ret = strbuf_addf(sb, "%s=", config_term_name(term->type_term));
if (ret < 0)
return ret;
}
+ assert(!term->no_value);
ret = strbuf_addf(sb, "%s", term->val.str);
}
if (ret < 0)
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 534daed91c50..4a305df61f74 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -712,7 +712,7 @@ name_or_raw '=' PE_VALUE
{
struct parse_events_term *term;
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $3, false, &@1, &@3);
+ $1, $3, /*novalue=*/false, &@1, &@3);
if (err) {
free($1);
@@ -739,7 +739,7 @@ PE_LEGACY_CACHE
{
struct parse_events_term *term;
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE,
- $1, 1, true, &@1, NULL);
+ $1, /*num=*/1, /*novalue=*/true, &@1, /*loc_val=*/NULL);
if (err) {
free($1);
@@ -752,7 +752,7 @@ PE_NAME
{
struct parse_events_term *term;
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, 1, true, &@1, NULL);
+ $1, /*num=*/1, /*novalue=*/true, &@1, /*loc_val=*/NULL);
if (err) {
free($1);
@@ -765,7 +765,8 @@ PE_TERM_HW
{
struct parse_events_term *term;
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_HARDWARE,
- $1.str, $1.num & 255, false, &@1, NULL);
+ $1.str, $1.num & 255, /*novalue=*/false,
+ &@1, /*loc_val=*/NULL);
if (err) {
free($1.str);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 152cda84f273..d85602aa4b9f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1396,7 +1396,7 @@ static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,
return NULL;
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
- if (term->val.num != 1)
+ if (!term->no_value)
return NULL;
if (pmu_find_format(&pmu->format, term->config))
return NULL;
--
2.42.0.rc2.253.gd59a3bf2b4-goog
Powered by blists - more mailing lists