fix some memleaks for parse_events_term__sym_hw and parse_events_term__clone.
v1 ==> v2
1. split into two patches
2. add jump targets common exception handling
3. add Fixes tag
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
From: Cheng Jian <[email protected]>
Fix memory leak of in function parse_events_term__sym_hw()
and parse_events_term__clone() when error occur.
Fixes: b6645a723595 ("perf parse: Ensure config and str in terms are unique")
Signed-off-by: Cheng Jian <[email protected]>
Signed-off-by: Chen Wandun <[email protected]>
---
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
Fix potential memory leak. Function new_term may return error, so
it is need to free memory when the return value is negative.
Fixes: b6645a723595 ("perf parse: Ensure config and str in terms are unique")
Signed-off-by: Chen Wandun <[email protected]>
---
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
> Fix potential memory leak. Function new_term may return error, so
> it is need to free memory when the return value is negative.
How do you think about a wording variant like the following?
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.
Regards,
Markus
> Fix memory leak of in function parse_events_term__sym_hw()
> and parse_events_term__clone() when error occur.
How do you think about a wording variant like the following?
Release a configuration object after a string duplication failed.
Regards,
Markus
On Thu, Jun 11, 2020 at 08:50:58PM +0200, Markus Elfring wrote:
> > Fix memory leak of in function parse_events_term__sym_hw()
> > and parse_events_term__clone() when error occur.
>
> How do you think about a wording variant like the following?
>
> Release a configuration object after a string duplication failed.
>
> Regards,
> Markus
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
在 2020/6/12 2:50, Markus Elfring 写道:
>> Fix memory leak of in function parse_events_term__sym_hw()
>> and parse_events_term__clone() when error occur.
> How do you think about a wording variant like the following?
>
> Release a configuration object after a string duplication failed.
Thank you for your reply, I will update in v3
Best Regards,
Chen Wandun
>
> Regards,
> Markus
>