* [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main()
@ 2019-10-16 13:50 Yunfeng Ye
2019-10-16 14:25 ` Arnaldo Carvalho de Melo
2019-11-12 11:18 ` [tip: perf/core] " tip-bot2 for Yunfeng Ye
0 siblings, 2 replies; 5+ messages in thread
From: Yunfeng Ye @ 2019-10-16 13:50 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, john.garry, ak, lukemujica, yeyunfeng, kan.liang,
yuzenghui
Cc: linux-kernel, hushiyuan, linfeilong
There are memory leaks and file descriptor resource leaks in
process_mapfile() and main().
Fix this by adding free(), fclose() and free_arch_std_events()
on the error paths.
Fixes: 80eeb67fe577 ("perf jevents: Program to convert JSON file")
Fixes: 3f056b66647b ("perf jevents: Make build fail on JSON parse error")
Fixes: e9d32c1bf0cd ("perf vendor events: Add support for arch standard events")
Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
---
v1 -> v2:
- add free(eventsfp) to fix eventsfp resource leaks
- add free_arch_std_events() on the error path
tools/perf/pmu-events/jevents.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index e2837260ca4d..99e3fd04a5cb 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -758,6 +758,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
char *line, *p;
int line_num;
char *tblname;
+ int ret = 0;
pr_info("%s: Processing mapfile %s\n", prog, fpath);
@@ -769,6 +770,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
if (!mapfp) {
pr_info("%s: Error %s opening %s\n", prog, strerror(errno),
fpath);
+ free(line);
return -1;
}
@@ -795,7 +797,8 @@ static int process_mapfile(FILE *outfp, char *fpath)
/* TODO Deal with lines longer than 16K */
pr_info("%s: Mapfile %s: line %d too long, aborting\n",
prog, fpath, line_num);
- return -1;
+ ret = -1;
+ goto out;
}
line[strlen(line)-1] = '\0';
@@ -825,7 +828,9 @@ static int process_mapfile(FILE *outfp, char *fpath)
out:
print_mapping_table_suffix(outfp);
- return 0;
+ fclose(mapfp);
+ free(line);
+ return ret;
}
/*
@@ -1122,6 +1127,7 @@ int main(int argc, char *argv[])
goto empty_map;
} else if (rc < 0) {
/* Make build fail */
+ fclose(eventsfp);
free_arch_std_events();
return 1;
} else if (rc) {
@@ -1134,6 +1140,7 @@ int main(int argc, char *argv[])
goto empty_map;
} else if (rc < 0) {
/* Make build fail */
+ fclose(eventsfp);
free_arch_std_events();
return 1;
} else if (rc) {
@@ -1151,6 +1158,8 @@ int main(int argc, char *argv[])
if (process_mapfile(eventsfp, mapfile)) {
pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
/* Make build fail */
+ fclose(eventsfp);
+ free_arch_std_events();
return 1;
}
--
2.7.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main()
2019-10-16 13:50 [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main() Yunfeng Ye
@ 2019-10-16 14:25 ` Arnaldo Carvalho de Melo
2019-10-23 8:22 ` Yunfeng Ye
2019-11-12 11:18 ` [tip: perf/core] " tip-bot2 for Yunfeng Ye
1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-16 14:25 UTC (permalink / raw)
To: Yunfeng Ye
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
john.garry, ak, lukemujica, kan.liang, yuzenghui, linux-kernel,
hushiyuan, linfeilong
Em Wed, Oct 16, 2019 at 09:50:17PM +0800, Yunfeng Ye escreveu:
> There are memory leaks and file descriptor resource leaks in
> process_mapfile() and main().
>
> Fix this by adding free(), fclose() and free_arch_std_events()
> on the error paths.
>
> Fixes: 80eeb67fe577 ("perf jevents: Program to convert JSON file")
> Fixes: 3f056b66647b ("perf jevents: Make build fail on JSON parse error")
> Fixes: e9d32c1bf0cd ("perf vendor events: Add support for arch standard events")
Nice, thanks for adding the fixes line, I looked at those three patches
and indeed they were leaky, thanks for the fixes, we shouldn't have
those leaks even if that, for now, makes the tool to end anyway.
- Arnaldo
> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
> ---
> v1 -> v2:
> - add free(eventsfp) to fix eventsfp resource leaks
> - add free_arch_std_events() on the error path
>
> tools/perf/pmu-events/jevents.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index e2837260ca4d..99e3fd04a5cb 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -758,6 +758,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
> char *line, *p;
> int line_num;
> char *tblname;
> + int ret = 0;
>
> pr_info("%s: Processing mapfile %s\n", prog, fpath);
>
> @@ -769,6 +770,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
> if (!mapfp) {
> pr_info("%s: Error %s opening %s\n", prog, strerror(errno),
> fpath);
> + free(line);
> return -1;
> }
>
> @@ -795,7 +797,8 @@ static int process_mapfile(FILE *outfp, char *fpath)
> /* TODO Deal with lines longer than 16K */
> pr_info("%s: Mapfile %s: line %d too long, aborting\n",
> prog, fpath, line_num);
> - return -1;
> + ret = -1;
> + goto out;
> }
> line[strlen(line)-1] = '\0';
>
> @@ -825,7 +828,9 @@ static int process_mapfile(FILE *outfp, char *fpath)
>
> out:
> print_mapping_table_suffix(outfp);
> - return 0;
> + fclose(mapfp);
> + free(line);
> + return ret;
> }
>
> /*
> @@ -1122,6 +1127,7 @@ int main(int argc, char *argv[])
> goto empty_map;
> } else if (rc < 0) {
> /* Make build fail */
> + fclose(eventsfp);
> free_arch_std_events();
> return 1;
> } else if (rc) {
> @@ -1134,6 +1140,7 @@ int main(int argc, char *argv[])
> goto empty_map;
> } else if (rc < 0) {
> /* Make build fail */
> + fclose(eventsfp);
> free_arch_std_events();
> return 1;
> } else if (rc) {
> @@ -1151,6 +1158,8 @@ int main(int argc, char *argv[])
> if (process_mapfile(eventsfp, mapfile)) {
> pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
> /* Make build fail */
> + fclose(eventsfp);
> + free_arch_std_events();
> return 1;
> }
>
> --
> 2.7.4.3
--
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main()
2019-10-16 14:25 ` Arnaldo Carvalho de Melo
@ 2019-10-23 8:22 ` Yunfeng Ye
2019-10-24 13:48 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 5+ messages in thread
From: Yunfeng Ye @ 2019-10-23 8:22 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
john.garry, ak, lukemujica, kan.liang, yuzenghui, linux-kernel,
hushiyuan, linfeilong
On 2019/10/16 22:25, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 16, 2019 at 09:50:17PM +0800, Yunfeng Ye escreveu:
>> There are memory leaks and file descriptor resource leaks in
>> process_mapfile() and main().
>>
>> Fix this by adding free(), fclose() and free_arch_std_events()
>> on the error paths.
>>
>> Fixes: 80eeb67fe577 ("perf jevents: Program to convert JSON file")
>> Fixes: 3f056b66647b ("perf jevents: Make build fail on JSON parse error")
>> Fixes: e9d32c1bf0cd ("perf vendor events: Add support for arch standard events")
>
> Nice, thanks for adding the fixes line, I looked at those three patches
> and indeed they were leaky, thanks for the fixes, we shouldn't have
> those leaks even if that, for now, makes the tool to end anyway.
>
The other 3 patchs have been applied, is this patch applied ? thanks.
> - Arnaldo
>
>> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
>> ---
>> v1 -> v2:
>> - add free(eventsfp) to fix eventsfp resource leaks
>> - add free_arch_std_events() on the error path
>>
>> tools/perf/pmu-events/jevents.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
>> index e2837260ca4d..99e3fd04a5cb 100644
>> --- a/tools/perf/pmu-events/jevents.c
>> +++ b/tools/perf/pmu-events/jevents.c
>> @@ -758,6 +758,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
>> char *line, *p;
>> int line_num;
>> char *tblname;
>> + int ret = 0;
>>
>> pr_info("%s: Processing mapfile %s\n", prog, fpath);
>>
>> @@ -769,6 +770,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
>> if (!mapfp) {
>> pr_info("%s: Error %s opening %s\n", prog, strerror(errno),
>> fpath);
>> + free(line);
>> return -1;
>> }
>>
>> @@ -795,7 +797,8 @@ static int process_mapfile(FILE *outfp, char *fpath)
>> /* TODO Deal with lines longer than 16K */
>> pr_info("%s: Mapfile %s: line %d too long, aborting\n",
>> prog, fpath, line_num);
>> - return -1;
>> + ret = -1;
>> + goto out;
>> }
>> line[strlen(line)-1] = '\0';
>>
>> @@ -825,7 +828,9 @@ static int process_mapfile(FILE *outfp, char *fpath)
>>
>> out:
>> print_mapping_table_suffix(outfp);
>> - return 0;
>> + fclose(mapfp);
>> + free(line);
>> + return ret;
>> }
>>
>> /*
>> @@ -1122,6 +1127,7 @@ int main(int argc, char *argv[])
>> goto empty_map;
>> } else if (rc < 0) {
>> /* Make build fail */
>> + fclose(eventsfp);
>> free_arch_std_events();
>> return 1;
>> } else if (rc) {
>> @@ -1134,6 +1140,7 @@ int main(int argc, char *argv[])
>> goto empty_map;
>> } else if (rc < 0) {
>> /* Make build fail */
>> + fclose(eventsfp);
>> free_arch_std_events();
>> return 1;
>> } else if (rc) {
>> @@ -1151,6 +1158,8 @@ int main(int argc, char *argv[])
>> if (process_mapfile(eventsfp, mapfile)) {
>> pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
>> /* Make build fail */
>> + fclose(eventsfp);
>> + free_arch_std_events();
>> return 1;
>> }
>>
>> --
>> 2.7.4.3
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main()
2019-10-23 8:22 ` Yunfeng Ye
@ 2019-10-24 13:48 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-24 13:48 UTC (permalink / raw)
To: Yunfeng Ye
Cc: Arnaldo Carvalho de Melo, peterz, mingo, mark.rutland,
alexander.shishkin, jolsa, namhyung, john.garry, ak, lukemujica,
kan.liang, yuzenghui, linux-kernel, hushiyuan, linfeilong
Em Wed, Oct 23, 2019 at 04:22:25PM +0800, Yunfeng Ye escreveu:
>
>
> On 2019/10/16 22:25, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Oct 16, 2019 at 09:50:17PM +0800, Yunfeng Ye escreveu:
> >> There are memory leaks and file descriptor resource leaks in
> >> process_mapfile() and main().
> >>
> >> Fix this by adding free(), fclose() and free_arch_std_events()
> >> on the error paths.
> >>
> >> Fixes: 80eeb67fe577 ("perf jevents: Program to convert JSON file")
> >> Fixes: 3f056b66647b ("perf jevents: Make build fail on JSON parse error")
> >> Fixes: e9d32c1bf0cd ("perf vendor events: Add support for arch standard events")
> >
> > Nice, thanks for adding the fixes line, I looked at those three patches
> > and indeed they were leaky, thanks for the fixes, we shouldn't have
> > those leaks even if that, for now, makes the tool to end anyway.
> >
> The other 3 patchs have been applied, is this patch applied ? thanks.
Applied now, thanks for the reminder,
- Arnaldo
> > - Arnaldo
> >
> >> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
> >> ---
> >> v1 -> v2:
> >> - add free(eventsfp) to fix eventsfp resource leaks
> >> - add free_arch_std_events() on the error path
> >>
> >> tools/perf/pmu-events/jevents.c | 13 +++++++++++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> >> index e2837260ca4d..99e3fd04a5cb 100644
> >> --- a/tools/perf/pmu-events/jevents.c
> >> +++ b/tools/perf/pmu-events/jevents.c
> >> @@ -758,6 +758,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
> >> char *line, *p;
> >> int line_num;
> >> char *tblname;
> >> + int ret = 0;
> >>
> >> pr_info("%s: Processing mapfile %s\n", prog, fpath);
> >>
> >> @@ -769,6 +770,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
> >> if (!mapfp) {
> >> pr_info("%s: Error %s opening %s\n", prog, strerror(errno),
> >> fpath);
> >> + free(line);
> >> return -1;
> >> }
> >>
> >> @@ -795,7 +797,8 @@ static int process_mapfile(FILE *outfp, char *fpath)
> >> /* TODO Deal with lines longer than 16K */
> >> pr_info("%s: Mapfile %s: line %d too long, aborting\n",
> >> prog, fpath, line_num);
> >> - return -1;
> >> + ret = -1;
> >> + goto out;
> >> }
> >> line[strlen(line)-1] = '\0';
> >>
> >> @@ -825,7 +828,9 @@ static int process_mapfile(FILE *outfp, char *fpath)
> >>
> >> out:
> >> print_mapping_table_suffix(outfp);
> >> - return 0;
> >> + fclose(mapfp);
> >> + free(line);
> >> + return ret;
> >> }
> >>
> >> /*
> >> @@ -1122,6 +1127,7 @@ int main(int argc, char *argv[])
> >> goto empty_map;
> >> } else if (rc < 0) {
> >> /* Make build fail */
> >> + fclose(eventsfp);
> >> free_arch_std_events();
> >> return 1;
> >> } else if (rc) {
> >> @@ -1134,6 +1140,7 @@ int main(int argc, char *argv[])
> >> goto empty_map;
> >> } else if (rc < 0) {
> >> /* Make build fail */
> >> + fclose(eventsfp);
> >> free_arch_std_events();
> >> return 1;
> >> } else if (rc) {
> >> @@ -1151,6 +1158,8 @@ int main(int argc, char *argv[])
> >> if (process_mapfile(eventsfp, mapfile)) {
> >> pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
> >> /* Make build fail */
> >> + fclose(eventsfp);
> >> + free_arch_std_events();
> >> return 1;
> >> }
> >>
> >> --
> >> 2.7.4.3
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: perf/core] perf jevents: Fix resource leak in process_mapfile() and main()
2019-10-16 13:50 [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main() Yunfeng Ye
2019-10-16 14:25 ` Arnaldo Carvalho de Melo
@ 2019-11-12 11:18 ` tip-bot2 for Yunfeng Ye
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Yunfeng Ye @ 2019-11-12 11:18 UTC (permalink / raw)
To: linux-tip-commits
Cc: Yunfeng Ye, Alexander Shishkin, Andi Kleen, Feilong Lin,
Hu Shiyuan, Jiri Olsa, John Garry, Kan Liang, Luke Mujica,
Mark Rutland, Namhyung Kim, Peter Zijlstra, Zenghui Yu,
Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
linux-kernel
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 1785fbb73896dbd9d27a406f0d73047df42db710
Gitweb: https://git.kernel.org/tip/1785fbb73896dbd9d27a406f0d73047df42db710
Author: Yunfeng Ye <yeyunfeng@huawei.com>
AuthorDate: Wed, 16 Oct 2019 21:50:17 +08:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00
perf jevents: Fix resource leak in process_mapfile() and main()
There are memory leaks and file descriptor resource leaks in
process_mapfile() and main().
Fix this by adding free(), fclose() and free_arch_std_events() on the
error paths.
Fixes: 80eeb67fe577 ("perf jevents: Program to convert JSON file")
Fixes: 3f056b66647b ("perf jevents: Make build fail on JSON parse error")
Fixes: e9d32c1bf0cd ("perf vendor events: Add support for arch standard events")
Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Feilong Lin <linfeilong@huawei.com>
Cc: Hu Shiyuan <hushiyuan@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Luke Mujica <lukemujica@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Zenghui Yu <yuzenghui@huawei.com>
Link: http://lore.kernel.org/lkml/d7907042-ec9c-2bef-25b4-810e14602f89@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/pmu-events/jevents.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 7d69727..079c77b 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -772,6 +772,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
char *line, *p;
int line_num;
char *tblname;
+ int ret = 0;
pr_info("%s: Processing mapfile %s\n", prog, fpath);
@@ -783,6 +784,7 @@ static int process_mapfile(FILE *outfp, char *fpath)
if (!mapfp) {
pr_info("%s: Error %s opening %s\n", prog, strerror(errno),
fpath);
+ free(line);
return -1;
}
@@ -809,7 +811,8 @@ static int process_mapfile(FILE *outfp, char *fpath)
/* TODO Deal with lines longer than 16K */
pr_info("%s: Mapfile %s: line %d too long, aborting\n",
prog, fpath, line_num);
- return -1;
+ ret = -1;
+ goto out;
}
line[strlen(line)-1] = '\0';
@@ -839,7 +842,9 @@ static int process_mapfile(FILE *outfp, char *fpath)
out:
print_mapping_table_suffix(outfp);
- return 0;
+ fclose(mapfp);
+ free(line);
+ return ret;
}
/*
@@ -1136,6 +1141,7 @@ int main(int argc, char *argv[])
goto empty_map;
} else if (rc < 0) {
/* Make build fail */
+ fclose(eventsfp);
free_arch_std_events();
return 1;
} else if (rc) {
@@ -1148,6 +1154,7 @@ int main(int argc, char *argv[])
goto empty_map;
} else if (rc < 0) {
/* Make build fail */
+ fclose(eventsfp);
free_arch_std_events();
return 1;
} else if (rc) {
@@ -1165,6 +1172,8 @@ int main(int argc, char *argv[])
if (process_mapfile(eventsfp, mapfile)) {
pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
/* Make build fail */
+ fclose(eventsfp);
+ free_arch_std_events();
return 1;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-12 11:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-16 13:50 [PATCH v2] perf jevents: Fix resource leak in process_mapfile() and main() Yunfeng Ye
2019-10-16 14:25 ` Arnaldo Carvalho de Melo
2019-10-23 8:22 ` Yunfeng Ye
2019-10-24 13:48 ` Arnaldo Carvalho de Melo
2019-11-12 11:18 ` [tip: perf/core] " tip-bot2 for Yunfeng Ye
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).