linux-kernel - Re: [PATCH 1/2] perf/bench: Fix perf bench internals pmu-scan core dump

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: <CAP-5=fV-X7YKVrd9336KP4uK8YsMs2wtfYRWEqOUvEuQuD2WdA@mail.gmail.com>
Date: Tue, 18 Mar 2025 09:28:33 -0700
From: Ian Rogers <irogers@...gle.com>
To: Thomas Richter <tmricht@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org, 
	linux-perf-users@...r.kernel.org, acme@...nel.org, namhyung@...nel.org, 
	acme@...hat.com, agordeev@...ux.ibm.com, gor@...ux.ibm.com, 
	sumanthk@...ux.ibm.com, hca@...ux.ibm.com, 
	James Clark <james.clark@...aro.org>
Subject: Re: [PATCH 1/2] perf/bench: Fix perf bench internals pmu-scan core dump

On Tue, Mar 18, 2025 at 2:52 AM Thomas Richter <tmricht@...ux.ibm.com> wrote:
>
> On s390 z/VM systems the command 'perf bench internals pmu-scan'
> dumps core, as can be seen:
>
>  # ./perf bench internals pmu-scan
>  # Running 'internals/pmu-scan' benchmark:
>  Computing performance of sysfs PMU event scan for 100 times
>  double free or corruption (out)
>  Aborted (core dumped)
>  # gdb ./perf core.xxxx
>  ....
>  #9  0x00000000012fb57a in perf_pmu__delete (pmu=0x160e370 <tool>)
>      at util/pmu.c:2318
>  #10 0x00000000012fbfca in perf_pmus__destroy () at util/pmus.c:103
>  #11 0x0000000001186f72 in save_result () at bench/pmu-scan.c:71
>  #12 0x00000000011873c2 in run_pmu_scan () at bench/pmu-scan.c:140
>  #13 0x00000000011876a8 in bench_pmu_scan (argc=0, argv=0x3fff3a77338)
>      at bench/pmu-scan.c:183
>  #14 0x0000000001174556 in run_bench (coll_name=0x14709ba "internals",
>      bench_name=0x1470700 "pmu-scan", fn=0x1187620 <bench_pmu_scan>,
>      argc=1, argv=0x3fff3a77338) at builtin-bench.c:229
>  #15 0x0000000001174a1e in cmd_bench (argc=2, argv=0x3fff3a77330)
>      at builtin-bench.c:330
>  ...
>
> The root cause is in PMU buildup. The PMUs are constructed via
>
>   run_bench()
>   +--> bench_pmu_scan()
>        +--> run_pmu_scan()
>             +--> save_result()
>                  +--> perf_pmus__scan()
>                       +--> pmu_read_sysfs()
>                            +--> perf_pmus__tool_pmu()
>
> perf_pmus__tool_pmu() returns a pointer to a static defined variable:
>
>   static struct perf_pmu tool = {
>                 .name = "tool",
>                 .type = PERF_PMU_TYPE_TOOL,
>                 .aliases = LIST_HEAD_INIT(tool.aliases),
>                 .caps = LIST_HEAD_INIT(tool.caps),
>                 .format = LIST_HEAD_INIT(tool.format),
>   };
>
> and that PMU is added to the list of other_cpus in file
> ./util/pmus.c, function pmu_read_sysfs().
>
> Later on after the list of PMUs is constructed,
> that list is removed again via:
>
>    save_result()
>    +--> perf_pmus__destroy()
>         +--> perf_pmu__delete()
>
> This works fine until the PMU named "tool" is deleted.
> Its name is a constant pointer possibly located in read-only data
> section and can not be freed using zfree().
>
> Remedy this and check for dynamic memory allocation for the PMU.
>
> Background: s390 z/VM system do not support PMUs for sampling and
> counting. In this case dummy events are created by the perf tool
> and the PMUs "tool" and "fake" are created and freed.
>
> Fixes: efe98a7a3977 ("perf pmu: Use zfree() to reduce chances of use after free")
> Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Ian Rogers <irogers@...gle.com>

Hi Thomas,

Was this already addressed by James' patch:
https://lore.kernel.org/linux-perf-users/20250226104111.564443-1-james.clark@linaro.org/

Thanks,
Ian

> ---
>  tools/perf/util/pmu.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 6206c8fe2bf9..59cec4d2909e 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -2315,10 +2315,13 @@ void perf_pmu__delete(struct perf_pmu *pmu)
>
>         perf_cpu_map__put(pmu->cpus);
>
> -       zfree(&pmu->name);
> -       zfree(&pmu->alias_name);
> -       zfree(&pmu->id);
> -       free(pmu);
> +       /* Static variables can not be free'ed */
> +       if (pmu->type != PERF_PMU_TYPE_TOOL && pmu->type != PERF_PMU_TYPE_FAKE) {
> +               zfree(&pmu->alias_name);
> +               zfree(&pmu->id);
> +               zfree(&pmu->name);
> +               free(pmu);
> +       }
>  }
>
>  const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
> --
> 2.48.1
>