* [PATCH v1] perf stat: Avoid uninitialized memory read on error
@ 2026-09-08 22:07 Ian Rogers
2026-09-08 22:23 ` sashiko-bot
2026-09-09 11:10 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 4+ messages in thread
From: Ian Rogers @ 2026-09-08 22:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
linux-perf-users, linux-kernel
If has_supported_counters fails then msg was passed to ui__error
regardless of whether an error was written into msg leading to reading
uninitialized memory. Avoid this by always terminating msg at
initialization.
Opportunistically reduce the scope of msg to make its use clearer.
Fixes: c9a8c343ef2f ("perf stat: When no events, don't report an error if there is none")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-stat.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index c50cf230f16e..37752d6bfef4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -777,7 +777,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
int interval = stat_config.interval;
int times = stat_config.times;
int timeout = stat_config.timeout;
- char msg[BUFSIZ];
unsigned long long t0, t1;
struct evsel *counter;
size_t l;
@@ -908,6 +907,9 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
}
}
if (!has_supported_counters && !stat_config.null_run) {
+ char msg[BUFSIZ];
+
+ msg[0] = '\0';
if (open_err) {
evsel__open_strerror(evlist__first(evsel_list), &target, open_err,
msg, sizeof(msg));
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf stat: Avoid uninitialized memory read on error
2026-09-08 22:07 [PATCH v1] perf stat: Avoid uninitialized memory read on error Ian Rogers
@ 2026-09-08 22:23 ` sashiko-bot
2026-09-08 22:37 ` Ian Rogers
2026-09-09 11:10 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-09-08 22:23 UTC (permalink / raw)
To: Ian Rogers; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Atomic Conclusion: Unchecked ERR_PTR from hashmap__new() causes predictable application crashes. The return value is incorrectly checked against NULL instead of IS_ERR().
--
commit 525c560536a62c39331ec585b1ce897decf7179c
Author: Ian Rogers <irogers@google.com>
perf stat: Avoid uninitialized memory read on error
If has_supported_counters fails then msg was passed to ui__error
regardless of whether an error was written into msg leading to reading
uninitialized memory. Avoid this by always terminating msg at
initialization.
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index c50cf230f16ed..37752d6bfef45 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but I noticed that the return
value of hashmap__new() is incorrectly checked against NULL instead of
IS_ERR().
Since hashmap__new() returns an ERR_PTR on failure, this will
cause predictable crashes under memory pressure instead of graceful error
handling.
For example, in tools/perf/util/pmu.c:perf_pmu__init():
pmu->aliases = hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL);
if (!pmu->aliases)
return -ENOMEM;
And similarly in tools/perf/util/pmu.c:perf_pmu__create_placeholder_core_pmu():
pmu->aliases = hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL);
INIT_LIST_HEAD(&pmu->caps);
Also in tools/perf/util/fncache.c:fncache__init():
fncache = hashmap__new(fncache__hash, fncache__equal, /*ctx=*/NULL);
Should these locations be updated to use IS_ERR() when validating the result
of hashmap__new()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260908220720.564614-1-irogers@google.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf stat: Avoid uninitialized memory read on error
2026-09-08 22:23 ` sashiko-bot
@ 2026-09-08 22:37 ` Ian Rogers
0 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2026-09-08 22:37 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users
On Tue, Sep 8, 2026 at 3:23 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Atomic Conclusion: Unchecked ERR_PTR from hashmap__new() causes predictable application crashes. The return value is incorrectly checked against NULL instead of IS_ERR().
Yep, libbpf's hashmap is a thorn in the side of correctness and
ERR_PTRs are the devil's work. The hashmap code also intentionally has
undefined behavior. I sent this series:
https://lore.kernel.org/linux-perf-users/20260322005823.981079-1-irogers@google.com/
to address this. Since the BPF folks are happy with the current state,
I think we should fork the code to avoid issues in perf - we'll still
suffer with the hashmaps in libbpf. I was investigating whether we can
add some form of generics to hashmaps too. Anyway, the issues reported
here are correct and genuine bugs, but they aren't relevant to this
patch.
Thanks,
Ian
> --
>
> commit 525c560536a62c39331ec585b1ce897decf7179c
> Author: Ian Rogers <irogers@google.com>
>
> perf stat: Avoid uninitialized memory read on error
>
> If has_supported_counters fails then msg was passed to ui__error
> regardless of whether an error was written into msg leading to reading
> uninitialized memory. Avoid this by always terminating msg at
> initialization.
>
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index c50cf230f16ed..37752d6bfef45 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
>
> [ ... ]
>
> [Severity: High]
> This isn't a bug introduced by this patch, but I noticed that the return
> value of hashmap__new() is incorrectly checked against NULL instead of
> IS_ERR().
>
> Since hashmap__new() returns an ERR_PTR on failure, this will
> cause predictable crashes under memory pressure instead of graceful error
> handling.
>
> For example, in tools/perf/util/pmu.c:perf_pmu__init():
>
> pmu->aliases = hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL);
> if (!pmu->aliases)
> return -ENOMEM;
>
> And similarly in tools/perf/util/pmu.c:perf_pmu__create_placeholder_core_pmu():
>
> pmu->aliases = hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL);
> INIT_LIST_HEAD(&pmu->caps);
>
> Also in tools/perf/util/fncache.c:fncache__init():
>
> fncache = hashmap__new(fncache__hash, fncache__equal, /*ctx=*/NULL);
>
> Should these locations be updated to use IS_ERR() when validating the result
> of hashmap__new()?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260908220720.564614-1-irogers@google.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf stat: Avoid uninitialized memory read on error
2026-09-08 22:07 [PATCH v1] perf stat: Avoid uninitialized memory read on error Ian Rogers
2026-09-08 22:23 ` sashiko-bot
@ 2026-09-09 11:10 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-09-09 11:10 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Jiri Olsa,
Adrian Hunter, James Clark, linux-perf-users, linux-kernel
On Tue, Sep 08, 2026 at 03:07:20PM -0700, Ian Rogers wrote:
> If has_supported_counters fails then msg was passed to ui__error
> regardless of whether an error was written into msg leading to reading
> uninitialized memory. Avoid this by always terminating msg at
> initialization.
>
> Opportunistically reduce the scope of msg to make its use clearer.
Thanks, applied to perf-tools-next, for v7.4.
- Arnaldo
> Fixes: c9a8c343ef2f ("perf stat: When no events, don't report an error if there is none")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/builtin-stat.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index c50cf230f16e..37752d6bfef4 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -777,7 +777,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> int interval = stat_config.interval;
> int times = stat_config.times;
> int timeout = stat_config.timeout;
> - char msg[BUFSIZ];
> unsigned long long t0, t1;
> struct evsel *counter;
> size_t l;
> @@ -908,6 +907,9 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> }
> }
> if (!has_supported_counters && !stat_config.null_run) {
> + char msg[BUFSIZ];
> +
> + msg[0] = '\0';
> if (open_err) {
> evsel__open_strerror(evlist__first(evsel_list), &target, open_err,
> msg, sizeof(msg));
> --
> 2.55.0.979.g7e5102b832-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-09 11:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 22:07 [PATCH v1] perf stat: Avoid uninitialized memory read on error Ian Rogers
2026-09-08 22:23 ` sashiko-bot
2026-09-08 22:37 ` Ian Rogers
2026-09-09 11:10 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox