[PATCH] perf trace: free intlist on filter_pids calloc failure

[PATCH] perf trace: free intlist on filter_pids calloc failure

[Gu Bin]

trace__set_filter_pids_from_option() builds an intlist and stores its
entry count in trace->filter_pids.nr before allocating the entries
array.  If that calloc fails, the function jumps to the out label,
which returns without freeing the intlist: the option callback exits
perf with an error, so the leak is the only damage, but it is
triggered on every --filter-pids run under memory pressure.

Restructure the function so that every path that has created the
intlist also frees it: the failure path now falls through to
intlist__delete() as well, and the out label is gone.

Fixes: f078c3852c73 ("perf trace: Introduce --filter-pids")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/perf/builtin-trace.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4094141461f91..d435b35f1351c 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -5432,17 +5432,16 @@ static int trace__set_filter_pids_from_option(const struct option *opt, const ch
 	i = trace->filter_pids.nr = intlist__nr_entries(list) + 1;
 	trace->filter_pids.entries = calloc(i, sizeof(pid_t));
 
-	if (trace->filter_pids.entries == NULL)
-		goto out;
+	if (trace->filter_pids.entries) {
+		trace->filter_pids.entries[0] = getpid();
 
-	trace->filter_pids.entries[0] = getpid();
+		for (i = 1; i < trace->filter_pids.nr; ++i)
+			trace->filter_pids.entries[i] = intlist__entry(list, i - 1)->i;
 
-	for (i = 1; i < trace->filter_pids.nr; ++i)
-		trace->filter_pids.entries[i] = intlist__entry(list, i - 1)->i;
+		ret = 0;
+	}
 
 	intlist__delete(list);
-	ret = 0;
-out:
 	return ret;
 }
 
-- 
2.50.1.windows.1

Re: [PATCH] perf trace: free intlist on filter_pids calloc failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] perf trace: memory leak in trace__exit() [1]
--

commit e8d1d018ef3c0a4fdf8d8ce3c0227b4280b491f6
Author: Gu Bin <gubin@cmss.chinamobile.com>

perf trace: free intlist on filter_pids calloc failure

This commit restructures trace__set_filter_pids_from_option() so that the
temporary intlist is freed even when allocating trace->filter_pids.entries
fails.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 4094141461f91..d435b35f1351c 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -5432,17 +5432,16 @@ static int trace__set_filter_pids_from_option(const struct option *opt, const ch
>  	i = trace->filter_pids.nr = intlist__nr_entries(list) + 1;
>  	trace->filter_pids.entries = calloc(i, sizeof(pid_t));
>  
> -	if (trace->filter_pids.entries == NULL)
> -		goto out;
> +	if (trace->filter_pids.entries) {
> +		trace->filter_pids.entries[0] = getpid();
>  
> -	trace->filter_pids.entries[0] = getpid();
> +		for (i = 1; i < trace->filter_pids.nr; ++i)
> +			trace->filter_pids.entries[i] = intlist__entry(list, i - 1)->i;
>  
> -	for (i = 1; i < trace->filter_pids.nr; ++i)
> -		trace->filter_pids.entries[i] = intlist__entry(list, i - 1)->i;
> +		ret = 0;
> +	}

[Severity: Medium]
This problem wasn't introduced by this patch, but does trace__exit() leak
trace->filter_pids.entries [1]?  trace__set_filter_pids_from_option()
allocates the array with calloc(), and trace__exit() never frees it on
teardown.

[1] https://sashiko.dev/#/bug/linux-ae3f475f-4084-473d-a5c5-25fd465ce61d

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112083123.2154-1-gubin@cmss.chinamobile.com?part=1