tools/perf/util/parse-events.c | 96 ++++++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 16 deletions(-)
The "Read backward ring buffer" test crashes on big-endian (e.g. s390x)
due to a NULL dereference when the backward mmap path isn't enabled.
Reproducer:
# ./perf test -F 'Read backward ring buffer'
Segmentation fault (core dumped)
# uname -m
s390x
#
Root cause:
get_config_terms() stores into evsel_config_term::val.val (u64) while later
code reads boolean fields such as evsel_config_term::val.overwrite.
On big-endian the 1-byte boolean is left-aligned, so writing
evsel_config_term::val.val = 1 is read back as
evsel_config_term::val.overwrite = 0,
leaving backward mmap disabled and a NULL map being used.
Store values in the union member that matches the term type, e.g.:
/* for OVERWRITE */
new_term->val.overwrite = 1; /* not new_term->val.val = 1 */
to fix this. Improve add_config_term() and add two more parameters for
string and value. add_config_term() now creates a complete node element
of type evsel_config_term and handles all evsel_config_term::val union
members.
Impact:
Enables backward mmap on big-endian and prevents the crash.
No change on little-endian.
Output after:
# ./perf test -Fv 44
--- start ---
Using CPUID IBM,9175,705,ME1,3.8,002f
mmap size 1052672B
mmap size 8192B
---- end ----
44: Read backward ring buffer : Ok
#
Fixes: 159ca97cd97c ("perf parse-events: Refactor get_config_terms() to remove macros")
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Jan Polensky <japo@linux.ibm.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Ian Rogers <irogers@google.com>
---
tools/perf/util/parse-events.c | 96 ++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b9efb296bba5..bd48435a3e13 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1117,7 +1117,7 @@ static int config_attr(struct perf_event_attr *attr,
static struct evsel_config_term *add_config_term(enum evsel_term_type type,
struct list_head *head_terms,
- bool weak)
+ bool weak, char *str, u64 val)
{
struct evsel_config_term *t;
@@ -1128,8 +1128,78 @@ static struct evsel_config_term *add_config_term(enum evsel_term_type type,
INIT_LIST_HEAD(&t->list);
t->type = type;
t->weak = weak;
+
+ if (str) {
+ t->val.str = strdup(str);
+ if (!t->val.str) {
+ zfree(&t);
+ return NULL;
+ }
+ t->free_str = true;
+ } else {
+ t->val.val = val;
+ }
+
+ switch (type) {
+ case EVSEL__CONFIG_TERM_PERIOD:
+ /* Union member type match, assignment above */
+ break;
+ case EVSEL__CONFIG_TERM_FREQ:
+ /* Union member type match, assignment above */
+ break;
+ case EVSEL__CONFIG_TERM_TIME:
+ t->val.time = val;
+ break;
+ case EVSEL__CONFIG_TERM_STACK_USER:
+ /* Union member type match, assignment above */
+ break;
+ case EVSEL__CONFIG_TERM_INHERIT:
+ t->val.inherit = val;
+ break;
+ case EVSEL__CONFIG_TERM_OVERWRITE:
+ t->val.overwrite = val;
+ break;
+ case EVSEL__CONFIG_TERM_MAX_STACK:
+ t->val.max_stack = val;
+ break;
+ case EVSEL__CONFIG_TERM_MAX_EVENTS:
+ t->val.max_events = val;
+ break;
+ case EVSEL__CONFIG_TERM_PERCORE:
+ t->val.percore = val;
+ break;
+ case EVSEL__CONFIG_TERM_AUX_OUTPUT:
+ t->val.aux_output = val;
+ break;
+ case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
+ t->val.aux_sample_size = val;
+ break;
+ case EVSEL__CONFIG_TERM_CALLGRAPH:
+ /* Type is string, assigned above */
+ break;
+ case EVSEL__CONFIG_TERM_BRANCH:
+ /* Type is string, assigned above */
+ break;
+ case EVSEL__CONFIG_TERM_DRV_CFG:
+ /* Type is string, assigned above */
+ break;
+ case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
+ /* Type is string, assigned above */
+ break;
+ case EVSEL__CONFIG_TERM_AUX_ACTION:
+ /* Type is string, assigned above */
+ break;
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
+ case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
+ /* Union member type match, assignment above */
+ break;
+ default:
+ }
+
list_add_tail(&t->list, head_terms);
-
return t;
}
@@ -1234,20 +1304,15 @@ static int get_config_terms(const struct parse_events_terms *head_config,
continue;
}
- new_term = add_config_term(new_type, head_terms, term->weak);
+ /*
+ * Note: Members evsel_config_term::val and
+ * parse_events_term::val are unions and endianness needs
+ * to be taken into account when changing such union members.
+ */
+ new_term = add_config_term(new_type, head_terms, term->weak,
+ str_type ? term->val.str : NULL, val);
if (!new_term)
return -ENOMEM;
-
- if (str_type) {
- new_term->val.str = strdup(term->val.str);
- if (!new_term->val.str) {
- zfree(&new_term);
- return -ENOMEM;
- }
- new_term->free_str = true;
- } else {
- new_term->val.val = val;
- }
}
return 0;
}
@@ -1277,10 +1342,9 @@ static int add_cfg_chg(const struct perf_pmu *pmu,
if (bits) {
struct evsel_config_term *new_term;
- new_term = add_config_term(new_term_type, head_terms, false);
+ new_term = add_config_term(new_term_type, head_terms, false, NULL, bits);
if (!new_term)
return -ENOMEM;
- new_term->val.cfg_chg = bits;
}
return 0;
--
2.53.0
On 16/02/2026 1:10 pm, Thomas Richter wrote:
> The "Read backward ring buffer" test crashes on big-endian (e.g. s390x)
> due to a NULL dereference when the backward mmap path isn't enabled.
>
> Reproducer:
> # ./perf test -F 'Read backward ring buffer'
> Segmentation fault (core dumped)
> # uname -m
> s390x
> #
>
> Root cause:
> get_config_terms() stores into evsel_config_term::val.val (u64) while later
> code reads boolean fields such as evsel_config_term::val.overwrite.
> On big-endian the 1-byte boolean is left-aligned, so writing
> evsel_config_term::val.val = 1 is read back as
> evsel_config_term::val.overwrite = 0,
> leaving backward mmap disabled and a NULL map being used.
>
> Store values in the union member that matches the term type, e.g.:
> /* for OVERWRITE */
> new_term->val.overwrite = 1; /* not new_term->val.val = 1 */
> to fix this. Improve add_config_term() and add two more parameters for
> string and value. add_config_term() now creates a complete node element
> of type evsel_config_term and handles all evsel_config_term::val union
> members.
>
> Impact:
> Enables backward mmap on big-endian and prevents the crash.
> No change on little-endian.
>
> Output after:
> # ./perf test -Fv 44
> --- start ---
> Using CPUID IBM,9175,705,ME1,3.8,002f
> mmap size 1052672B
> mmap size 8192B
> ---- end ----
> 44: Read backward ring buffer : Ok
> #
>
> Fixes: 159ca97cd97c ("perf parse-events: Refactor get_config_terms() to remove macros")
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Reviewed-by: Jan Polensky <japo@linux.ibm.com>
> Cc: James Clark <james.clark@linaro.org>
> Cc: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/parse-events.c | 96 ++++++++++++++++++++++++++++------
> 1 file changed, 80 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index b9efb296bba5..bd48435a3e13 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1117,7 +1117,7 @@ static int config_attr(struct perf_event_attr *attr,
>
> static struct evsel_config_term *add_config_term(enum evsel_term_type type,
> struct list_head *head_terms,
> - bool weak)
> + bool weak, char *str, u64 val)
> {
> struct evsel_config_term *t;
>
> @@ -1128,8 +1128,78 @@ static struct evsel_config_term *add_config_term(enum evsel_term_type type,
> INIT_LIST_HEAD(&t->list);
> t->type = type;
> t->weak = weak;
> +
> + if (str) {
> + t->val.str = strdup(str);
> + if (!t->val.str) {
> + zfree(&t);
> + return NULL;
> + }
> + t->free_str = true;
> + } else {
> + t->val.val = val;
> + }
You could drop this if/else and do each thing in the switch, then
there's no need for the comments saying it's already done e.g.:
case EVSEL__CONFIG_TERM_PERIOD
case EVSEL__CONFIG_TERM_FREQ:
case EVSEL__CONFIG_TERM_STACK_USER:
t->val.val = val;
break;
case EVSEL__CONFIG_TERM_DRV_CFG:
case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
case EVSEL__CONFIG_TERM_AUX_ACTION:
assert(str);
t->val.str = strdup(str);
if (!t->val.str) {
zfree(&t);
return NULL;
}
t->free_str = true;
break;
> +
> + switch (type) {
> + case EVSEL__CONFIG_TERM_PERIOD:
> + /* Union member type match, assignment above */
> + break;
> + case EVSEL__CONFIG_TERM_FREQ:
> + /* Union member type match, assignment above */
> + break;
> + case EVSEL__CONFIG_TERM_TIME:
> + t->val.time = val;
> + break;
> + case EVSEL__CONFIG_TERM_STACK_USER:
> + /* Union member type match, assignment above */
> + break;
> + case EVSEL__CONFIG_TERM_INHERIT:
> + t->val.inherit = val;
> + break;
> + case EVSEL__CONFIG_TERM_OVERWRITE:
> + t->val.overwrite = val;
> + break;
> + case EVSEL__CONFIG_TERM_MAX_STACK:
> + t->val.max_stack = val;
> + break;
> + case EVSEL__CONFIG_TERM_MAX_EVENTS:
> + t->val.max_events = val;
> + break;
> + case EVSEL__CONFIG_TERM_PERCORE:
> + t->val.percore = val;
> + break;
> + case EVSEL__CONFIG_TERM_AUX_OUTPUT:
> + t->val.aux_output = val;
> + break;
> + case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
> + t->val.aux_sample_size = val;
> + break;
> + case EVSEL__CONFIG_TERM_CALLGRAPH:
> + /* Type is string, assigned above */
> + break;
> + case EVSEL__CONFIG_TERM_BRANCH:
> + /* Type is string, assigned above */
Minor nit: You can collapse all the ones with the same comments, same as
you have done for EVSEL__CONFIG_TERM_USR_CHG_CONFIG*
> + break;
> + case EVSEL__CONFIG_TERM_DRV_CFG:
> + /* Type is string, assigned above */
> + break;
> + case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
> + /* Type is string, assigned above */
> + break;
> + case EVSEL__CONFIG_TERM_AUX_ACTION:
> + /* Type is string, assigned above */
> + break;
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
> + /* Union member type match, assignment above */
> + break;
> + default:
I get this with clang:
util/parse-events.c:1200:10: error: label at end of compound statement:
expected statement
default:
Other than that it looks correct.
> + }
> +
> list_add_tail(&t->list, head_terms);
> -
> return t;
> }
>
> @@ -1234,20 +1304,15 @@ static int get_config_terms(const struct parse_events_terms *head_config,
> continue;
> }
>
> - new_term = add_config_term(new_type, head_terms, term->weak);
> + /*
> + * Note: Members evsel_config_term::val and
> + * parse_events_term::val are unions and endianness needs
> + * to be taken into account when changing such union members.
> + */
> + new_term = add_config_term(new_type, head_terms, term->weak,
> + str_type ? term->val.str : NULL, val);
> if (!new_term)
> return -ENOMEM;
> -
> - if (str_type) {
> - new_term->val.str = strdup(term->val.str);
> - if (!new_term->val.str) {
> - zfree(&new_term);
> - return -ENOMEM;
> - }
> - new_term->free_str = true;
> - } else {
> - new_term->val.val = val;
> - }
> }
> return 0;
> }
> @@ -1277,10 +1342,9 @@ static int add_cfg_chg(const struct perf_pmu *pmu,
> if (bits) {
> struct evsel_config_term *new_term;
>
> - new_term = add_config_term(new_term_type, head_terms, false);
> + new_term = add_config_term(new_term_type, head_terms, false, NULL, bits);
> if (!new_term)
> return -ENOMEM;
> - new_term->val.cfg_chg = bits;
> }
>
> return 0;
© 2016 - 2026 Red Hat, Inc.