Skip to contentSkip to conversation

perf script: Fix memory leaks in list_scripts()

PATCH In review Gustavo A. R. Silva 3 participants 7 messages Apr 8, 2019 – Aug 27, 2019 1 message not in the archive perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf script: Fix memory leaks in list_scripts()

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +4 -2
index 27cf3ab88d13..f4edb18f67ec 100644
@@ -131,8 +131,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);
Signature · 1 line
2.21.0
#1 lore raw

Hi all,

Friendly ping:

Who can take this?

Thanks

Quoted 28 lines · Gustavo A. R. Silva

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +4 -2
index 27cf3ab88d13..f4edb18f67ec 100644
@@ -131,8 +131,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);
#2 lore raw

Hi all,

Friendly ping (second one after 4 months):

Who can take this?

Thanks
--
Gustavo

Quoted 37 lines · Gustavo A. R. Silva

Hi all,

Friendly ping:

Who can take this?

Thanks

Quoted 28 lines · Gustavo A. R. Silva

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +4 -2
index 27cf3ab88d13..f4edb18f67ec 100644
@@ -131,8 +131,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);
#3 lore raw

Em Sun, Aug 25, 2019 at 11:06:25PM -0500, Gustavo A. R. Silva escreveu:

Quoted 5 lines

Hi all,

Friendly ping (second one after 4 months):

Who can take this?

Sorry for the delay, finally applied, thanks!

- Arnaldo

Quoted 42 lines

Thanks
--
Gustavo

Quoted 37 lines · Gustavo A. R. Silva

Hi all,

Friendly ping:

Who can take this?

Thanks

Quoted 28 lines · Gustavo A. R. Silva

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +4 -2
index 27cf3ab88d13..f4edb18f67ec 100644
@@ -131,8 +131,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);
Signature · 1 line
- Arnaldo
#4 lore raw
Quoted 9 lines · Arnaldo Carvalho de Melo

Em Sun, Aug 25, 2019 at 11:06:25PM -0500, Gustavo A. R. Silva escreveu:

Quoted 5 lines

Hi all,

Friendly ping (second one after 4 months):

Who can take this?

Sorry for the delay, finally applied, thanks!

Awesome. :)

Thanks, Arnaldo.
--
Gustavo

Quoted 45 lines

- Arnaldo

Quoted 42 lines

Thanks
--
Gustavo

Quoted 37 lines · Gustavo A. R. Silva

Hi all,

Friendly ping:

Who can take this?

Thanks

Quoted 28 lines · Gustavo A. R. Silva

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +4 -2
index 27cf3ab88d13..f4edb18f67ec 100644
@@ -131,8 +131,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);

A message that is not in the archive (sent off-list or not yet received): 156689437793.24518.1210962260082729908.tip-bot2@tip-bot2

#6 lore raw

Hi,

Quoted 9 lines · On 8/27/19 3:26 AM, tip-bot2 for Gustavo A. R. Silva wrote:

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     3b4acbb92dbda4829e021e5c6d5410658849fa1c
Gitweb:        https://git.kernel.org/tip/3b4acbb92dbda4829e021e5c6d5410658849fa1c
Author:        Gustavo A. R. Silva <[email protected]>
AuthorDate:    Mon, 08 Apr 2019 11:27:48 -05:00
Committer:     Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Mon, 26 Aug 2019 11:58:30 -03:00
Quoted 6 lines

perf script: Fix memory leaks in list_scripts()

In case memory resources for *buf* and *paths* were allocated, jump to *out* and release them before return.

This should be tagged for stable:

Quoted 22 lines

Cc: Alexander Shishkin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Gustavo A. R. Silva <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Addresses-Coverity-ID: 1444328 ("Resource leak")

Fixes 6f3da20e151f ("perf report: Support builtin perf script in scripts menu")
---
 tools/perf/ui/browsers/scripts.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/ui/browsers/scripts.c +0 -0
index f2fd9f0..50e0c03 100644
@@ -133,8 +133,10 @@ static int list_scripts(char *script_name, bool *custom,
 		int key = ui_browser__input_window("perf script command",
 				"Enter perf script command line (without perf script prefix)",

tools/perf/ui/browsers/scripts.c static int list_scripts(char *script_name, bool *custom, · line 142

Quoted 10 lines
tools/perf/ui/browsers/scripts.c +4 -2
 				script_args, "", 0);
-		if (key != K_ENTER)
-			return -1;
+		if (key != K_ENTER) {
+			ret = -1;
+			goto out;
+		}
 		sprintf(script_name, "%s script %s", perf, script_args);
 	} else if (choice < num + max_std) {
 		strcpy(script_name, paths[choice]);

--
Gustavo

#7 lore raw
Quoted 5 lines
Quoted 1 line

This should be tagged for stable:

It's a theoretical problem (which are explicitely ruled out by stable rules) because if you ever see user space malloc() returning NULL the system is likely already randomly killing your processes in OOM, including eventually perf.

I can see the value of shutting up coverity though, but that's not something that needs to be in stable.

-Andi