History

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 9c880c24cb0d ("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/49c6fccb-b716-1bf0-18a6-cace1cdb66b9@huawei.com/

John Garry committed 5 years ago

Commit fc96ec41 parent 9ed13a11 file · +3 −3Browse files

Unified · Split

tools/perf/util/metricgroup.c+3−3
@@ -162,10 +162,10 @@ static bool contains_event(struct evsel **metric_events, int num_events, · 161 unmodified lines
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, · 116 unmodified lines
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);

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t