Messages in this thread | | | From | Namhyung Kim <> | Date | Tue, 5 Sep 2023 19:58:20 -0700 | Subject | Re: [PATCH v1] perf stat: Fix aggr mode initialization |
| |
Hi Ian,
On Tue, Sep 5, 2023 at 5:39 PM Ian Rogers <irogers@google.com> wrote: > > Generating metrics llc_code_read_mpi_demand_plus_prefetch, > llc_data_read_mpi_demand_plus_prefetch, > llc_miss_local_memory_bandwidth_read, > llc_miss_local_memory_bandwidth_write, > nllc_miss_remote_memory_bandwidth_read, memory_bandwidth_read, > memory_bandwidth_write, uncore_frequency, upi_data_transmit_bw, > C2_Pkg_Residency, C3_Core_Residency, C3_Pkg_Residency, > C6_Core_Residency, C6_Pkg_Residency, C7_Core_Residency, > C7_Pkg_Residency, UNCORE_FREQ and tma_info_system_socket_clks would > trigger an address sanitizer heap-buffer-overflows on a SkylakeX. > > ``` > ==2567752==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5020003ed098 at pc 0x5621a816654e bp 0x7fffb55d4da0 sp 0x7fffb55d4d98 > READ of size 4 at 0x5020003eee78 thread T0 > #0 0x558265d6654d in aggr_cpu_id__is_empty tools/perf/util/cpumap.c:694:12 > #1 0x558265c914da in perf_stat__get_aggr tools/perf/builtin-stat.c:1490:6 > #2 0x558265c914da in perf_stat__get_global_cached tools/perf/builtin-stat.c:1530:9 > #3 0x558265e53290 in should_skip_zero_counter tools/perf/util/stat-display.c:947:31 > #4 0x558265e53290 in print_counter_aggrdata tools/perf/util/stat-display.c:985:18 > #5 0x558265e51931 in print_counter tools/perf/util/stat-display.c:1110:3 > #6 0x558265e51931 in evlist__print_counters tools/perf/util/stat-display.c:1571:5 > #7 0x558265c8ec87 in print_counters tools/perf/builtin-stat.c:981:2 > #8 0x558265c8cc71 in cmd_stat tools/perf/builtin-stat.c:2837:3 > #9 0x558265bb9bd4 in run_builtin tools/perf/perf.c:323:11 > #10 0x558265bb98eb in handle_internal_command tools/perf/perf.c:377:8 > #11 0x558265bb9389 in run_argv tools/perf/perf.c:421:2 > #12 0x558265bb9389 in main tools/perf/perf.c:537:3 > ``` > > The issue was the use of testing a cpumap with NULL rather than using > empty, as a map containing the dummy value isn't NULL and the -1 > results in an empty aggr map being allocated which legitimately > overflows when any member is accessed.
Right, evlist__create_maps() set it to non-NULL.
> > Fixes: 8a96f454f566 ("perf stat: Avoid SEGV if core.cpus isn't set") > Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks, Namhyung
> --- > tools/perf/builtin-stat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 07b48f6df48e..a3af805a1d57 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -1622,7 +1622,7 @@ static int perf_stat_init_aggr_mode(void) > * taking the highest cpu number to be the size of > * the aggregation translate cpumap. > */ > - if (evsel_list->core.user_requested_cpus) > + if (!perf_cpu_map__empty(evsel_list->core.user_requested_cpus)) > nr = perf_cpu_map__max(evsel_list->core.user_requested_cpus).cpu; > else > nr = 0; > -- > 2.42.0.283.g2d96d420d3-goog >
| |