Re: [PATCH perf-tools-next v2 1/4] perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: Memory leak of buf and linebuf on early ftrace setup error paths
--

commit e5886f5c4e0cc584a5a7ac05c7c069ab1b80f135
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

This commit refactors __cmd_ftrace() to use a dynamically allocated buffer
and a string buffer for streaming trace data instead of a fixed stack array.

> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 6017493ff1797..4e77a0b2513f8 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c

[ ... ]

> @@ -767,6 +771,13 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  
>  	setup_pager();
>  
> +	buf = malloc(TRACE_BUF_SIZE);
> +	if (!buf) {
> +		pr_err("failed to allocate trace buffer\n");
> +		goto out_reset;
> +	}
> +	strbuf_init(&linebuf, 512);
> +
>  	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");

[Severity: Medium]
If get_tracing_instance_file("trace_pipe") returns NULL, or if the subsequent
open(trace_file, O_RDONLY) fails within __cmd_ftrace() in
tools/perf/builtin-ftrace.c, the error path executes goto out_reset.

Because the out_reset label completely skips the resource teardown label
out_close_fd:

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

Does this permanently leak the heap-allocated buf (TRACE_BUF_SIZE bytes) and
the internal buffer of linebuf on these early ftrace setup error paths?

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