Skip to content

Commit fc96ec4

Browse files
John Garryacmel
John Garry
authored andcommitted
perf metricgroup: Fix find_evsel_group() event selector
The following command segfaults on my x86 broadwell: $ ./perf stat -M frontend_bound,retiring,backend_bound,bad_speculation sleep 1 WARNING: grouped events cpus do not match, disabling group: anon group { raw 0x10e } anon group { raw 0x10e } perf: util/evsel.c:1596: get_group_fd: Assertion `!(!leader->core.fd)' failed. Aborted (core dumped) The issue shows itself as a use-after-free in evlist__check_cpu_maps(), whereby the leader of an event selector (evsel) has been deleted (yet we still attempt to verify for an evsel). Fundamentally the problem comes from metricgroup__setup_events() -> find_evsel_group(), and has developed from the previous fix attempt in commit 9c880c2 ("perf metricgroup: Fix for metrics containing duration_time"). The problem now is that the logic in checking if an evsel is in the same group is subtly broken for the "cycles" event. For the "cycles" event, the pmu_name is NULL; however the logic in find_evsel_group() may set an event matched against "cycles" as used, when it should not be. This leads to a condition where an evsel is set, yet its leader is not. Fix the check for evsel pmu_name by not matching evsels when either has a NULL pmu_name. There is still a pre-existing metric issue whereby the ordering of the metrics may break the 'stat' function, as discussed at: https://lore.kernel.org/lkml/[email protected]/ Fixes: 9c880c2 ("perf metricgroup: Fix for metrics containing duration_time") Signed-off-by: John Garry <[email protected]> Tested-by: Arnaldo Carvalho de Melo <[email protected]> # On a Thinkpad T450S Cc: Alexander Shishkin <[email protected]> Cc: Ian Rogers <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Kajol Jain <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Link: http://lore.kernel.org/lkml/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 9ed13a1 commit fc96ec4

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

tools/perf/util/metricgroup.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ static bool contains_event(struct evsel **metric_events, int num_events,
162162
return false;
163163
}
164164

165-
static bool evsel_same_pmu(struct evsel *ev1, struct evsel *ev2)
165+
static bool evsel_same_pmu_or_none(struct evsel *ev1, struct evsel *ev2)
166166
{
167167
if (!ev1->pmu_name || !ev2->pmu_name)
168-
return false;
168+
return true;
169169

170170
return !strcmp(ev1->pmu_name, ev2->pmu_name);
171171
}
@@ -288,7 +288,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
288288
*/
289289
if (!has_constraint &&
290290
ev->leader != metric_events[i]->leader &&
291-
evsel_same_pmu(ev->leader, metric_events[i]->leader))
291+
evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
292292
break;
293293
if (!strcmp(metric_events[i]->name, ev->name)) {
294294
set_bit(ev->idx, evlist_used);

0 commit comments

Comments
 (0)