Skip to content

Commit d26e314

Browse files
captain5050gregkh
authored andcommitted
perf cgroup: Update metric leader in evlist__expand_cgroup
[ Upstream commit c9ef786 ] When the evlist is expanded the metric leader wasn't being updated. As the original evsel is deleted this creates a use-after-free in stat-shadow's prepare_metric. This was detected running the "perf stat --bpf-counters --for-each-cgroup test" with sanitizers. The change itself puts the copied evsel into the priv field (known unused because of evsel__clone use) and then in a second pass over the list updates the copied values using the priv pointer. Fixes: d1c5a0e ("perf stat: Add --for-each-cgroup option") Signed-off-by: Ian Rogers <irogers@google.com> Acked-by: Sun Jian <sun.jian.kdev@gmail.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 9cd2640 commit d26e314

1 file changed

Lines changed: 23 additions & 7 deletions

File tree

tools/perf/util/cgroup.c

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,6 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str,
417417
struct rblist *metric_events, bool open_cgroup)
418418
{
419419
struct evlist *orig_list, *tmp_list;
420-
struct evsel *pos, *evsel, *leader;
421420
struct rblist orig_metric_events;
422421
struct cgroup *cgrp = NULL;
423422
struct cgroup_name *cn;
@@ -456,6 +455,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str,
456455
goto out_err;
457456

458457
list_for_each_entry(cn, &cgroup_list, list) {
458+
struct evsel *pos;
459459
char *name;
460460

461461
if (!cn->used)
@@ -471,21 +471,37 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str,
471471
if (cgrp == NULL)
472472
continue;
473473

474-
leader = NULL;
474+
/* copy the list and set to the new cgroup. */
475475
evlist__for_each_entry(orig_list, pos) {
476-
evsel = evsel__clone(/*dest=*/NULL, pos);
476+
struct evsel *evsel = evsel__clone(/*dest=*/NULL, pos);
477+
477478
if (evsel == NULL)
478479
goto out_err;
479480

481+
/* stash the copy during the copying. */
482+
pos->priv = evsel;
480483
cgroup__put(evsel->cgrp);
481484
evsel->cgrp = cgroup__get(cgrp);
482485

483-
if (evsel__is_group_leader(pos))
484-
leader = evsel;
485-
evsel__set_leader(evsel, leader);
486-
487486
evlist__add(tmp_list, evsel);
488487
}
488+
/* update leader information using stashed pointer to copy. */
489+
evlist__for_each_entry(orig_list, pos) {
490+
struct evsel *evsel = pos->priv;
491+
492+
if (evsel__leader(pos))
493+
evsel__set_leader(evsel, evsel__leader(pos)->priv);
494+
495+
if (pos->metric_leader)
496+
evsel->metric_leader = pos->metric_leader->priv;
497+
498+
if (pos->first_wildcard_match)
499+
evsel->first_wildcard_match = pos->first_wildcard_match->priv;
500+
}
501+
/* the stashed copy is no longer used. */
502+
evlist__for_each_entry(orig_list, pos)
503+
pos->priv = NULL;
504+
489505
/* cgroup__new() has a refcount, release it here */
490506
cgroup__put(cgrp);
491507
nr_cgroups++;

0 commit comments

Comments
 (0)