linux-kernel - Re: [PATCH v1] perf stat: Fix aggr mode initialization

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: <CAM9d7cgBZufrSWQhSeipV35_y3Kb1EWSoqtXMvLBnCFaoS9Atg@mail.gmail.com>
Date:   Tue, 5 Sep 2023 19:58:20 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] perf stat: Fix aggr mode initialization

Hi Ian,

On Tue, Sep 5, 2023 at 5:39 PM Ian Rogers <irogers@...gle.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@...gle.com>

Acked-by: Namhyung Kim <namhyung@...nel.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
>