Skip to content

Commit 07eafd4

Browse files
captain5050acmel
authored andcommitted
perf parse-event: Add init and exit to parse_event_error
parse_events() may succeed but leave string memory allocations reachable in the error. Add an init/exit that must be called to initialize and clean up the error. This fixes a leak in metricgroup parse_ids. Signed-off-by: Ian Rogers <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: John Garry <[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]>
1 parent 6c19128 commit 07eafd4

File tree

13 files changed

+74
-51
lines changed

13 files changed

+74
-51
lines changed

tools/perf/arch/powerpc/util/kvm-stat.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,11 @@ static int is_tracepoint_available(const char *str, struct evlist *evlist)
113113
struct parse_events_error err;
114114
int ret;
115115

116-
bzero(&err, sizeof(err));
116+
parse_events_error__init(&err);
117117
ret = parse_events(evlist, str, &err);
118118
if (err.str)
119119
parse_events_error__print(&err, "tracepoint");
120+
parse_events_error__exit(&err);
120121
return ret;
121122
}
122123

tools/perf/bench/evlist-open-close.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ static int evlist__count_evsel_fds(struct evlist *evlist)
7878

7979
static struct evlist *bench__create_evlist(char *evstr)
8080
{
81-
struct parse_events_error err = { .idx = 0, };
81+
struct parse_events_error err;
8282
struct evlist *evlist = evlist__new();
8383
int ret;
8484

@@ -87,14 +87,16 @@ static struct evlist *bench__create_evlist(char *evstr)
8787
return NULL;
8888
}
8989

90+
parse_events_error__init(&err);
9091
ret = parse_events(evlist, evstr, &err);
9192
if (ret) {
9293
parse_events_error__print(&err, evstr);
94+
parse_events_error__exit(&err);
9395
pr_err("Run 'perf list' for a list of valid events\n");
9496
ret = 1;
9597
goto out_delete_evlist;
9698
}
97-
99+
parse_events_error__exit(&err);
98100
ret = evlist__create_maps(evlist, &opts.target);
99101
if (ret < 0) {
100102
pr_err("Not enough memory to create thread/cpu maps\n");

tools/perf/builtin-stat.c

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1750,14 +1750,12 @@ static int add_default_attributes(void)
17501750
(PERF_COUNT_HW_CACHE_OP_PREFETCH << 8) |
17511751
(PERF_COUNT_HW_CACHE_RESULT_MISS << 16) },
17521752
};
1753-
struct parse_events_error errinfo;
1754-
17551753
/* Set attrs if no event is selected and !null_run: */
17561754
if (stat_config.null_run)
17571755
return 0;
17581756

1759-
bzero(&errinfo, sizeof(errinfo));
17601757
if (transaction_run) {
1758+
struct parse_events_error errinfo;
17611759
/* Handle -T as -M transaction. Once platform specific metrics
17621760
* support has been added to the json files, all architectures
17631761
* will use this approach. To determine transaction support
@@ -1772,6 +1770,7 @@ static int add_default_attributes(void)
17721770
&stat_config.metric_events);
17731771
}
17741772

1773+
parse_events_error__init(&errinfo);
17751774
if (pmu_have_event("cpu", "cycles-ct") &&
17761775
pmu_have_event("cpu", "el-start"))
17771776
err = parse_events(evsel_list, transaction_attrs,
@@ -1783,12 +1782,13 @@ static int add_default_attributes(void)
17831782
if (err) {
17841783
fprintf(stderr, "Cannot set up transaction events\n");
17851784
parse_events_error__print(&errinfo, transaction_attrs);
1786-
return -1;
17871785
}
1788-
return 0;
1786+
parse_events_error__exit(&errinfo);
1787+
return err ? -1 : 0;
17891788
}
17901789

17911790
if (smi_cost) {
1791+
struct parse_events_error errinfo;
17921792
int smi;
17931793

17941794
if (sysfs__read_int(FREEZE_ON_SMI_PATH, &smi) < 0) {
@@ -1804,23 +1804,23 @@ static int add_default_attributes(void)
18041804
smi_reset = true;
18051805
}
18061806

1807-
if (pmu_have_event("msr", "aperf") &&
1808-
pmu_have_event("msr", "smi")) {
1809-
if (!force_metric_only)
1810-
stat_config.metric_only = true;
1811-
err = parse_events(evsel_list, smi_cost_attrs, &errinfo);
1812-
} else {
1807+
if (!pmu_have_event("msr", "aperf") ||
1808+
!pmu_have_event("msr", "smi")) {
18131809
fprintf(stderr, "To measure SMI cost, it needs "
18141810
"msr/aperf/, msr/smi/ and cpu/cycles/ support\n");
1815-
parse_events_error__print(&errinfo, smi_cost_attrs);
18161811
return -1;
18171812
}
1813+
if (!force_metric_only)
1814+
stat_config.metric_only = true;
1815+
1816+
parse_events_error__init(&errinfo);
1817+
err = parse_events(evsel_list, smi_cost_attrs, &errinfo);
18181818
if (err) {
18191819
parse_events_error__print(&errinfo, smi_cost_attrs);
18201820
fprintf(stderr, "Cannot set up SMI cost events\n");
1821-
return -1;
18221821
}
1823-
return 0;
1822+
parse_events_error__exit(&errinfo);
1823+
return err ? -1 : 0;
18241824
}
18251825

18261826
if (topdown_run) {
@@ -1875,18 +1875,22 @@ static int add_default_attributes(void)
18751875
return -1;
18761876
}
18771877
if (topdown_attrs[0] && str) {
1878+
struct parse_events_error errinfo;
18781879
if (warn)
18791880
arch_topdown_group_warn();
18801881
setup_metrics:
1882+
parse_events_error__init(&errinfo);
18811883
err = parse_events(evsel_list, str, &errinfo);
18821884
if (err) {
18831885
fprintf(stderr,
18841886
"Cannot set up top down events %s: %d\n",
18851887
str, err);
18861888
parse_events_error__print(&errinfo, str);
1889+
parse_events_error__exit(&errinfo);
18871890
free(str);
18881891
return -1;
18891892
}
1893+
parse_events_error__exit(&errinfo);
18901894
} else {
18911895
fprintf(stderr, "System does not support topdown\n");
18921896
return -1;
@@ -1896,6 +1900,7 @@ static int add_default_attributes(void)
18961900

18971901
if (!evsel_list->core.nr_entries) {
18981902
if (perf_pmu__has_hybrid()) {
1903+
struct parse_events_error errinfo;
18991904
const char *hybrid_str = "cycles,instructions,branches,branch-misses";
19001905

19011906
if (target__has_cpu(&target))
@@ -1906,15 +1911,16 @@ static int add_default_attributes(void)
19061911
return -1;
19071912
}
19081913

1914+
parse_events_error__init(&errinfo);
19091915
err = parse_events(evsel_list, hybrid_str, &errinfo);
19101916
if (err) {
19111917
fprintf(stderr,
19121918
"Cannot set up hybrid events %s: %d\n",
19131919
hybrid_str, err);
19141920
parse_events_error__print(&errinfo, hybrid_str);
1915-
return -1;
19161921
}
1917-
return err;
1922+
parse_events_error__exit(&errinfo);
1923+
return err ? -1 : 0;
19181924
}
19191925

19201926
if (target__has_cpu(&target))

tools/perf/builtin-trace.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3063,15 +3063,11 @@ static bool evlist__add_vfs_getname(struct evlist *evlist)
30633063
struct parse_events_error err;
30643064
int ret;
30653065

3066-
bzero(&err, sizeof(err));
3066+
parse_events_error__init(&err);
30673067
ret = parse_events(evlist, "probe:vfs_getname*", &err);
3068-
if (ret) {
3069-
free(err.str);
3070-
free(err.help);
3071-
free(err.first_str);
3072-
free(err.first_help);
3068+
parse_events_error__exit(&err);
3069+
if (ret)
30733070
return false;
3074-
}
30753071

30763072
evlist__for_each_entry_safe(evlist, evsel, tmp) {
30773073
if (!strstarts(evsel__name(evsel), "probe:vfs_getname"))
@@ -4925,12 +4921,13 @@ int cmd_trace(int argc, const char **argv)
49254921
if (trace.perfconfig_events != NULL) {
49264922
struct parse_events_error parse_err;
49274923

4928-
bzero(&parse_err, sizeof(parse_err));
4924+
parse_events_error__init(&parse_err);
49294925
err = parse_events(trace.evlist, trace.perfconfig_events, &parse_err);
4930-
if (err) {
4926+
if (err)
49314927
parse_events_error__print(&parse_err, trace.perfconfig_events);
4928+
parse_events_error__exit(&parse_err);
4929+
if (err)
49324930
goto out;
4933-
}
49344931
}
49354932

49364933
if ((nr_cgroups || trace.cgroup) && !trace.opts.target.system_wide) {

tools/perf/tests/backward-ring-buffer.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,13 @@ int test__backward_ring_buffer(struct test *test __maybe_unused, int subtest __m
115115
goto out_delete_evlist;
116116
}
117117

118-
bzero(&parse_error, sizeof(parse_error));
118+
parse_events_error__init(&parse_error);
119119
/*
120120
* Set backward bit, ring buffer should be writing from end. Record
121121
* it in aux evlist
122122
*/
123123
err = parse_events(evlist, "syscalls:sys_enter_prctl/overwrite/", &parse_error);
124+
parse_events_error__exit(&parse_error);
124125
if (err) {
125126
pr_debug("Failed to parse tracepoint event, try use root\n");
126127
ret = TEST_SKIP;

tools/perf/tests/bpf.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,13 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
123123
struct parse_events_state parse_state;
124124
struct parse_events_error parse_error;
125125

126-
bzero(&parse_error, sizeof(parse_error));
126+
parse_events_error__init(&parse_error);
127127
bzero(&parse_state, sizeof(parse_state));
128128
parse_state.error = &parse_error;
129129
INIT_LIST_HEAD(&parse_state.list);
130130

131131
err = parse_events_load_bpf_obj(&parse_state, &parse_state.list, obj, NULL);
132+
parse_events_error__exit(&parse_error);
132133
if (err || list_empty(&parse_state.list)) {
133134
pr_debug("Failed to add events selected by BPF\n");
134135
return TEST_FAIL;

tools/perf/tests/expand-cgroup.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ static int expand_group_events(void)
124124
evlist = evlist__new();
125125
TEST_ASSERT_VAL("failed to get evlist", evlist);
126126

127+
parse_events_error__init(&err);
127128
ret = parse_events(evlist, event_str, &err);
128129
if (ret < 0) {
129130
pr_debug("failed to parse event '%s', err %d, str '%s'\n",
@@ -135,6 +136,7 @@ static int expand_group_events(void)
135136
rblist__init(&metric_events);
136137
ret = test_expand_events(evlist, &metric_events);
137138
out:
139+
parse_events_error__exit(&err);
138140
evlist__delete(evlist);
139141
return ret;
140142
}

tools/perf/tests/parse-events.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,7 +2045,6 @@ static int test_event(struct evlist_test *e)
20452045
struct evlist *evlist;
20462046
int ret;
20472047

2048-
bzero(&err, sizeof(err));
20492048
if (e->valid && !e->valid()) {
20502049
pr_debug("... SKIP");
20512050
return 0;
@@ -2055,6 +2054,7 @@ static int test_event(struct evlist_test *e)
20552054
if (evlist == NULL)
20562055
return -ENOMEM;
20572056

2057+
parse_events_error__init(&err);
20582058
ret = parse_events(evlist, e->name, &err);
20592059
if (ret) {
20602060
pr_debug("failed to parse event '%s', err %d, str '%s'\n",
@@ -2063,7 +2063,7 @@ static int test_event(struct evlist_test *e)
20632063
} else {
20642064
ret = e->check(evlist);
20652065
}
2066-
2066+
parse_events_error__exit(&err);
20672067
evlist__delete(evlist);
20682068

20692069
return ret;

tools/perf/tests/pmu-events.c

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -787,9 +787,11 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
787787

788788
static int check_parse_cpu(const char *id, bool same_cpu, const struct pmu_event *pe)
789789
{
790-
struct parse_events_error error = { .idx = 0, };
790+
struct parse_events_error error;
791+
int ret;
791792

792-
int ret = check_parse_id(id, &error, NULL);
793+
parse_events_error__init(&error);
794+
ret = check_parse_id(id, &error, NULL);
793795
if (ret && same_cpu) {
794796
pr_warning("Parse event failed metric '%s' id '%s' expr '%s'\n",
795797
pe->metric_name, id, pe->metric_expr);
@@ -800,22 +802,18 @@ static int check_parse_cpu(const char *id, bool same_cpu, const struct pmu_event
800802
id, pe->metric_name, pe->metric_expr);
801803
ret = 0;
802804
}
803-
free(error.str);
804-
free(error.help);
805-
free(error.first_str);
806-
free(error.first_help);
805+
parse_events_error__exit(&error);
807806
return ret;
808807
}
809808

810809
static int check_parse_fake(const char *id)
811810
{
812-
struct parse_events_error error = { .idx = 0, };
813-
int ret = check_parse_id(id, &error, &perf_pmu__fake);
811+
struct parse_events_error error;
812+
int ret;
814813

815-
free(error.str);
816-
free(error.help);
817-
free(error.first_str);
818-
free(error.first_help);
814+
parse_events_error__init(&error);
815+
ret = check_parse_id(id, &error, &perf_pmu__fake);
816+
parse_events_error__exit(&error);
819817
return ret;
820818
}
821819

tools/perf/tests/topology.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ static int session_write_header(char *path)
4949

5050
session->evlist = evlist__new();
5151
TEST_ASSERT_VAL("can't get evlist", session->evlist);
52+
parse_events_error__init(&err);
5253
parse_events(session->evlist, "cpu_core/cycles/", &err);
54+
parse_events_error__exit(&err);
5355
}
5456

5557
perf_header__set_feat(&session->header, HEADER_CPU_TOPOLOGY);

0 commit comments

Comments
 (0)