[<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
>
Powered by blists - more mailing lists