Skip to content

Commit

Permalink
perf evsel: Fix memory leaks relating to unit
Browse files Browse the repository at this point in the history
unit may have a strdup pointer or be to a literal, consequently memory
assocciated with it isn't freed. Change it so the unit is always strdup
and so the memory can be safely freed.

Fix related issue in perf_event__process_event_update() for name and
own_cpus. Leaks were spotted by leak sanitizer.

Signed-off-by: Ian Rogers <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
captain5050 authored and acmel committed Nov 18, 2021
1 parent d9fc706 commit b194c9c
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 18 deletions.
5 changes: 2 additions & 3 deletions tools/perf/tests/event_update.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
struct evsel *evsel;
struct event_name tmp;
struct evlist *evlist = evlist__new_default();
char *unit = strdup("KRAVA");

TEST_ASSERT_VAL("failed to get evlist", evlist);

Expand All @@ -99,7 +98,8 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes

perf_evlist__id_add(&evlist->core, &evsel->core, 0, 0, 123);

evsel->unit = unit;
free((char *)evsel->unit);
evsel->unit = strdup("KRAVA");

TEST_ASSERT_VAL("failed to synthesize attr update unit",
!perf_event__synthesize_event_update_unit(NULL, evsel, process_event_unit));
Expand All @@ -119,7 +119,6 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
TEST_ASSERT_VAL("failed to synthesize attr update cpus",
!perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus));

free(unit);
evlist__delete(evlist);
return 0;
}
Expand Down
18 changes: 9 additions & 9 deletions tools/perf/util/evsel.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void evsel__init(struct evsel *evsel,
{
perf_evsel__init(&evsel->core, attr, idx);
evsel->tracking = !idx;
evsel->unit = "";
evsel->unit = strdup("");
evsel->scale = 1.0;
evsel->max_events = ULONG_MAX;
evsel->evlist = NULL;
Expand Down Expand Up @@ -276,13 +276,8 @@ struct evsel *evsel__new_idx(struct perf_event_attr *attr, int idx)
}

if (evsel__is_clock(evsel)) {
/*
* The evsel->unit points to static alias->unit
* so it's ok to use static string in here.
*/
static const char *unit = "msec";

evsel->unit = unit;
free((char *)evsel->unit);
evsel->unit = strdup("msec");
evsel->scale = 1e-6;
}

Expand Down Expand Up @@ -420,7 +415,11 @@ struct evsel *evsel__clone(struct evsel *orig)

evsel->max_events = orig->max_events;
evsel->tool_event = orig->tool_event;
evsel->unit = orig->unit;
free((char *)evsel->unit);
evsel->unit = strdup(orig->unit);
if (evsel->unit == NULL)
goto out_err;

evsel->scale = orig->scale;
evsel->snapshot = orig->snapshot;
evsel->per_pkg = orig->per_pkg;
Expand Down Expand Up @@ -1441,6 +1440,7 @@ void evsel__exit(struct evsel *evsel)
zfree(&evsel->group_name);
zfree(&evsel->name);
zfree(&evsel->pmu_name);
zfree(&evsel->unit);
zfree(&evsel->metric_id);
evsel__zero_per_pkg(evsel);
hashmap__free(evsel->per_pkg_mask);
Expand Down
8 changes: 5 additions & 3 deletions tools/perf/util/header.c
Original file line number Diff line number Diff line change
Expand Up @@ -4257,9 +4257,11 @@ int perf_event__process_event_update(struct perf_tool *tool __maybe_unused,

switch (ev->type) {
case PERF_EVENT_UPDATE__UNIT:
free((char *)evsel->unit);
evsel->unit = strdup(ev->data);
break;
case PERF_EVENT_UPDATE__NAME:
free(evsel->name);
evsel->name = strdup(ev->data);
break;
case PERF_EVENT_UPDATE__SCALE:
Expand All @@ -4268,11 +4270,11 @@ int perf_event__process_event_update(struct perf_tool *tool __maybe_unused,
break;
case PERF_EVENT_UPDATE__CPUS:
ev_cpus = (struct perf_record_event_update_cpus *)ev->data;

map = cpu_map__new_data(&ev_cpus->cpus);
if (map)
if (map) {
perf_cpu_map__put(evsel->core.own_cpus);
evsel->core.own_cpus = map;
else
} else
pr_err("failed to get event_update cpus\n");
default:
break;
Expand Down
9 changes: 6 additions & 3 deletions tools/perf/util/parse-events.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,10 @@ static int add_event_tool(struct list_head *list, int *idx,
if (!evsel)
return -ENOMEM;
evsel->tool_event = tool_event;
if (tool_event == PERF_TOOL_DURATION_TIME)
evsel->unit = "ns";
if (tool_event == PERF_TOOL_DURATION_TIME) {
free((char *)evsel->unit);
evsel->unit = strdup("ns");
}
return 0;
}

Expand Down Expand Up @@ -1630,7 +1632,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
if (parse_state->fake_pmu)
return 0;

evsel->unit = info.unit;
free((char *)evsel->unit);
evsel->unit = strdup(info.unit);
evsel->scale = info.scale;
evsel->per_pkg = info.per_pkg;
evsel->snapshot = info.snapshot;
Expand Down

0 comments on commit b194c9c

Please sign in to comment.