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

[sashiko-bot@kernel.org]

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