[PATCH] perf parse-events: free evsel and config terms if term conversion fails

[PATCH] perf parse-events: free evsel and config terms if term conversion fails

[Gu Bin]

add_tracepoint() creates the evsel with evsel__newtp_idx() before
converting the user-supplied config terms with get_config_terms().
If a term allocation fails there, the function returns -ENOMEM with
the evsel not yet on any list, so nobody frees it, and the terms
that get_config_terms() already queued on the local config_terms
list are leaked as well.

Free the partially built config_terms list, then release the evsel
with evsel__put(), before returning.

Fixes: e637d17757a1 ("perf tools: Enable event_config terms to tracepoint events")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/perf/util/parse-events.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f3ba70d4f76b..e4f842532145 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -490,8 +490,11 @@ static int add_tracepoint(struct parse_events_state *parse_state,
 	if (head_config) {
 		LIST_HEAD(config_terms);
 
-		if (get_config_terms(head_config, &config_terms))
+		if (get_config_terms(head_config, &config_terms)) {
+			free_config_terms(&config_terms);
+			evsel__put(evsel);
 			return -ENOMEM;
+		}
 		list_splice(&config_terms, &evsel->config_terms);
 	}
 
-- 
2.50.1.windows.1

Re: [PATCH] perf parse-events: free evsel and config terms if term conversion fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] perf parse-events: memory leak in parse_events_terms__copy()
  [1]
--

commit 4ef9371b87de8bcf35fad7acbbc63f271bc56ae7
Author: Gu Bin <gubin@cmss.chinamobile.com>

perf parse-events: free evsel and config terms if term conversion fails

This commit frees the partially populated config_terms list and releases the
allocated evsel when get_config_terms() fails in add_tracepoint().

> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index f3ba70d4f76b2..e4f8425321451 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -490,8 +490,11 @@ static int add_tracepoint(struct parse_events_state *parse_state,
>  	if (head_config) {
>  		LIST_HEAD(config_terms);
>  
> -		if (get_config_terms(head_config, &config_terms))
> +		if (get_config_terms(head_config, &config_terms)) {
> +			free_config_terms(&config_terms);
> +			evsel__put(evsel);
>  			return -ENOMEM;
> +		}
>  		list_splice(&config_terms, &evsel->config_terms);
>  	}

[Severity: Medium]
This problem wasn't introduced by this patch, but can
parse_events_terms__copy() similarly leak [1] the cloned terms on
dest->terms when parse_events_term__clone() fails partway through the list?

tools/perf/util/parse-events.c:parse_events_terms__copy() {
	list_for_each_entry (term, &src->terms, list) {
		struct parse_events_term *n;
		int ret;

		ret = parse_events_term__clone(&n, term);
		if (ret)
			return ret;

		list_add_tail(&n->list, &dest->terms);
	}
	return 0;
}

When parse_events_add_pmu() or parse_events_multi_pmu_add() calls
parse_events_terms__copy() into a local parsed_terms struct and returns on
error without calling parse_events_terms__exit(), any already cloned
parse_events_term entries and their strings are leaked.

[1] https://sashiko.dev/#/bug/linux-dd6119ed-0ace-40b2-bfe9-492b265c7283

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113085202.2334-1-gubin@cmss.chinamobile.com?part=1