linux-kernel - Re: [PATCH 2/5] perf sched: Fix memory leak in perf_sched_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZcEv5BBvNEye4ZYf@x1>
Date: Mon, 5 Feb 2024 15:58:44 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Yang Jihong <yangjihong1@...wei.com>
Cc: peterz@...radead.org, mingo@...hat.com, namhyung@...nel.org,
	mark.rutland@....com, alexander.shishkin@...ux.intel.com,
	jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] perf sched: Fix memory leak in perf_sched__map()

On Mon, Feb 05, 2024 at 10:46:13AM +0000, Yang Jihong wrote:
> +++ b/tools/perf/builtin-sched.c
> @@ -3267,20 +3264,34 @@ static int setup_color_cpus(struct perf_sched *sched)
>  
>  static int perf_sched__map(struct perf_sched *sched)
>  {
> +	int rc = -1;
> +
>  	if (setup_map_cpus(sched))
> -		return -1;
> +		return rc;
>  
>  	if (setup_color_pids(sched))
> -		return -1;
> +		goto out_free_map_cpus;

I think renaming the goto labels to what they will do, dropping a
refcount, is more clear, i.e.:

		goto out_put_map_cpus;

>  
>  	if (setup_color_cpus(sched))
> -		return -1;
> +		goto out_free_color_pids;
>  
>  	setup_pager();
>  	if (perf_sched__read_events(sched))
> -		return -1;
> +		goto out_free_color_cpus;
> +
> +	rc = 0;
>  	print_bad_events(sched);
> -	return 0;
> +
> +out_free_color_cpus:
> +	perf_cpu_map__put(sched->map.color_cpus);
> +
> +out_free_color_pids:
> +	perf_thread_map__put(sched->map.color_pids);
> +
> +out_free_map_cpus:
> +	free(sched->map.comp_cpus);

Please use:

	zfree(&sched->map.comp_cpus);

> +	perf_cpu_map__put(sched->map.cpus);
> +	return rc;
>  }