[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKwvOdnNgd9QgvrR5H2rL8eb1Qc--ELCcOie8Bv=xTFqg9Zh-A@mail.gmail.com>
Date: Mon, 24 Jul 2023 14:09:19 -0700
From: Nick Desaulniers <ndesaulniers@...gle.com>
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>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Nathan Chancellor <nathan@...nel.org>,
Tom Rix <trix@...hat.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Yang Jihong <yangjihong1@...wei.com>,
Ravi Bangoria <ravi.bangoria@....com>,
Carsten Haitzler <carsten.haitzler@....com>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
James Clark <james.clark@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
bpf@...r.kernel.org, llvm@...ts.linux.dev, maskray@...gle.com
Subject: Re: [PATCH v1 1/4] perf stat: Avoid uninitialized use of perf_stat_config
On Mon, Jul 24, 2023 at 1:12 PM Ian Rogers <irogers@...gle.com> wrote:
>
> perf_event__read_stat_config will assign values based on number of
> tags and tag values. Initialize the structs to zero before they are
> assigned so that no uninitialized values can be seen.
>
> This potential error was reported by GCC with LTO enabled.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/tests/stat.c | 2 +-
> tools/perf/util/stat.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/stat.c b/tools/perf/tests/stat.c
> index 500974040fe3..706780fb5695 100644
> --- a/tools/perf/tests/stat.c
> +++ b/tools/perf/tests/stat.c
> @@ -27,7 +27,7 @@ static int process_stat_config_event(struct perf_tool *tool __maybe_unused,
> struct machine *machine __maybe_unused)
> {
> struct perf_record_stat_config *config = &event->stat_config;
> - struct perf_stat_config stat_config;
> + struct perf_stat_config stat_config = {};
^ how did this code ever work?
1. stat_config is not initialized
2. perf_event__read_stat_config maybe assigns to &stat_config->__val
3. process_stat_config_event() tests other members of stat_config
I hope I've missed something obvious.
Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
>
> #define HAS(term, val) \
> has_term(config, PERF_STAT_CONFIG_TERM__##term, val)
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 967e583392c7..ec3506042217 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -729,7 +729,7 @@ size_t perf_event__fprintf_stat_round(union perf_event *event, FILE *fp)
>
> size_t perf_event__fprintf_stat_config(union perf_event *event, FILE *fp)
> {
> - struct perf_stat_config sc;
> + struct perf_stat_config sc = {};
> size_t ret;
>
> perf_event__read_stat_config(&sc, &event->stat_config);
> --
> 2.41.0.487.g6d72f3e995-goog
>
--
Thanks,
~Nick Desaulniers
Powered by blists - more mailing lists