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

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Jun 05, 2026 at 11:23:17PM +0000, sashiko-bot@kernel.org wrote:
> 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?

Actually this is handled in another series.

Thanks,
Namhyung