All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] perf tool: small coverity clean ups
@ 2018-10-02 14:29 Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
@ 2018-10-02 14:29 ` Sanskriti Sharma
  2018-10-02 15:45   ` Arnaldo Carvalho de Melo
  2018-10-09  5:29   ` [tip:perf/core] perf strbuf: Match " tip-bot for Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak Sanskriti Sharma
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
@ 2018-10-02 14:29 ` Sanskriti Sharma
  2018-10-02 15:47   ` Arnaldo Carvalho de Melo
  2018-10-09  5:29   ` [tip:perf/core] perf tools: Cleanup " tip-bot for Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk() Sanskriti Sharma
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk()
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak Sanskriti Sharma
@ 2018-10-02 14:29 ` Sanskriti Sharma
  2018-10-02 15:47   ` Arnaldo Carvalho de Melo
  2018-10-09  5:30   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 4/5] perf tools: avoid double free in read_event_file() Sanskriti Sharma
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] perf tools: avoid double free in read_event_file()
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
                   ` (2 preceding siblings ...)
  2018-10-02 14:29 ` [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk() Sanskriti Sharma
@ 2018-10-02 14:29 ` Sanskriti Sharma
  2018-10-02 15:48   ` Arnaldo Carvalho de Melo
  2018-10-09  5:31   ` [tip:perf/core] perf tools: Avoid " tip-bot for Sanskriti Sharma
  2018-10-02 14:29 ` [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files() Sanskriti Sharma
  2018-10-02 16:02 ` [PATCH 0/5] perf tool: small coverity clean ups Jiri Olsa
  5 siblings, 2 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files()
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
                   ` (3 preceding siblings ...)
  2018-10-02 14:29 ` [PATCH 4/5] perf tools: avoid double free in read_event_file() Sanskriti Sharma
@ 2018-10-02 14:29 ` Sanskriti Sharma
  2018-10-02 15:50   ` Arnaldo Carvalho de Melo
  2018-10-09  5:31   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
  2018-10-02 16:02 ` [PATCH 0/5] perf tool: small coverity clean ups Jiri Olsa
  5 siblings, 2 replies; 18+ messages in thread
From: Sanskriti Sharma @ 2018-10-02 14:29 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Joe Lawrence

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end
  2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
@ 2018-10-02 15:45   ` Arnaldo Carvalho de Melo
  2018-10-09  5:29   ` [tip:perf/core] perf strbuf: Match " tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:45 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Jiri Olsa, Joe Lawrence, acme

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak
  2018-10-02 14:29 ` [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak Sanskriti Sharma
@ 2018-10-02 15:47   ` Arnaldo Carvalho de Melo
  2018-10-09  5:29   ` [tip:perf/core] perf tools: Cleanup " tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:47 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Jiri Olsa, Joe Lawrence, acme

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk()
  2018-10-02 14:29 ` [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk() Sanskriti Sharma
@ 2018-10-02 15:47   ` Arnaldo Carvalho de Melo
  2018-10-09  5:30   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:47 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Jiri Olsa, Joe Lawrence

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] perf tools: avoid double free in read_event_file()
  2018-10-02 14:29 ` [PATCH 4/5] perf tools: avoid double free in read_event_file() Sanskriti Sharma
@ 2018-10-02 15:48   ` Arnaldo Carvalho de Melo
  2018-10-09  5:31   ` [tip:perf/core] perf tools: Avoid " tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:48 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Jiri Olsa, Joe Lawrence

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files()
  2018-10-02 14:29 ` [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files() Sanskriti Sharma
@ 2018-10-02 15:50   ` Arnaldo Carvalho de Melo
  2018-10-09  5:31   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 15:50 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Jiri Olsa, Joe Lawrence

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] perf tool: small coverity clean ups
  2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
                   ` (4 preceding siblings ...)
  2018-10-02 14:29 ` [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files() Sanskriti Sharma
@ 2018-10-02 16:02 ` Jiri Olsa
  2018-10-02 16:14   ` Arnaldo Carvalho de Melo
  5 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2018-10-02 16:02 UTC (permalink / raw)
  To: Sanskriti Sharma; +Cc: linux-kernel, Arnaldo Carvalho de Melo, Joe Lawrence

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] perf tool: small coverity clean ups
  2018-10-02 16:02 ` [PATCH 0/5] perf tool: small coverity clean ups Jiri Olsa
@ 2018-10-02 16:14   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-02 16:14 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Sanskriti Sharma, linux-kernel, Joe Lawrence

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf strbuf: Match va_{add,copy} with va_end
  2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
  2018-10-02 15:45   ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:29   ` tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Sanskriti Sharma @ 2018-10-09  5:29 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux-kernel, acme, joe.lawrence, sansharm, jolsa, hpa, tglx

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);
 }
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf tools: Cleanup trace-event-info 'tdata' leak
  2018-10-02 14:29 ` [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak Sanskriti Sharma
  2018-10-02 15:47   ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:29   ` tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Sanskriti Sharma @ 2018-10-09  5:29 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, tglx, hpa, acme, joe.lawrence, sansharm, linux-kernel, jolsa

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;
 		}
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf tools: Free 'printk' string in parse_ftrace_printk()
  2018-10-02 14:29 ` [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk() Sanskriti Sharma
  2018-10-02 15:47   ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:30   ` tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Sanskriti Sharma @ 2018-10-09  5:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, tglx, jolsa, hpa, acme, mingo, sansharm, joe.lawrence

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);
 	}
 }
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf tools: Avoid double free in read_event_file()
  2018-10-02 14:29 ` [PATCH 4/5] perf tools: avoid double free in read_event_file() Sanskriti Sharma
  2018-10-02 15:48   ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:31   ` tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Sanskriti Sharma @ 2018-10-09  5:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, sansharm, jolsa, joe.lawrence, linux-kernel, hpa, tglx, acme

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)

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf tools: Free temporary 'sys' string in read_event_files()
  2018-10-02 14:29 ` [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files() Sanskriti Sharma
  2018-10-02 15:50   ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:31   ` tip-bot for Sanskriti Sharma
  1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Sanskriti Sharma @ 2018-10-09  5:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, jolsa, joe.lawrence, sansharm, acme, mingo, tglx, hpa

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;
 }

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-10-09  5:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-02 14:29 [PATCH 0/5] perf tool: small coverity clean ups Sanskriti Sharma
2018-10-02 14:29 ` [PATCH 1/5] perf strbuf: match va_{add,copy} with va_end Sanskriti Sharma
2018-10-02 15:45   ` Arnaldo Carvalho de Melo
2018-10-09  5:29   ` [tip:perf/core] perf strbuf: Match " tip-bot for Sanskriti Sharma
2018-10-02 14:29 ` [PATCH 2/5] perf tools: cleanup trace-event-info 'tdata' leak Sanskriti Sharma
2018-10-02 15:47   ` Arnaldo Carvalho de Melo
2018-10-09  5:29   ` [tip:perf/core] perf tools: Cleanup " tip-bot for Sanskriti Sharma
2018-10-02 14:29 ` [PATCH 3/5] perf tools: free 'printk' string in parse_ftrace_printk() Sanskriti Sharma
2018-10-02 15:47   ` Arnaldo Carvalho de Melo
2018-10-09  5:30   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
2018-10-02 14:29 ` [PATCH 4/5] perf tools: avoid double free in read_event_file() Sanskriti Sharma
2018-10-02 15:48   ` Arnaldo Carvalho de Melo
2018-10-09  5:31   ` [tip:perf/core] perf tools: Avoid " tip-bot for Sanskriti Sharma
2018-10-02 14:29 ` [PATCH 5/5] perf tools: free temporary 'sys' string in read_event_files() Sanskriti Sharma
2018-10-02 15:50   ` Arnaldo Carvalho de Melo
2018-10-09  5:31   ` [tip:perf/core] perf tools: Free " tip-bot for Sanskriti Sharma
2018-10-02 16:02 ` [PATCH 0/5] perf tool: small coverity clean ups Jiri Olsa
2018-10-02 16:14   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.