Skip to content

Commit 72a8b9c

Browse files
Thomas Richteracmel
authored andcommitted
perf parse-events: Fix big-endian 'overwrite' by writing correct union member
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. Function 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: 159ca97 ("perf parse-events: Refactor get_config_terms() to remove macros") Reviewed-by: James Clark <james.clark@linaro.org> Reviewed-by: Jan Polensky <japo@linux.ibm.com> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com> Acked-by: Ian Rogers <irogers@google.com> Cc: James Clark <james.clark@linaro.org> Signed-off-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
1 parent 8dd1d9a commit 72a8b9c

1 file changed

Lines changed: 65 additions & 17 deletions

File tree

tools/perf/util/parse-events.c

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,7 +1117,7 @@ static int config_attr(struct perf_event_attr *attr,
11171117

11181118
static struct evsel_config_term *add_config_term(enum evsel_term_type type,
11191119
struct list_head *head_terms,
1120-
bool weak)
1120+
bool weak, char *str, u64 val)
11211121
{
11221122
struct evsel_config_term *t;
11231123

@@ -1128,8 +1128,62 @@ static struct evsel_config_term *add_config_term(enum evsel_term_type type,
11281128
INIT_LIST_HEAD(&t->list);
11291129
t->type = type;
11301130
t->weak = weak;
1131-
list_add_tail(&t->list, head_terms);
11321131

1132+
switch (type) {
1133+
case EVSEL__CONFIG_TERM_PERIOD:
1134+
case EVSEL__CONFIG_TERM_FREQ:
1135+
case EVSEL__CONFIG_TERM_STACK_USER:
1136+
case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
1137+
case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
1138+
case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
1139+
case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
1140+
case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
1141+
t->val.val = val;
1142+
break;
1143+
case EVSEL__CONFIG_TERM_TIME:
1144+
t->val.time = val;
1145+
break;
1146+
case EVSEL__CONFIG_TERM_INHERIT:
1147+
t->val.inherit = val;
1148+
break;
1149+
case EVSEL__CONFIG_TERM_OVERWRITE:
1150+
t->val.overwrite = val;
1151+
break;
1152+
case EVSEL__CONFIG_TERM_MAX_STACK:
1153+
t->val.max_stack = val;
1154+
break;
1155+
case EVSEL__CONFIG_TERM_MAX_EVENTS:
1156+
t->val.max_events = val;
1157+
break;
1158+
case EVSEL__CONFIG_TERM_PERCORE:
1159+
t->val.percore = val;
1160+
break;
1161+
case EVSEL__CONFIG_TERM_AUX_OUTPUT:
1162+
t->val.aux_output = val;
1163+
break;
1164+
case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
1165+
t->val.aux_sample_size = val;
1166+
break;
1167+
case EVSEL__CONFIG_TERM_CALLGRAPH:
1168+
case EVSEL__CONFIG_TERM_BRANCH:
1169+
case EVSEL__CONFIG_TERM_DRV_CFG:
1170+
case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
1171+
case EVSEL__CONFIG_TERM_AUX_ACTION:
1172+
if (str) {
1173+
t->val.str = strdup(str);
1174+
if (!t->val.str) {
1175+
zfree(&t);
1176+
return NULL;
1177+
}
1178+
t->free_str = true;
1179+
}
1180+
break;
1181+
default:
1182+
t->val.val = val;
1183+
break;
1184+
}
1185+
1186+
list_add_tail(&t->list, head_terms);
11331187
return t;
11341188
}
11351189

@@ -1142,7 +1196,7 @@ static int get_config_terms(const struct parse_events_terms *head_config,
11421196
struct evsel_config_term *new_term;
11431197
enum evsel_term_type new_type;
11441198
bool str_type = false;
1145-
u64 val;
1199+
u64 val = 0;
11461200

11471201
switch (term->type_term) {
11481202
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
@@ -1234,20 +1288,15 @@ static int get_config_terms(const struct parse_events_terms *head_config,
12341288
continue;
12351289
}
12361290

1237-
new_term = add_config_term(new_type, head_terms, term->weak);
1291+
/*
1292+
* Note: Members evsel_config_term::val and
1293+
* parse_events_term::val are unions and endianness needs
1294+
* to be taken into account when changing such union members.
1295+
*/
1296+
new_term = add_config_term(new_type, head_terms, term->weak,
1297+
str_type ? term->val.str : NULL, val);
12381298
if (!new_term)
12391299
return -ENOMEM;
1240-
1241-
if (str_type) {
1242-
new_term->val.str = strdup(term->val.str);
1243-
if (!new_term->val.str) {
1244-
zfree(&new_term);
1245-
return -ENOMEM;
1246-
}
1247-
new_term->free_str = true;
1248-
} else {
1249-
new_term->val.val = val;
1250-
}
12511300
}
12521301
return 0;
12531302
}
@@ -1277,10 +1326,9 @@ static int add_cfg_chg(const struct perf_pmu *pmu,
12771326
if (bits) {
12781327
struct evsel_config_term *new_term;
12791328

1280-
new_term = add_config_term(new_term_type, head_terms, false);
1329+
new_term = add_config_term(new_term_type, head_terms, false, NULL, bits);
12811330
if (!new_term)
12821331
return -ENOMEM;
1283-
new_term->val.cfg_chg = bits;
12841332
}
12851333

12861334
return 0;

0 commit comments

Comments
 (0)