Re: [PATCH perf-tools-next v2 2/4] perf ftrace: Support filtering return address comments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/ftrace: memory leak of trace buffer on error paths in __cmd_ftrace()
--

commit 473e334ba892f3b7d5261168be5e86956a4de438
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Support filtering return address comments

This commit introduces a feature to filter out return address comments from
the function graph tracer output in perf ftrace. It implements parsing and
filtering logic, adds a --filter-retaddr command-line option, and includes
unit tests for validation.

> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -827,6 +827,8 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  				break;
>  			for (int i = 0; i < n; i++) {
>  				if (buf[i] == '\n') {
> +					if (ftrace->filter_retaddr)
> +						ftrace_filter_retaddr(linebuf.buf);
>  					fprintf(stdout, "%s\n", linebuf.buf);
>  					strbuf_setlen(&linebuf, 0);

[Severity: Medium]
Does __cmd_ftrace() in tools/perf/builtin-ftrace.c leak the allocated buf
and linebuf on error paths?

Looking at the initialization sequence, buf is dynamically allocated and
linebuf is initialized:

	buf = malloc(TRACE_BUF_SIZE);
	if (!buf) {
		pr_err("failed to allocate trace buffer\n");
		goto out_reset;
	}
	strbuf_init(&linebuf, 512);

However, if get_tracing_instance_file() fails or trace_fd fails to open,
the code jumps to out_reset:

	if (!trace_file) {
		pr_err("failed to open trace_pipe\n");
		goto out_reset;
	}

The out_reset label is located after the cleanup for buf and linebuf:

out_close_fd:
	free(buf);
	strbuf_release(&linebuf);
	close(trace_fd);
out_reset:
	exit_tracing_instance();

Will this bypass the free(buf) and strbuf_release(&linebuf) calls, causing
a regression where these resources are leaked when early error paths are
taken?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006232756.65620-1-atomlin@atomlin.com?part=2