lkml.org 
[lkml]   [2023]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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
>

\
 
 \ /
  Last update: 2023-09-06 04:59    [W:0.037 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site