All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] perf sched stats: Fix memory leaks
@ 2026-07-13 20:47 Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 1/3] perf sched: Add missing perf_session__delete() Namhyung Kim
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-13 20:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

Hello,

This is a small series to fix memory leaks in the perf sched stats.
I've tested the changes with the address sanitizer.

Thanks,
Namhyung


Namhyung Kim (3):
  perf sched: Add missing perf_session__delete()
  perf sched: Fix memory leaks in perf sched stats report
  perf sched: Free subcommand string after perf sched stats

 tools/perf/builtin-sched.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
2.55.0.795.g602f6c329a-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1 1/3] perf sched: Add missing perf_session__delete()
  2026-07-13 20:47 [PATCH v1 0/3] perf sched stats: Fix memory leaks Namhyung Kim
@ 2026-07-13 20:47 ` Namhyung Kim
  2026-07-13 20:59   ` sashiko-bot
  2026-07-13 20:47 ` [PATCH v1 2/3] perf sched: Fix memory leaks in perf sched stats report Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 3/3] perf sched: Free subcommand string after perf sched stats Namhyung Kim
  2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2026-07-13 20:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, Swapnil Sapkal

The perf sched stats record missed to release the session and ASAN
reported a leak.

Fixes: c3030995f23b ("perf sched stats: Add record and rawdump support")
Cc: Swapnil Sapkal <swapnil.sapkal@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-sched.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 6e0d8f4d270e2d88..68147a0be5c710ed 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -4027,6 +4027,7 @@ static int perf_sched__schedstat_record(struct perf_sched *sched,
 	else
 		fprintf(stderr, "[ perf sched stats: Failed !! ]\n");
 
+	perf_session__delete(session);
 	evlist__put(evlist);
 	close(fd);
 	return err;
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 2/3] perf sched: Fix memory leaks in perf sched stats report
  2026-07-13 20:47 [PATCH v1 0/3] perf sched stats: Fix memory leaks Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 1/3] perf sched: Add missing perf_session__delete() Namhyung Kim
@ 2026-07-13 20:47 ` Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 3/3] perf sched: Free subcommand string after perf sched stats Namhyung Kim
  2 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-13 20:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, Swapnil Sapkal

The second pass data is not saved in the list and only used to calculate
delta from the first pass.  Let's free the data after use.

Fixes: 5a357ae6ad63 ("perf sched stats: Add support for report subcommand")
Cc: Swapnil Sapkal <swapnil.sapkal@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-sched.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 68147a0be5c710ed..439e50d9a936f6c1 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -4632,6 +4632,8 @@ static int perf_sched__process_schedstat(const struct perf_tool *tool __maybe_un
 			domain_second_pass = list_first_entry(&cpu_second_pass->domain_head,
 							      struct schedstat_domain, domain_list);
 			store_schedstat_cpu_diff(temp);
+			free(temp->cpu_data);
+			free(temp);
 		}
 	} else if (event->header.type == PERF_RECORD_SCHEDSTAT_DOMAIN) {
 		struct schedstat_cpu *cpu_tail;
@@ -4652,6 +4654,8 @@ static int perf_sched__process_schedstat(const struct perf_tool *tool __maybe_un
 		} else {
 			store_schedstat_domain_diff(temp);
 			domain_second_pass = list_next_entry(domain_second_pass, domain_list);
+			free(temp->domain_data);
+			free(temp);
 		}
 	}
 
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 3/3] perf sched: Free subcommand string after perf sched stats
  2026-07-13 20:47 [PATCH v1 0/3] perf sched stats: Fix memory leaks Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 1/3] perf sched: Add missing perf_session__delete() Namhyung Kim
  2026-07-13 20:47 ` [PATCH v1 2/3] perf sched: Fix memory leaks in perf sched stats report Namhyung Kim
@ 2026-07-13 20:47 ` Namhyung Kim
  2 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-13 20:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, Swapnil Sapkal

The first entry of the sched_usage is dynamically allocated in
parse_options_subcommand() so it should be released at the end.
Do not return from a subcommand directly.

Fixes: 064790a3d4a8 ("perf sched stats: Add support for diff subcommand")
Cc: Swapnil Sapkal <swapnil.sapkal@amd.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-sched.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 439e50d9a936f6c1..cc5b61cb9291fa02 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -5260,19 +5260,20 @@ int cmd_sched(int argc, const char **argv)
 			if (argc)
 				argc = parse_options(argc, argv, stats_options,
 						     stats_usage, 0);
-			return perf_sched__schedstat_record(&sched, argc, argv);
+			ret = perf_sched__schedstat_record(&sched, argc, argv);
 		} else if (argv[0] && !strcmp(argv[0], "report")) {
 			if (argc)
 				argc = parse_options(argc, argv, stats_options,
 						     stats_usage, 0);
-			return perf_sched__schedstat_report(&sched);
+			ret = perf_sched__schedstat_report(&sched);
 		} else if (argv[0] && !strcmp(argv[0], "diff")) {
 			if (argc)
 				argc = parse_options(argc, argv, stats_options,
 						     stats_usage, 0);
-			return perf_sched__schedstat_diff(&sched, argc, argv);
+			ret = perf_sched__schedstat_diff(&sched, argc, argv);
+		} else {
+			ret = perf_sched__schedstat_live(&sched, argc, argv);
 		}
-		return perf_sched__schedstat_live(&sched, argc, argv);
 	} else {
 		usage_with_options(sched_usage, sched_options);
 	}
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] perf sched: Add missing perf_session__delete()
  2026-07-13 20:47 ` [PATCH v1 1/3] perf sched: Add missing perf_session__delete() Namhyung Kim
@ 2026-07-13 20:59   ` sashiko-bot
  2026-07-13 23:00     ` Namhyung Kim
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-07-13 20:59 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a double close of the file descriptor `fd` because `perf_session__delete(session)` inherently closes it before the explicit `close(fd)`.
--

--- Patch [1]: [PATCH v1 1/3] perf sched: Add missing perf_session__delete() ---
commit 2da9bd6a7ce8e17e442d8be81b1b57837aaee6e1
Author: Namhyung Kim <namhyung@kernel.org>

perf sched: Add missing perf_session__delete()

This patch addresses an ASAN-reported memory leak in the perf sched stats
record command by explicitly deleting the session before returning.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 6e0d8f4d270e2..68147a0be5c71 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -4027,6 +4027,7 @@ static int perf_sched__schedstat_record(struct perf_sched *sched,
>  	else
>  		fprintf(stderr, "[ perf sched stats: Failed !! ]\n");
>  
> +	perf_session__delete(session);
>  	evlist__put(evlist);
>  	close(fd);

[Severity: Medium]
Does adding perf_session__delete() result in closing the file descriptor twice?

Since perf_session__delete() internally closes the file descriptor associated
with the session data, the subsequent explicit close(fd) will attempt to
close it again. This could return EBADF or erroneously close an unrelated
file descriptor if another thread opens a file in the meantime.

>  	return err;
>  }

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/3] perf sched: Add missing perf_session__delete()
  2026-07-13 20:59   ` sashiko-bot
@ 2026-07-13 23:00     ` Namhyung Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-13 23:00 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-perf-users

On Mon, Jul 13, 2026 at 08:59:07PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch introduces a double close of the file descriptor `fd` because `perf_session__delete(session)` inherently closes it before the explicit `close(fd)`.
> --
> 
> --- Patch [1]: [PATCH v1 1/3] perf sched: Add missing perf_session__delete() ---
> commit 2da9bd6a7ce8e17e442d8be81b1b57837aaee6e1
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf sched: Add missing perf_session__delete()
> 
> This patch addresses an ASAN-reported memory leak in the perf sched stats
> record command by explicitly deleting the session before returning.
> 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 6e0d8f4d270e2..68147a0be5c71 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -4027,6 +4027,7 @@ static int perf_sched__schedstat_record(struct perf_sched *sched,
> >  	else
> >  		fprintf(stderr, "[ perf sched stats: Failed !! ]\n");
> >  
> > +	perf_session__delete(session);
> >  	evlist__put(evlist);
> >  	close(fd);
> 
> [Severity: Medium]
> Does adding perf_session__delete() result in closing the file descriptor twice?
> 
> Since perf_session__delete() internally closes the file descriptor associated
> with the session data, the subsequent explicit close(fd) will attempt to
> close it again. This could return EBADF or erroneously close an unrelated
> file descriptor if another thread opens a file in the meantime.

Hmm.. looks right.  I think I can just remove the close().

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-13 23:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 20:47 [PATCH v1 0/3] perf sched stats: Fix memory leaks Namhyung Kim
2026-07-13 20:47 ` [PATCH v1 1/3] perf sched: Add missing perf_session__delete() Namhyung Kim
2026-07-13 20:59   ` sashiko-bot
2026-07-13 23:00     ` Namhyung Kim
2026-07-13 20:47 ` [PATCH v1 2/3] perf sched: Fix memory leaks in perf sched stats report Namhyung Kim
2026-07-13 20:47 ` [PATCH v1 3/3] perf sched: Free subcommand string after perf sched stats Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.