linux-kernel - Re: [PATCH 2/2] perf stat: Fix segfault when -r option is used

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: <CAM9d7cjA7aNJoGxi8PjpF3G0zD89V39Eg=JBAOoPOxW3KtWx3Q@mail.gmail.com>
Date:   Wed, 24 Feb 2021 17:45:32 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>,
        Ian Rogers <irogers@...gle.com>
Subject: Re: [PATCH 2/2] perf stat: Fix segfault when -r option is used

On Wed, Feb 24, 2021 at 5:11 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> I got a segfault when using -r option with event groups.  The option
> makes it run the workload multiple times and it will reuse the evlist
> and evsel for each run.

Well, it might not see a segfault because the freed memory region is
likely to be reused.  But you can see the bug clearly with asan.

Thanks,
Namhyung

>
> While most of resources are allocated and freed properly, the id hash
> in the evlist was not and it resulted in a crash.  You can see it with
> the address sanitizer like below:
>
>   $ perf stat -r 100 -e '{cycles,instructions}' true
>   =================================================================
>   ==693052==ERROR: AddressSanitizer: heap-use-after-free on
>       address 0x6080000003d0 at pc 0x558c57732835 bp 0x7fff1526adb0 sp 0x7fff1526ada8
>   WRITE of size 8 at 0x6080000003d0 thread T0
>     #0 0x558c57732834 in hlist_add_head /home/namhyung/project/linux/tools/include/linux/list.h:644
>     #1 0x558c57732834 in perf_evlist__id_hash /home/namhyung/project/linux/tools/lib/perf/evlist.c:237
>     #2 0x558c57732834 in perf_evlist__id_add /home/namhyung/project/linux/tools/lib/perf/evlist.c:244
>     #3 0x558c57732834 in perf_evlist__id_add_fd /home/namhyung/project/linux/tools/lib/perf/evlist.c:285
>     #4 0x558c5747733e in store_evsel_ids util/evsel.c:2765
>     #5 0x558c5747733e in evsel__store_ids util/evsel.c:2782
>     #6 0x558c5730b717 in __run_perf_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:895
>     #7 0x558c5730b717 in run_perf_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:1014
>     #8 0x558c5730b717 in cmd_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:2446
>     #9 0x558c57427c24 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:313
>     #10 0x558c572b1a48 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:365
>     #11 0x558c572b1a48 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:409
>     #12 0x558c572b1a48 in main /home/namhyung/project/linux/tools/perf/perf.c:539
>     #13 0x7fcadb9f7d09 in __libc_start_main ../csu/libc-start.c:308
>     #14 0x558c572b60f9 in _start (/home/namhyung/project/linux/tools/perf/perf+0x45d0f9)
>
> Actually the nodes in the hash table are struct perf_stream_id and
> they were freed in the previous run.  Fix it by resetting the hash.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/evlist.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 5121b4db66fe..882cd1f721d9 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1306,6 +1306,7 @@ void evlist__close(struct evlist *evlist)
>                 perf_evsel__free_fd(&evsel->core);
>                 perf_evsel__free_id(&evsel->core);
>         }
> +       perf_evlist__reset_id_hash(&evlist->core);
>  }
>
>  static int evlist__create_syswide_maps(struct evlist *evlist)
> --
> 2.30.0.617.g56c4b15f3c-goog
>