* [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* 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* [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
* [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* 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* [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
* [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* 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
* [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
* [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* 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* [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
* [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 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* [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
* 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