[PATCH 0/5] perf tool: small coverity clean ups

[PATCH 0/5] perf tool: small coverity clean ups

[Sanskriti Sharma]

This patch set fixes a few coverity static code analyzer complaints. Build 
tested only.

Sanskriti Sharma (5):
  perf strbuf: match va_{add,copy} with va_end
  perf tools: cleanup trace-event-info 'tdata' leak
  perf tools: free 'printk' string in parse_ftrace_printk()
  perf tools: avoid double free in read_event_file()
  perf tools: free temporary 'sys' string in read_event_files()

 tools/perf/util/strbuf.c            | 10 ++++++++--
 tools/perf/util/trace-event-info.c  |  2 ++
 tools/perf/util/trace-event-parse.c |  1 +
 tools/perf/util/trace-event-read.c  |  9 +++++----
 4 files changed, 16 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] perf strbuf: match va_{add,copy} with va_end

[Sanskriti Sharma]

Ensure that all code paths in strbuf_addv() call va_end() on the
ap_saved copy that was made.

Fixes the following coverity complaint:

  Error: VARARGS (CWE-237): [#def683]
  tools/perf/util/strbuf.c:106: missing_va_end: va_end was not called
  for "ap_saved".

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
---
 tools/perf/util/strbuf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
index 3d1cf5b..9005fbe 100644
--- a/tools/perf/util/strbuf.c
+++ b/tools/perf/util/strbuf.c
@@ -98,19 +98,25 @@ static int strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap)
 
 	va_copy(ap_saved, ap);
 	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
-	if (len < 0)
+	if (len < 0) {
+		va_end(ap_saved);
 		return len;
+	}
 	if (len > strbuf_avail(sb)) {
 		ret = strbuf_grow(sb, len);
-		if (ret)
+		if (ret) {
+			va_end(ap_saved);
 			return ret;
+		}
 		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved);
 		va_end(ap_saved);
 		if (len > strbuf_avail(sb)) {
 			pr_debug("this should not happen, your vsnprintf is broken");
+			va_end(ap_saved);
 			return -EINVAL;
 		}
 	}
+	va_end(ap_saved);
 	return strbuf_setlen(sb, sb->len + len);
 }
 
-- 
1.8.3.1

Re: [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 10:29:10AM -0400, Sanskriti Sharma escreveu:
> Ensure that all code paths in strbuf_addv() call va_end() on the
> ap_saved copy that was made.
> 
> Fixes the following coverity complaint:
> 
>   Error: VARARGS (CWE-237): [#def683]
>   tools/perf/util/strbuf.c:106: missing_va_end: va_end was not called
>   for "ap_saved".
 

Thanks, applied to perf/core

- Arnaldo

> Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
> ---
>  tools/perf/util/strbuf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
> index 3d1cf5b..9005fbe 100644
> --- a/tools/perf/util/strbuf.c
> +++ b/tools/perf/util/strbuf.c
> @@ -98,19 +98,25 @@ static int strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap)
>  
>  	va_copy(ap_saved, ap);
>  	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> -	if (len < 0)
> +	if (len < 0) {
> +		va_end(ap_saved);
>  		return len;
> +	}
>  	if (len > strbuf_avail(sb)) {
>  		ret = strbuf_grow(sb, len);
> -		if (ret)
> +		if (ret) {
> +			va_end(ap_saved);
>  			return ret;
> +		}
>  		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved);
>  		va_end(ap_saved);
>  		if (len > strbuf_avail(sb)) {
>  			pr_debug("this should not happen, your vsnprintf is broken");
> +			va_end(ap_saved);
>  			return -EINVAL;
>  		}
>  	}
> +	va_end(ap_saved);
>  	return strbuf_setlen(sb, sb->len + len);
>  }
>  
> -- 
> 1.8.3.1

[tip:perf/core] perf strbuf: Match va_{add,copy} with va_end

[tip-bot for Sanskriti Sharma]

Commit-ID:  ce49d8436cffa9b7a6a5f110879d53e89dbc6746
Gitweb:     https://git.kernel.org/tip/ce49d8436cffa9b7a6a5f110879d53e89dbc6746
Author:     Sanskriti Sharma <sansharm@redhat.com>
AuthorDate: Tue, 2 Oct 2018 10:29:10 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 14:23:44 -0300

perf strbuf: Match va_{add,copy} with va_end

Ensure that all code paths in strbuf_addv() call va_end() on the
ap_saved copy that was made.

Fixes the following coverity complaint:

  Error: VARARGS (CWE-237): [#def683]
  tools/perf/util/strbuf.c:106: missing_va_end: va_end was not called
  for "ap_saved".

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Link: http://lkml.kernel.org/r/1538490554-8161-2-git-send-email-sansharm@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/strbuf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
index 3d1cf5bf7f18..9005fbe0780e 100644
--- a/tools/perf/util/strbuf.c
+++ b/tools/perf/util/strbuf.c
@@ -98,19 +98,25 @@ static int strbuf_addv(struct strbuf *sb, const char *fmt, va_list ap)
 
 	va_copy(ap_saved, ap);
 	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
-	if (len < 0)
+	if (len < 0) {
+		va_end(ap_saved);
 		return len;
+	}
 	if (len > strbuf_avail(sb)) {
 		ret = strbuf_grow(sb, len);
-		if (ret)
+		if (ret) {
+			va_end(ap_saved);
 			return ret;
+		}
 		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved);
 		va_end(ap_saved);
 		if (len > strbuf_avail(sb)) {
 			pr_debug("this should not happen, your vsnprintf is broken");
+			va_end(ap_saved);
 			return -EINVAL;
 		}
 	}
+	va_end(ap_saved);
 	return strbuf_setlen(sb, sb->len + len);
 }
 

[PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak

[Sanskriti Sharma]

Free tracing_data structure in tracing_data_get() error paths.

Fixes the following coverity complaint:

  Error: RESOURCE_LEAK (CWE-772):
  leaked_storage: Variable "tdata" going out of scope leaks the storage

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
---
 tools/perf/util/trace-event-info.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index 7b0ca7c..8ad8e75 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -531,12 +531,14 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
 			 "/tmp/perf-XXXXXX");
 		if (!mkstemp(tdata->temp_file)) {
 			pr_debug("Can't make temp file");
+			free(tdata);
 			return NULL;
 		}
 
 		temp_fd = open(tdata->temp_file, O_RDWR);
 		if (temp_fd < 0) {
 			pr_debug("Can't read '%s'", tdata->temp_file);
+			free(tdata);
 			return NULL;
 		}
 
-- 
1.8.3.1

Re: [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 10:29:11AM -0400, Sanskriti Sharma escreveu:
> Free tracing_data structure in tracing_data_get() error paths.
> 
> Fixes the following coverity complaint:
> 
>   Error: RESOURCE_LEAK (CWE-772):
>   leaked_storage: Variable "tdata" going out of scope leaks the storage

Thanks, applied

- Arnaldo
 
> Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
> ---
>  tools/perf/util/trace-event-info.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
> index 7b0ca7c..8ad8e75 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -531,12 +531,14 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
>  			 "/tmp/perf-XXXXXX");
>  		if (!mkstemp(tdata->temp_file)) {
>  			pr_debug("Can't make temp file");
> +			free(tdata);
>  			return NULL;
>  		}
>  
>  		temp_fd = open(tdata->temp_file, O_RDWR);
>  		if (temp_fd < 0) {
>  			pr_debug("Can't read '%s'", tdata->temp_file);
> +			free(tdata);
>  			return NULL;
>  		}
>  
> -- 
> 1.8.3.1

[tip:perf/core] perf tools: Cleanup trace-event-info 'tdata' leak

[tip-bot for Sanskriti Sharma]

Commit-ID:  faedbf3fd19f2511a39397f76359e4cc6ee93072
Gitweb:     https://git.kernel.org/tip/faedbf3fd19f2511a39397f76359e4cc6ee93072
Author:     Sanskriti Sharma <sansharm@redhat.com>
AuthorDate: Tue, 2 Oct 2018 10:29:11 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 14:23:45 -0300

perf tools: Cleanup trace-event-info 'tdata' leak

Free tracing_data structure in tracing_data_get() error paths.

Fixes the following coverity complaint:

  Error: RESOURCE_LEAK (CWE-772):
  leaked_storage: Variable "tdata" going out of scope leaks the storage

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Link: http://lkml.kernel.org/r/1538490554-8161-3-git-send-email-sansharm@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-info.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index 7b0ca7cbb7de..8ad8e755127b 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -531,12 +531,14 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
 			 "/tmp/perf-XXXXXX");
 		if (!mkstemp(tdata->temp_file)) {
 			pr_debug("Can't make temp file");
+			free(tdata);
 			return NULL;
 		}
 
 		temp_fd = open(tdata->temp_file, O_RDWR);
 		if (temp_fd < 0) {
 			pr_debug("Can't read '%s'", tdata->temp_file);
+			free(tdata);
 			return NULL;
 		}
 

[PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk()

[Sanskriti Sharma]

parse_ftrace_printk() tokenizes and parses a line, calling strdup() each
iteration.  Add code to free this temporary format string duplicate.

Fixes the following coverity complaints:

  Error: RESOURCE_LEAK (CWE-772):
  tools/perf/util/trace-event-parse.c:158: overwrite_var: Overwriting
  "printk" in "printk = strdup(fmt + 1)" leaks the storage that "printk"
  points to.

  tools/perf/util/trace-event-parse.c:162: leaked_storage: Variable
  "printk" going out of scope leaks the storage it points to.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
---
 tools/perf/util/trace-event-parse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index e76214f..b15a9bf 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -158,6 +158,7 @@ void parse_ftrace_printk(struct tep_handle *pevent,
 		printk = strdup(fmt+1);
 		line = strtok_r(NULL, "\n", &next);
 		tep_register_print_string(pevent, printk, addr);
+		free(printk);
 	}
 }
 
-- 
1.8.3.1

Re: [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk()

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 10:29:12AM -0400, Sanskriti Sharma escreveu:
> parse_ftrace_printk() tokenizes and parses a line, calling strdup() each
> iteration.  Add code to free this temporary format string duplicate.
> 
> Fixes the following coverity complaints:
> 
>   Error: RESOURCE_LEAK (CWE-772):
>   tools/perf/util/trace-event-parse.c:158: overwrite_var: Overwriting
>   "printk" in "printk = strdup(fmt + 1)" leaks the storage that "printk"
>   points to.
> 
>   tools/perf/util/trace-event-parse.c:162: leaked_storage: Variable
>   "printk" going out of scope leaks the storage it points to.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
> ---
>  tools/perf/util/trace-event-parse.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index e76214f..b15a9bf 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -158,6 +158,7 @@ void parse_ftrace_printk(struct tep_handle *pevent,
>  		printk = strdup(fmt+1);
>  		line = strtok_r(NULL, "\n", &next);
>  		tep_register_print_string(pevent, printk, addr);
> +		free(printk);
>  	}
>  }
>  
> -- 
> 1.8.3.1

[tip:perf/core] perf tools: Free 'printk' string in parse_ftrace_printk()

[tip-bot for Sanskriti Sharma]

Commit-ID:  9c8a182e5a73e01afd11742a2ab887bf338fdafd
Gitweb:     https://git.kernel.org/tip/9c8a182e5a73e01afd11742a2ab887bf338fdafd
Author:     Sanskriti Sharma <sansharm@redhat.com>
AuthorDate: Tue, 2 Oct 2018 10:29:12 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 14:23:45 -0300

perf tools: Free 'printk' string in parse_ftrace_printk()

parse_ftrace_printk() tokenizes and parses a line, calling strdup() each
iteration.  Add code to free this temporary format string duplicate.

Fixes the following coverity complaints:

  Error: RESOURCE_LEAK (CWE-772):
  tools/perf/util/trace-event-parse.c:158: overwrite_var: Overwriting
  "printk" in "printk = strdup(fmt + 1)" leaks the storage that "printk"
  points to.

  tools/perf/util/trace-event-parse.c:162: leaked_storage: Variable
  "printk" going out of scope leaks the storage it points to.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Link: http://lkml.kernel.org/r/1538490554-8161-4-git-send-email-sansharm@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-parse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index a4d7de1c96d1..02f97f5dd588 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -158,6 +158,7 @@ void parse_ftrace_printk(struct tep_handle *pevent,
 		printk = strdup(fmt+1);
 		line = strtok_r(NULL, "\n", &next);
 		tep_register_print_string(pevent, printk, addr);
+		free(printk);
 	}
 }
 

[PATCH 4/5] perf tools: avoid double free in read_event_file()

[Sanskriti Sharma]

The temporary 'buf' buffer allocated in read_event_file() may be freed
twice.  Move the free() call to the common function exit point.

Fixes the following coverity complaints:

  Error: USE_AFTER_FREE (CWE-825):
  tools/perf/util/trace-event-read.c:309: double_free: Calling "free"
  frees pointer "buf" which has already been freed.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
---
 tools/perf/util/trace-event-read.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 3dfc1db..6a0d0f2 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -297,10 +297,8 @@ static int read_event_file(struct tep_handle *pevent, char *sys,
 	}
 
 	ret = do_read(buf, size);
-	if (ret < 0) {
-		free(buf);
+	if (ret < 0)
 		goto out;
-	}
 
 	ret = parse_event_file(pevent, buf, size, sys);
 	if (ret < 0)
-- 
1.8.3.1

Re: [PATCH 4/5] perf tools: avoid double free in read_event_file()

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 10:29:13AM -0400, Sanskriti Sharma escreveu:
> The temporary 'buf' buffer allocated in read_event_file() may be freed
> twice.  Move the free() call to the common function exit point.
> 
> Fixes the following coverity complaints:
> 
>   Error: USE_AFTER_FREE (CWE-825):
>   tools/perf/util/trace-event-read.c:309: double_free: Calling "free"
>   frees pointer "buf" which has already been freed.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
> ---
>  tools/perf/util/trace-event-read.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index 3dfc1db..6a0d0f2 100644
> --- a/tools/perf/util/trace-event-read.c
> +++ b/tools/perf/util/trace-event-read.c
> @@ -297,10 +297,8 @@ static int read_event_file(struct tep_handle *pevent, char *sys,
>  	}
>  
>  	ret = do_read(buf, size);
> -	if (ret < 0) {
> -		free(buf);
> +	if (ret < 0)
>  		goto out;
> -	}
>  
>  	ret = parse_event_file(pevent, buf, size, sys);
>  	if (ret < 0)
> -- 
> 1.8.3.1

[tip:perf/core] perf tools: Avoid double free in read_event_file()

[tip-bot for Sanskriti Sharma]

Commit-ID:  470c8f7c88de013d266e1b61044efe8937728b7f
Gitweb:     https://git.kernel.org/tip/470c8f7c88de013d266e1b61044efe8937728b7f
Author:     Sanskriti Sharma <sansharm@redhat.com>
AuthorDate: Tue, 2 Oct 2018 10:29:13 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 14:23:46 -0300

perf tools: Avoid double free in read_event_file()

The temporary 'buf' buffer allocated in read_event_file() may be freed
twice.  Move the free() call to the common function exit point.

Fixes the following coverity complaints:

  Error: USE_AFTER_FREE (CWE-825):
  tools/perf/util/trace-event-read.c:309: double_free: Calling "free"
  frees pointer "buf" which has already been freed.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Link: http://lkml.kernel.org/r/1538490554-8161-5-git-send-email-sansharm@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-read.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index b98ee2a2eb44..a278e1eee5f5 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -297,10 +297,8 @@ static int read_event_file(struct tep_handle *pevent, char *sys,
 	}
 
 	ret = do_read(buf, size);
-	if (ret < 0) {
-		free(buf);
+	if (ret < 0)
 		goto out;
-	}
 
 	ret = parse_event_file(pevent, buf, size, sys);
 	if (ret < 0)

[PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files()

[Sanskriti Sharma]

For each system in a given pevent, read_event_files() reads in a
temporary 'sys' string.  Be sure to free this string before moving onto
to the next system and/or leaving read_event_files().

Fixes the following coverity complaints:

  Error: RESOURCE_LEAK (CWE-772):

  tools/perf/util/trace-event-read.c:343: overwrite_var: Overwriting
  "sys" in "sys = read_string()" leaks the storage that "sys" points to.

  tools/perf/util/trace-event-read.c:353: leaked_storage: Variable "sys"
  going out of scope leaks the storage it points to.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
---
 tools/perf/util/trace-event-read.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 6a0d0f2..dd045fd 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -347,9 +347,12 @@ static int read_event_files(struct tep_handle *pevent)
 		for (x=0; x < count; x++) {
 			size = read8(pevent);
 			ret = read_event_file(pevent, sys, size);
-			if (ret)
+			if (ret) {
+				free(sys);
 				return ret;
+			}
 		}
+		free(sys);
 	}
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files()

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 10:29:14AM -0400, Sanskriti Sharma escreveu:
> For each system in a given pevent, read_event_files() reads in a
> temporary 'sys' string.  Be sure to free this string before moving onto
> to the next system and/or leaving read_event_files().
> 
> Fixes the following coverity complaints:
> 
>   Error: RESOURCE_LEAK (CWE-772):
> 
>   tools/perf/util/trace-event-read.c:343: overwrite_var: Overwriting
>   "sys" in "sys = read_string()" leaks the storage that "sys" points to.
> 
>   tools/perf/util/trace-event-read.c:353: leaked_storage: Variable "sys"
>   going out of scope leaks the storage it points to.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
> ---
>  tools/perf/util/trace-event-read.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index 6a0d0f2..dd045fd 100644
> --- a/tools/perf/util/trace-event-read.c
> +++ b/tools/perf/util/trace-event-read.c
> @@ -347,9 +347,12 @@ static int read_event_files(struct tep_handle *pevent)
>  		for (x=0; x < count; x++) {
>  			size = read8(pevent);
>  			ret = read_event_file(pevent, sys, size);
> -			if (ret)
> +			if (ret) {
> +				free(sys);
>  				return ret;
> +			}
>  		}
> +		free(sys);
>  	}
>  	return 0;
>  }
> -- 
> 1.8.3.1

[tip:perf/core] perf tools: Free temporary 'sys' string in read_event_files()

[tip-bot for Sanskriti Sharma]

Commit-ID:  1e44224fb0528b4c0cc176bde2bb31e9127eb14b
Gitweb:     https://git.kernel.org/tip/1e44224fb0528b4c0cc176bde2bb31e9127eb14b
Author:     Sanskriti Sharma <sansharm@redhat.com>
AuthorDate: Tue, 2 Oct 2018 10:29:14 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 14:23:46 -0300

perf tools: Free temporary 'sys' string in read_event_files()

For each system in a given pevent, read_event_files() reads in a
temporary 'sys' string.  Be sure to free this string before moving onto
to the next system and/or leaving read_event_files().

Fixes the following coverity complaints:

  Error: RESOURCE_LEAK (CWE-772):

  tools/perf/util/trace-event-read.c:343: overwrite_var: Overwriting
  "sys" in "sys = read_string()" leaks the storage that "sys" points to.

  tools/perf/util/trace-event-read.c:353: leaked_storage: Variable "sys"
  going out of scope leaks the storage it points to.

Signed-off-by: Sanskriti Sharma <sansharm@redhat.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Link: http://lkml.kernel.org/r/1538490554-8161-6-git-send-email-sansharm@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-read.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index a278e1eee5f5..add8441de579 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -347,9 +347,12 @@ static int read_event_files(struct tep_handle *pevent)
 		for (x=0; x < count; x++) {
 			size = read8(pevent);
 			ret = read_event_file(pevent, sys, size);
-			if (ret)
+			if (ret) {
+				free(sys);
 				return ret;
+			}
 		}
+		free(sys);
 	}
 	return 0;
 }

Re: [PATCH 0/5] perf tool: small coverity clean ups

[Jiri Olsa]

On Tue, Oct 02, 2018 at 10:29:09AM -0400, Sanskriti Sharma wrote:
> This patch set fixes a few coverity static code analyzer complaints. Build 
> tested only.
> 
> Sanskriti Sharma (5):
>   perf strbuf: match va_{add,copy} with va_end
>   perf tools: cleanup trace-event-info 'tdata' leak
>   perf tools: free 'printk' string in parse_ftrace_printk()
>   perf tools: avoid double free in read_event_file()
>   perf tools: free temporary 'sys' string in read_event_files()

nice, thanks for posting those

Reviewed-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
>  tools/perf/util/strbuf.c            | 10 ++++++++--
>  tools/perf/util/trace-event-info.c  |  2 ++
>  tools/perf/util/trace-event-parse.c |  1 +
>  tools/perf/util/trace-event-read.c  |  9 +++++----
>  4 files changed, 16 insertions(+), 6 deletions(-)
> 
> -- 
> 1.8.3.1
> 

Re: [PATCH 0/5] perf tool: small coverity clean ups

[Arnaldo Carvalho de Melo]

Em Tue, Oct 02, 2018 at 06:02:13PM +0200, Jiri Olsa escreveu:
> On Tue, Oct 02, 2018 at 10:29:09AM -0400, Sanskriti Sharma wrote:
> > This patch set fixes a few coverity static code analyzer complaints. Build 
> > tested only.
> > 
> > Sanskriti Sharma (5):
> >   perf strbuf: match va_{add,copy} with va_end
> >   perf tools: cleanup trace-event-info 'tdata' leak
> >   perf tools: free 'printk' string in parse_ftrace_printk()
> >   perf tools: avoid double free in read_event_file()
> >   perf tools: free temporary 'sys' string in read_event_files()
> 
> nice, thanks for posting those
> 
> Reviewed-by: Jiri Olsa <jolsa@kernel.org>

yeah, thanks a lot!

Adding your r-by tag now.

- Arnaldo