* [PATCH next v3 0/2] fix potential memleak in perf events parser
@ 2020-06-15 1:36 Chen Wandun
2020-06-15 1:36 ` [PATCH next v3 1/2] perf tools: " Chen Wandun
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chen Wandun @ 2020-06-15 1:36 UTC (permalink / raw)
To: acme, linux-kernel
Cc: peterz, mingo, Markus.Elfring, cj.chengjian, chenwandun
fix some memleaks in parse_events_term__sym_hw and parse_events_term__clone.
ls/perf
v1 ==> v2
1. split into two patches
2. add jump targets common exception handling
3. add Fixes tag
v2 ==> v3
add more commit log.
Chen Wandun (1):
perf tools: fix potential memleak in perf events parser
Cheng Jian (1):
perf tools: fix potential memleak in perf events parser
tools/perf/util/parse-events.c | 51 ++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 8 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH next v3 1/2] perf tools: fix potential memleak in perf events parser
2020-06-15 1:36 [PATCH next v3 0/2] fix potential memleak in perf events parser Chen Wandun
@ 2020-06-15 1:36 ` Chen Wandun
2020-06-15 8:45 ` [PATCH v3 1/2] perf tools: Fix potential memory leaks " Markus Elfring
2020-06-15 1:36 ` [PATCH next v3 2/2] perf tools: fix potential memleak " Chen Wandun
2020-06-15 8:30 ` [PATCH v3 0/2] Fixing " Markus Elfring
2 siblings, 1 reply; 6+ messages in thread
From: Chen Wandun @ 2020-06-15 1:36 UTC (permalink / raw)
To: acme, linux-kernel
Cc: peterz, mingo, Markus.Elfring, cj.chengjian, chenwandun
From: Cheng Jian <cj.chengjian@huawei.com>
Fix memory leak of in function parse_events_term__sym_hw()
and parse_events_term__clone() when string duplication failed.
Fixes: b6645a723595 ("perf parse: Ensure config and str in terms are unique")
Signed-off-by: Cheng Jian <cj.chengjian@huawei.com>
Signed-off-by: Chen Wandun <chenwandun@huawei.com>
---
tools/perf/util/parse-events.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3decbb203846..6f4dc8a92817 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2957,8 +2957,12 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
sym = &event_symbols_hw[idx];
str = strdup(sym->symbol);
- if (!str)
+ if (!str) {
+ if (!config)
+ free(temp.config);
return -ENOMEM;
+ }
+
return new_term(term, &temp, str, 0);
}
@@ -2983,8 +2987,12 @@ int parse_events_term__clone(struct parse_events_term **new,
return new_term(new, &temp, NULL, term->val.num);
str = strdup(term->val.str);
- if (!str)
+ if (!str) {
+ if (term->config)
+ free(temp.config);
return -ENOMEM;
+ }
+
return new_term(new, &temp, str, 0);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH next v3 2/2] perf tools: fix potential memleak in perf events parser
2020-06-15 1:36 [PATCH next v3 0/2] fix potential memleak in perf events parser Chen Wandun
2020-06-15 1:36 ` [PATCH next v3 1/2] perf tools: " Chen Wandun
@ 2020-06-15 1:36 ` Chen Wandun
2020-06-15 8:54 ` [PATCH v3 2/2] perf tools: Fix potential memory leaks " Markus Elfring
2020-06-15 8:30 ` [PATCH v3 0/2] Fixing " Markus Elfring
2 siblings, 1 reply; 6+ messages in thread
From: Chen Wandun @ 2020-06-15 1:36 UTC (permalink / raw)
To: acme, linux-kernel
Cc: peterz, mingo, Markus.Elfring, cj.chengjian, chenwandun
Fix potential memory leak. Function new_term may return error, so
it is need to free memory when the return value is negative.
What's more, add jump targets so that a configuration object and
a duplicated string are released after a call of the function
"strdup" or "new_term" failed.
Fixes: b6645a723595 ("perf parse: Ensure config and str in terms are unique")
Signed-off-by: Chen Wandun <chenwandun@huawei.com>
---
tools/perf/util/parse-events.c | 47 ++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6f4dc8a92817..a9f32032af08 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2947,6 +2947,7 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
.type_term = PARSE_EVENTS__TERM_TYPE_USER,
.config = config,
};
+ int ret;
if (!temp.config) {
temp.config = strdup("event");
@@ -2958,12 +2959,23 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
str = strdup(sym->symbol);
if (!str) {
- if (!config)
- free(temp.config);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto free_config;
}
- return new_term(term, &temp, str, 0);
+ ret = new_term(term, &temp, str, 0);
+ if (ret)
+ goto free_str;
+
+ return 0;
+
+free_str:
+ free(str);
+free_config:
+ if (!config)
+ free(temp.config);
+
+ return ret;
}
int parse_events_term__clone(struct parse_events_term **new,
@@ -2977,23 +2989,38 @@ int parse_events_term__clone(struct parse_events_term **new,
.err_term = term->err_term,
.err_val = term->err_val,
};
+ int ret;
if (term->config) {
temp.config = strdup(term->config);
if (!temp.config)
return -ENOMEM;
}
- if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
- return new_term(new, &temp, NULL, term->val.num);
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
+ ret = new_term(new, &temp, NULL, term->val.num);
+ if (ret)
+ goto free_config;
+ }
str = strdup(term->val.str);
if (!str) {
- if (term->config)
- free(temp.config);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto free_config;
}
- return new_term(new, &temp, str, 0);
+ ret = new_term(new, &temp, str, 0);
+ if (ret)
+ goto free_str;
+
+ return 0;
+
+free_str:
+ free(str);
+free_config:
+ if (term->config)
+ free(temp.config);
+
+ return ret;
}
void parse_events_term__delete(struct parse_events_term *term)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] Fixing memory leaks in perf events parser
2020-06-15 1:36 [PATCH next v3 0/2] fix potential memleak in perf events parser Chen Wandun
2020-06-15 1:36 ` [PATCH next v3 1/2] perf tools: " Chen Wandun
2020-06-15 1:36 ` [PATCH next v3 2/2] perf tools: fix potential memleak " Chen Wandun
@ 2020-06-15 8:30 ` Markus Elfring
2 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-06-15 8:30 UTC (permalink / raw)
To: Chen Wandun, Cheng Jian, netdev, bpf
Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra
> fix some memleaks in parse_events_term__sym_hw and parse_events_term__clone.
Can it be more appropriate to refer to the term “memory leak” in consistent ways?
> v1 ==> v2
> 1. split into two patches
Corresponding development consequences can become more interesting.
> v2 ==> v3
> add more commit log.
>
> Chen Wandun (1):
> perf tools: fix potential memleak in perf events parser
>
> Cheng Jian (1):
> perf tools: fix potential memleak in perf events parser
>
> tools/perf/util/parse-events.c | 51 ++++++++++++++++++++++++++++------
Are there any chances to make the change distinction a bit easier
by adjusting such commit subjects?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] perf tools: Fix potential memory leaks in perf events parser
2020-06-15 1:36 ` [PATCH next v3 1/2] perf tools: " Chen Wandun
@ 2020-06-15 8:45 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-06-15 8:45 UTC (permalink / raw)
To: Cheng Jian, Chen Wandun, netdev, bpf
Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra
> Fix memory leak of in function parse_events_term__sym_hw()
> and parse_events_term__clone() when string duplication failed.
Can a wording like “Fix memory leaks in …” be more appropriate for
the final commit message?
Would you find any other description variant more pleasing?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] perf tools: Fix potential memory leaks in perf events parser
2020-06-15 1:36 ` [PATCH next v3 2/2] perf tools: fix potential memleak " Chen Wandun
@ 2020-06-15 8:54 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-06-15 8:54 UTC (permalink / raw)
To: Chen Wandun, Cheng Jian, netdev, bpf
Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra
> Fix potential memory leak. Function new_term may return error, so
> it is need to free memory when the return value is negative.
I hope that a typo will be avoided for the final commit message.
Would you find any other description variant more pleasing?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-15 8:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-15 1:36 [PATCH next v3 0/2] fix potential memleak in perf events parser Chen Wandun
2020-06-15 1:36 ` [PATCH next v3 1/2] perf tools: " Chen Wandun
2020-06-15 8:45 ` [PATCH v3 1/2] perf tools: Fix potential memory leaks " Markus Elfring
2020-06-15 1:36 ` [PATCH next v3 2/2] perf tools: fix potential memleak " Chen Wandun
2020-06-15 8:54 ` [PATCH v3 2/2] perf tools: Fix potential memory leaks " Markus Elfring
2020-06-15 8:30 ` [PATCH v3 0/2] Fixing " Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).