[PATCH] perf trace: guard free(new_filter) to avoid double-free on OOM

[PATCH] perf trace: guard free(new_filter) to avoid double-free on OOM

[Gu Bin]

On the asprintf() failure path in trace__expand_filter(), free(new_filter)
is called unconditionally. On the first iteration new_filter aliases
evsel->filter, so this frees the evsel's own filter string; when the
error propagates and the evsel is later deleted, evsel->filter is freed
a second time.

The success path, right below, guards the free with
"if (new_filter != evsel->filter)". Do the same on the error path.

Fixes: 90df0249c2ea ("perf trace: Expand strings in filters to integers")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/perf/builtin-trace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4094141461f91..f4a67491f5be1 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4679,7 +4679,8 @@ static int trace__expand_filter(struct trace *trace, struct evsel *evsel)
 
 					if (asprintf(&n, "%.*s%s%s", expansion_offset, new_filter, expansion, right_end) < 0) {
 						pr_debug(" out of memory!\n");
-						free(new_filter);
+						if (new_filter != evsel->filter)
+							free(new_filter);
 						return -1;
 					}
 					if (new_filter != evsel->filter)
-- 
2.50.1.windows.1

Re: [PATCH] perf trace: guard free(new_filter) to avoid double-free on OOM

[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 trace: unhandled -ENOMEM in trace__expand_filter() [1]
--

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

perf trace: guard free(new_filter) to avoid double-free on OOM

This commit guards free(new_filter) on the asprintf() failure path in
trace__expand_filter() so that evsel->filter is not freed when new_filter
still aliases it.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 4094141461f91..f4a67491f5be1 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -4679,7 +4679,8 @@ static int trace__expand_filter(struct trace *trace, struct evsel *evsel)
>  
>  					if (asprintf(&n, "%.*s%s%s", expansion_offset, new_filter, expansion, right_end) < 0) {
>  						pr_debug(" out of memory!\n");
> -						free(new_filter);
> +						if (new_filter != evsel->filter)
> +							free(new_filter);
>  						return -1;
>  					}
>  					if (new_filter != evsel->filter)

[Severity: Medium]
This problem wasn't introduced by this patch, but at the end of
trace__expand_filter(), should the return value of evsel__set_filter() be
checked [1]?

tools/perf/builtin-trace.c:trace__expand_filter() {
	...
	if (new_filter != evsel->filter) {
		pr_debug("New filter for %s: %s\n", evsel->name, new_filter);
		evsel__set_filter(evsel, new_filter);
		free(new_filter);
	}

	return 0;
}

If strdup() inside evsel__set_filter() fails with -ENOMEM, the failure is
silently ignored and trace__expand_filter() returns 0, leaving the
unexpanded filter in place.

[1] https://sashiko.dev/#/bug/linux-cbf4bb01-a981-47dc-b440-68e5c9c3e81c

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