Re: [PATCH 2/4] perf timechart: Fix memory leaks during record

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The dynamically allocated `backtrace` string is assigned to multiple `cpu_sample` structs in `sched_switch()`, leading to a double-free during cleanup in `timechart__release()`.

Pre-existing issues:
- [High] The string dynamically allocated by `cat_backtrace()` is leaked for all events whose handlers do not explicitly take ownership of the pointer.
--

commit 3fc0db6fae9ace5123335636ffe90eb603c38998
Author: Namhyung Kim <namhyung@kernel.org>

perf timechart: Fix memory leaks during record

The command line options can be passed without copy as their lifetime is
limited within the same function.

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 73a08236ac299..fded4be93de0d 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1543,6 +1543,8 @@ static void timechart__release(struct timechart *tchart)
>  			free(c);
>  			c = next_comm;
>  		}
> +
> +		free(p);
>  		p = next_pid;
>  	}

[Severity: High]
Does this code trigger a double-free on the shared backtrace pointers?

In sched_switch(), if both prev_p and p tasks have been previously
observed, the exact same backtrace pointer is passed to pid_put_sample()
twice:

pid_put_sample(tchart, prev_pid, TYPE_RUNNING, cpu,
               prev_p->current->state_since, timestamp, backtrace);
/* ... */
pid_put_sample(tchart, next_pid, p->current->state, cpu,
               p->current->state_since, timestamp, backtrace);

pid_put_sample() stores this pointer in two newly allocated cpu_sample
structs without duplicating it.

Later, when timechart__release() traverses all cpu_sample structs it calls
free((char *)cs->backtrace). Since two samples point to the exact same
memory address, wouldn't this result in a double-free?

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but does process_sample_event() leak memory when calling event
handlers?

process_sample_event() unconditionally calls cat_backtrace(), which
dynamically allocates memory using open_memstream():

FILE *f = open_memstream(&p, &p_len);

This pointer is passed directly to the event's tracepoint_handler callback:

return f(tchart, evsel, sample, cat_backtrace(event, sample, machine));

However, numerous event handlers (e.g., process_sample_cpu_idle,
process_enter_read, process_exit_write) either ignore the backtrace
parameter or omit it entirely.

Since neither the caller nor these handlers free the memory, is it leaked
for every such sample?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605231155.2415760-1-namhyung@kernel.org?part=2