[PATCH 0/4] perf timechart: Fix memory leaks

[PATCH 0/4] perf timechart: Fix memory leaks

[Namhyung Kim]

Hello,

This patchset fixes memory leaks in the perf timechart code.
I've extended and verified the timechart test with ASAN.

Thanks,
Namhyung


Namhyung Kim (4):
  perf timechart: Release event samples at the end
  perf timechart: Fix memory leaks during record
  perf timechart: Fix memory leaks in draw_wakeups()
  perf test: Update perf timechart test

 tools/perf/builtin-timechart.c      | 93 ++++++++++++++++++++++++-----
 tools/perf/tests/shell/timechart.sh | 24 +++++---
 2 files changed, 94 insertions(+), 23 deletions(-)

-- 
2.54.0.1032.g2f8565e1d1-goog

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

[Namhyung Kim]

Add timechart__release() to free all data structures added during the
sample processing.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 04dbb944a42720b4..7c4ace51302e260c 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1548,6 +1548,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;
+	struct power_event *pwr = tchart->power_events;
+	struct wake_event *we = tchart->wake_events;
+
+	while (p) {
+		struct per_pid *next_pid = p->next;
+		struct per_pidcomm *c = p->all;
+
+		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;
+			}
+
+			while (ios) {
+				struct io_sample *next = ios->next;
+
+				free(ios);
+				ios = next;
+			}
+
+			free(c->comm);
+			free(c);
+			c = next_comm;
+		}
+		p = next_pid;
+	}
+
+	while (pwr) {
+		struct power_event *next = pwr->next;
+
+		free(pwr);
+		pwr = next;
+	}
+
+	while (we) {
+		struct wake_event *next = we->next;
+
+		free((char *)we->backtrace);
+		free(we);
+		we = next;
+	}
+}
+
 static int process_header(struct perf_file_section *section __maybe_unused,
 			  struct perf_header *ph,
 			  int feat,
@@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
 
 	ret = __cmd_timechart(&tchart, output_name);
 out:
+	timechart__release(&tchart);
 	zfree(&cpus_cstate_start_times);
 	zfree(&cpus_cstate_state);
 	zfree(&cpus_pstate_start_times);
-- 
2.54.0.1032.g2f8565e1d1-goog

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

[Namhyung Kim]

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

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 7c4ace51302e260c..d56adb41dcf9bb69 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1582,6 +1582,8 @@ static void timechart__release(struct timechart *tchart)
 			free(c);
 			c = next_comm;
 		}
+
+		free(p);
 		p = next_pid;
 	}
 
@@ -1820,6 +1822,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 		"syscalls:sys_exit_select",
 	};
 	unsigned int poll_events_nr = ARRAY_SIZE(poll_events);
+	int ret;
 
 	rec_argc = common_args_nr +
 		disk_events_nr * 4 +
@@ -1838,7 +1841,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 
 	p = rec_argv;
 	for (i = 0; i < common_args_nr; i++)
-		*p++ = strdup(common_args[i]);
+		*p++ = common_args[i];
 
 	for (i = 0; i < disk_events_nr; i++) {
 		if (!is_valid_tracepoint(disk_events[i])) {
@@ -1847,7 +1850,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 		}
 
 		*p++ = "-e";
-		*p++ = strdup(disk_events[i]);
+		*p++ = disk_events[i];
 		*p++ = "--filter";
 		*p++ = filter;
 	}
@@ -1858,7 +1861,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 		}
 
 		*p++ = "-e";
-		*p++ = strdup(net_events[i]);
+		*p++ = net_events[i];
 		*p++ = "--filter";
 		*p++ = filter;
 	}
@@ -1869,7 +1872,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 		}
 
 		*p++ = "-e";
-		*p++ = strdup(poll_events[i]);
+		*p++ = poll_events[i];
 		*p++ = "--filter";
 		*p++ = filter;
 	}
@@ -1877,7 +1880,11 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
 	for (i = 0; i < (unsigned int)argc; i++)
 		*p++ = argv[i];
 
-	return cmd_record(rec_argc, rec_argv);
+	ret = cmd_record(rec_argc, rec_argv);
+
+	free(rec_argv);
+	free(filter);
+	return ret;
 }
 
 
@@ -1888,6 +1895,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
 	const char **rec_argv;
 	const char **p;
 	unsigned int record_elems;
+	int ret;
 
 	const char * const common_args[] = {
 		"record", "-a", "-R", "-c", "1", "-o", output_data,
@@ -1952,24 +1960,27 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
 
 	p = rec_argv;
 	for (i = 0; i < common_args_nr; i++)
-		*p++ = strdup(common_args[i]);
+		*p++ = common_args[i];
 
 	for (i = 0; i < backtrace_args_no; i++)
-		*p++ = strdup(backtrace_args[i]);
+		*p++ = backtrace_args[i];
 
 	for (i = 0; i < tasks_args_nr; i++)
-		*p++ = strdup(tasks_args[i]);
+		*p++ = tasks_args[i];
 
 	for (i = 0; i < power_args_nr; i++)
-		*p++ = strdup(power_args[i]);
+		*p++ = power_args[i];
 
 	for (i = 0; i < old_power_args_nr; i++)
-		*p++ = strdup(old_power_args[i]);
+		*p++ = old_power_args[i];
 
 	for (j = 0; j < (unsigned int)argc; j++)
 		*p++ = argv[j];
 
-	return cmd_record(rec_argc, rec_argv);
+	ret = cmd_record(rec_argc, rec_argv);
+
+	free(rec_argv);
+	return ret;
 }
 
 static int
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups()

[Namhyung Kim]

The second loop for per_pidcommd is meaningful only when it doesn't have
from and to tasks.  Also make sure c->Y is set before copying the comm
string otherwise it will be overwritten by next one.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index d56adb41dcf9bb69..e8d01c8723959e80 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1110,12 +1110,12 @@ static void draw_wakeups(struct timechart *tchart)
 					c = c->next;
 				}
 				c = p->all;
-				while (c) {
-					if (p->pid == we->waker && !from) {
+				while (c && !from && !to) {
+					if (c->Y && p->pid == we->waker && !from) {
 						from = c->Y;
 						task_from = strdup(c->comm);
 					}
-					if (p->pid == we->wakee && !to) {
+					if (c->Y && p->pid == we->wakee && !to) {
 						to = c->Y;
 						task_to = strdup(c->comm);
 					}
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH 4/4] perf test: Update perf timechart test

[Namhyung Kim]

To include IO-only and backtrace modes to test different code paths.

  $ sudo perf test -vv timechart
  135: perf timechart tests                        : Running
  135: perf timechart tests:
  ---- start ----
  test child forked, pid 2413665
  perf timechart Basic test
  perf timechart Basic test [Success]
  perf timechart IO-only test
  perf timechart IO-only test [Success]
  perf timechart Backtrace test
  perf timechart Backtrace test [Success]
  ---- end(0) ----
  135: perf timechart tests                        : Ok

  === Test Summary ===
  Passed main tests : 1
  Passed subtests   : 0
  Skipped tests     : 0
  Failed tests      : 0

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/timechart.sh | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/perf/tests/shell/timechart.sh b/tools/perf/tests/shell/timechart.sh
index b14b3472c2846b32..40072b32171a9a1c 100755
--- a/tools/perf/tests/shell/timechart.sh
+++ b/tools/perf/tests/shell/timechart.sh
@@ -9,7 +9,7 @@ perfdata=$(mktemp /tmp/__perf_timechart_test.perf.data.XXXXX)
 output=$(mktemp /tmp/__perf_timechart_test.output.XXXXX.svg)
 
 cleanup() {
-	rm -f "${perfdata}"
+	rm -f "${perfdata}"*
 	rm -f "${output}"
 	trap - EXIT TERM INT
 }
@@ -22,38 +22,41 @@ trap_cleanup() {
 trap trap_cleanup EXIT TERM INT
 
 test_timechart() {
-	echo "Basic perf timechart test"
+	NAME=$1
+	OPTION=$2
+
+	echo "perf timechart ${NAME} test"
 
 	# Try to record timechart data.
 	# perf timechart record uses system-wide recording and specific tracepoints.
 	# If it fails (e.g. permissions, missing tracepoints), skip the test.
-	if ! perf timechart record -o "${perfdata}" true > /dev/null 2>&1; then
-		echo "Basic perf timechart test [Skipped: perf timechart record failed (permissions/events?)]"
+	if ! perf timechart record -o "${perfdata}" ${OPTION} true > /dev/null 2>&1; then
+		echo "perf timechart ${NAME} test [Skipped: perf timechart record failed (permissions/events?)]"
 		return
 	fi
 
 	# Generate the timechart
 	if ! perf timechart -i "${perfdata}" -o "${output}" > /dev/null 2>&1; then
-		echo "Basic perf timechart test [Failed: perf timechart command failed]"
+		echo "perf timechart ${NAME} test [Failed: perf timechart command failed]"
 		err=1
 		return
 	fi
 
 	# Check if output file exists and is not empty
 	if [ ! -s "${output}" ]; then
-		echo "Basic perf timechart test [Failed: output file is empty or missing]"
+		echo "perf timechart ${NAME} test [Failed: output file is empty or missing]"
 		err=1
 		return
 	fi
 
 	# Check if it looks like an SVG
 	if ! grep -q "svg" "${output}"; then
-		echo "Basic perf timechart test [Failed: output doesn't look like SVG]"
+		echo "perf timechart ${NAME} test [Failed: output doesn't look like SVG]"
 		err=1
 		return
 	fi
 
-	echo "Basic perf timechart test [Success]"
+	echo "perf timechart ${NAME} test [Success]"
 }
 
 if ! perf check feature -q libtraceevent ; then
@@ -62,6 +65,9 @@ if ! perf check feature -q libtraceevent ; then
 	exit 2
 fi
 
-test_timechart
+test_timechart "Basic" ""
+test_timechart "IO-only" "-I"
+test_timechart "Backtrace" "-g"
+
 cleanup
 exit $err
-- 
2.54.0.1032.g2f8565e1d1-goog

Re: [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The modified fallback loop condition `while (c && !from && !to)` prematurely aborts the search for task coordinates if either `from` or `to` has already been found.
--

commit 757fad46c9b9ecd50deaba0cc7cc1ec56200ab8e
Author: Namhyung Kim <namhyung@kernel.org>

perf timechart: Fix memory leaks in draw_wakeups()

This commit modifies the fallback coordinate search in draw_wakeups()
to only run when it doesn't have from and to tasks, and ensures c->Y is
set before copying the comm string to fix memory leaks.

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index fded4be93de0d..9ac4662ef16e9 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c

[ ... ]

> @@ -1071,12 +1071,12 @@ static void draw_wakeups(struct timechart *tchart)
>  					c = c->next;
>  				}
>  				c = p->all;
> -				while (c) {
> -					if (p->pid == we->waker && !from) {
> +				while (c && !from && !to) {

[Severity: Medium]
Will this condition prematurely abort the search for the second task's
coordinates?

Because the waker and wakee are typically two different PIDs, they are
processed in different iterations of the outer per_pid loop. When the first
one is found, either from or to becomes non-zero.

When the outer loop reaches the second PID, the condition !from && !to
evaluates to false immediately, which skips the fallback coordinate search
entirely for that second task.

Should this condition instead be !from || !to so the loop continues as long
as either coordinate is still missing?

> +					if (c->Y && p->pid == we->waker && !from) {
>  						from = c->Y;
>  						task_from = strdup(c->comm);
>  					}
> -					if (p->pid == we->wakee && !to) {
> +					if (c->Y && p->pid == we->wakee && !to) {
>  						to = c->Y;
>  						task_to = strdup(c->comm);
>  					}

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

Re: [PATCH 0/4] perf timechart: Fix memory leaks

[Arnaldo Carvalho de Melo]

On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> Hello,
> 
> This patchset fixes memory leaks in the perf timechart code.
> I've extended and verified the timechart test with ASAN.
 
Hi Namhyung,

	You submitted a series for timechart before, so I had to go on
looking if this was a second version or a different series.

	I suggest you always have a version in the cover letter, i.e.:

[PATCH v1 0/4] perf timechart: Fix memory leaks

	To avoid this brief distraction :-)

Regards,

- Arnaldo

> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (4):
>   perf timechart: Release event samples at the end
>   perf timechart: Fix memory leaks during record
>   perf timechart: Fix memory leaks in draw_wakeups()
>   perf test: Update perf timechart test
> 
>  tools/perf/builtin-timechart.c      | 93 ++++++++++++++++++++++++-----
>  tools/perf/tests/shell/timechart.sh | 24 +++++---
>  2 files changed, 94 insertions(+), 23 deletions(-)
> 
> -- 
> 2.54.0.1032.g2f8565e1d1-goog

Re: [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups()

[Namhyung Kim]

On Fri, Jun 05, 2026 at 11:17:33PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The modified fallback loop condition `while (c && !from && !to)` prematurely aborts the search for task coordinates if either `from` or `to` has already been found.
> --
> 
> commit 757fad46c9b9ecd50deaba0cc7cc1ec56200ab8e
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf timechart: Fix memory leaks in draw_wakeups()
> 
> This commit modifies the fallback coordinate search in draw_wakeups()
> to only run when it doesn't have from and to tasks, and ensures c->Y is
> set before copying the comm string to fix memory leaks.
> 
> > diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> > index fded4be93de0d..9ac4662ef16e9 100644
> > --- a/tools/perf/builtin-timechart.c
> > +++ b/tools/perf/builtin-timechart.c
> 
> [ ... ]
> 
> > @@ -1071,12 +1071,12 @@ static void draw_wakeups(struct timechart *tchart)
> >  					c = c->next;
> >  				}
> >  				c = p->all;
> > -				while (c) {
> > -					if (p->pid == we->waker && !from) {
> > +				while (c && !from && !to) {
> 
> [Severity: Medium]
> Will this condition prematurely abort the search for the second task's
> coordinates?
> 
> Because the waker and wakee are typically two different PIDs, they are
> processed in different iterations of the outer per_pid loop. When the first
> one is found, either from or to becomes non-zero.
> 
> When the outer loop reaches the second PID, the condition !from && !to
> evaluates to false immediately, which skips the fallback coordinate search
> entirely for that second task.
> 
> Should this condition instead be !from || !to so the loop continues as long
> as either coordinate is still missing?

Oops, right.  I meant it.  Will send v2 soon.

Thanks,
Namhyung

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

[sashiko-bot]

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

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

[Arnaldo Carvalho de Melo]

On Fri, Jun 05, 2026 at 04:11:52PM -0700, Namhyung Kim wrote:
> Add timechart__release() to free all data structures added during the
> sample processing.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 04dbb944a42720b4..7c4ace51302e260c 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1548,6 +1548,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;
> +	struct power_event *pwr = tchart->power_events;
> +	struct wake_event *we = tchart->wake_events;
> +
> +	while (p) {
> +		struct per_pid *next_pid = p->next;
> +		struct per_pidcomm *c = p->all;
> +
> +		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);

				zfree()? This way if somebody has a
				pointer to cs and tries to use
				cs->backtrace after this free, it will
				segfault instead of using garbage or
				worse, setting some other unrelated
				struct to some cpu_sample field supposed
				value.

> +				free(cs);
> +				cs = next;
> +			}
> +
> +			while (ios) {
> +				struct io_sample *next = ios->next;
> +
> +				free(ios);

				io->next = NULL;

> +				ios = next;
> +			}
> +
> +			free(c->comm);

			zfree

> +			free(c);
> +			c = next_comm;
> +		}
> +		p = next_pid;
> +	}
> +
> +	while (pwr) {
> +		struct power_event *next = pwr->next;
> +
> +		free(pwr);
		pwr->next = NULL;
> +		pwr = next;
> +	}
> +
> +	while (we) {
> +		struct wake_event *next = we->next;
> +
> +		free((char *)we->backtrace);
		zfree
> +		free(we);
> +		we = next;
> +	}
> +}
> +
>  static int process_header(struct perf_file_section *section __maybe_unused,
>  			  struct perf_header *ph,
>  			  int feat,
> @@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
>  
>  	ret = __cmd_timechart(&tchart, output_name);
>  out:
> +	timechart__release(&tchart);
>  	zfree(&cpus_cstate_start_times);
>  	zfree(&cpus_cstate_state);
>  	zfree(&cpus_pstate_start_times);
> -- 
> 2.54.0.1032.g2f8565e1d1-goog

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

[Arnaldo Carvalho de Melo]

On Fri, Jun 05, 2026 at 04:11:53PM -0700, Namhyung Kim wrote:
> The command line options can be passed without copy as their lifetime is
> limited within the same function.

From a quick look:

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-timechart.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 7c4ace51302e260c..d56adb41dcf9bb69 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1582,6 +1582,8 @@ static void timechart__release(struct timechart *tchart)
>  			free(c);
>  			c = next_comm;
>  		}
> +
> +		free(p);
>  		p = next_pid;
>  	}
>  
> @@ -1820,6 +1822,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  		"syscalls:sys_exit_select",
>  	};
>  	unsigned int poll_events_nr = ARRAY_SIZE(poll_events);
> +	int ret;
>  
>  	rec_argc = common_args_nr +
>  		disk_events_nr * 4 +
> @@ -1838,7 +1841,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  
>  	p = rec_argv;
>  	for (i = 0; i < common_args_nr; i++)
> -		*p++ = strdup(common_args[i]);
> +		*p++ = common_args[i];
>  
>  	for (i = 0; i < disk_events_nr; i++) {
>  		if (!is_valid_tracepoint(disk_events[i])) {
> @@ -1847,7 +1850,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  		}
>  
>  		*p++ = "-e";
> -		*p++ = strdup(disk_events[i]);
> +		*p++ = disk_events[i];
>  		*p++ = "--filter";
>  		*p++ = filter;
>  	}
> @@ -1858,7 +1861,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  		}
>  
>  		*p++ = "-e";
> -		*p++ = strdup(net_events[i]);
> +		*p++ = net_events[i];
>  		*p++ = "--filter";
>  		*p++ = filter;
>  	}
> @@ -1869,7 +1872,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  		}
>  
>  		*p++ = "-e";
> -		*p++ = strdup(poll_events[i]);
> +		*p++ = poll_events[i];
>  		*p++ = "--filter";
>  		*p++ = filter;
>  	}
> @@ -1877,7 +1880,11 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>  	for (i = 0; i < (unsigned int)argc; i++)
>  		*p++ = argv[i];
>  
> -	return cmd_record(rec_argc, rec_argv);
> +	ret = cmd_record(rec_argc, rec_argv);
> +
> +	free(rec_argv);
> +	free(filter);
> +	return ret;
>  }
>  
>  
> @@ -1888,6 +1895,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
>  	const char **rec_argv;
>  	const char **p;
>  	unsigned int record_elems;
> +	int ret;
>  
>  	const char * const common_args[] = {
>  		"record", "-a", "-R", "-c", "1", "-o", output_data,
> @@ -1952,24 +1960,27 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
>  
>  	p = rec_argv;
>  	for (i = 0; i < common_args_nr; i++)
> -		*p++ = strdup(common_args[i]);
> +		*p++ = common_args[i];
>  
>  	for (i = 0; i < backtrace_args_no; i++)
> -		*p++ = strdup(backtrace_args[i]);
> +		*p++ = backtrace_args[i];
>  
>  	for (i = 0; i < tasks_args_nr; i++)
> -		*p++ = strdup(tasks_args[i]);
> +		*p++ = tasks_args[i];
>  
>  	for (i = 0; i < power_args_nr; i++)
> -		*p++ = strdup(power_args[i]);
> +		*p++ = power_args[i];
>  
>  	for (i = 0; i < old_power_args_nr; i++)
> -		*p++ = strdup(old_power_args[i]);
> +		*p++ = old_power_args[i];
>  
>  	for (j = 0; j < (unsigned int)argc; j++)
>  		*p++ = argv[j];
>  
> -	return cmd_record(rec_argc, rec_argv);
> +	ret = cmd_record(rec_argc, rec_argv);
> +
> +	free(rec_argv);
> +	return ret;
>  }
>  
>  static int
> -- 
> 2.54.0.1032.g2f8565e1d1-goog

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

[sashiko-bot]

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

Re: [PATCH 0/4] perf timechart: Fix memory leaks

[Namhyung Kim]

On Fri, Jun 05, 2026 at 08:19:13PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> > Hello,
> > 
> > This patchset fixes memory leaks in the perf timechart code.
> > I've extended and verified the timechart test with ASAN.
>  
> Hi Namhyung,
> 
> 	You submitted a series for timechart before, so I had to go on
> looking if this was a second version or a different series.

Right, this is a different series.  I believe they are independent.

> 
> 	I suggest you always have a version in the cover letter, i.e.:
> 
> [PATCH v1 0/4] perf timechart: Fix memory leaks
> 
> 	To avoid this brief distraction :-)

Ok, I'll do that if that helps. :)

Thanks,
Namhyung

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

[Namhyung Kim]

On Fri, Jun 05, 2026 at 08:21:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jun 05, 2026 at 04:11:52PM -0700, Namhyung Kim wrote:
> > Add timechart__release() to free all data structures added during the
> > sample processing.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> > index 04dbb944a42720b4..7c4ace51302e260c 100644
> > --- a/tools/perf/builtin-timechart.c
> > +++ b/tools/perf/builtin-timechart.c
> > @@ -1548,6 +1548,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;
> > +	struct power_event *pwr = tchart->power_events;
> > +	struct wake_event *we = tchart->wake_events;
> > +
> > +	while (p) {
> > +		struct per_pid *next_pid = p->next;
> > +		struct per_pidcomm *c = p->all;
> > +
> > +		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);
> 
> 				zfree()? This way if somebody has a
> 				pointer to cs and tries to use
> 				cs->backtrace after this free, it will
> 				segfault instead of using garbage or
> 				worse, setting some other unrelated
> 				struct to some cpu_sample field supposed
> 				value.

Ok, will do here and others below.

Thanks,
Namhyung

> 
> > +				free(cs);
> > +				cs = next;
> > +			}
> > +
> > +			while (ios) {
> > +				struct io_sample *next = ios->next;
> > +
> > +				free(ios);
> 
> 				io->next = NULL;
> 
> > +				ios = next;
> > +			}
> > +
> > +			free(c->comm);
> 
> 			zfree
> 
> > +			free(c);
> > +			c = next_comm;
> > +		}
> > +		p = next_pid;
> > +	}
> > +
> > +	while (pwr) {
> > +		struct power_event *next = pwr->next;
> > +
> > +		free(pwr);
> 		pwr->next = NULL;
> > +		pwr = next;
> > +	}
> > +
> > +	while (we) {
> > +		struct wake_event *next = we->next;
> > +
> > +		free((char *)we->backtrace);
> 		zfree
> > +		free(we);
> > +		we = next;
> > +	}
> > +}
> > +
> >  static int process_header(struct perf_file_section *section __maybe_unused,
> >  			  struct perf_header *ph,
> >  			  int feat,
> > @@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
> >  
> >  	ret = __cmd_timechart(&tchart, output_name);
> >  out:
> > +	timechart__release(&tchart);
> >  	zfree(&cpus_cstate_start_times);
> >  	zfree(&cpus_cstate_state);
> >  	zfree(&cpus_pstate_start_times);
> > -- 
> > 2.54.0.1032.g2f8565e1d1-goog

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

[Namhyung Kim]

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

Re: [PATCH 0/4] perf timechart: Fix memory leaks

[Arnaldo Carvalho de Melo]

On Fri, Jun 05, 2026 at 04:28:13PM -0700, Namhyung Kim wrote:
> On Fri, Jun 05, 2026 at 08:19:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> > > This patchset fixes memory leaks in the perf timechart code.
> > > I've extended and verified the timechart test with ASAN.

> > 	You submitted a series for timechart before, so I had to go on
> > looking if this was a second version or a different series.
 
> Right, this is a different series.  I believe they are independent.

Thanks for clarifying.
 
> > 	I suggest you always have a version in the cover letter, i.e.:

> > [PATCH v1 0/4] perf timechart: Fix memory leaks

> > 	To avoid this brief distraction :-)
 
> Ok, I'll do that if that helps. :)

It is a small detail, not having anything it may have been that the
submitter forgot to add something or that it is the first entry, having
v1 removes this ambiguity.

Thanks,

- Arnaldo