Re: [PATCH 1/4] perf timechart: Release event samples at the end

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double-free of the `backtrace` string in `timechart__release()` due to shared pointers across multiple `cpu_sample` objects.
--

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

perf timechart: Release event samples at the end

This commit adds timechart__release() to clean up data structures created
during sample processing to prevent memory leaks when the perf tool exits.

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 28f33e39895d3..73a08236ac299 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
[ ... ]
> @@ -1509,6 +1509,59 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
>  	svg_close();
>  }
>  
> +static void timechart__release(struct timechart *tchart)
> +{
> +	struct per_pid *p = tchart->all_data;
[ ... ]
> +		while (c) {
> +			struct per_pidcomm *next_comm = c->next;
> +			struct cpu_sample *cs = c->samples;
> +			struct io_sample *ios = c->io_samples;
> +
> +			while (cs) {
> +				struct cpu_sample *next = cs->next;
> +
> +				free((char *)cs->backtrace);
> +				free(cs);
> +				cs = next;
> +			}

[Severity: High]
Will this result in a double-free of the backtrace string?

Looking at sched_switch(), when a context switch occurs between two active
processes, it shares the exact same backtrace pointer across multiple
cpu_sample objects:

if (prev_p->current && prev_p->current->state != TYPE_NONE)
	pid_put_sample(tchart, prev_pid, TYPE_RUNNING, cpu,
		       prev_p->current->state_since, timestamp,
		       backtrace);
if (p && p->current) {
	if (p->current->state != TYPE_NONE)
		pid_put_sample(tchart, next_pid, p->current->state, cpu,
			       p->current->state_since, timestamp,
			       backtrace);

Because timechart__release() iterates over all processes and unconditionally
calls free() on cs->backtrace, won't it attempt to free the same pointer 
multiple times when it cleans up these overlapping samples?

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