Re: [PATCH v12 28/33] perf kmem: Add bounds checks to tracepoint read values

sashiko-bot
2026-04-13T04:45:23Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Returning -1 on out-of-bounds trace data aborts the entire perf session.
--

commit 67635d34bc11f12c6dc78c6031f71bc9fb086276
Author: Ian Rogers <irogers@google.com>

perf kmem: Add bounds checks to tracepoint read values

This commit adds bounds checks for order and migrate_type tracepoint values
in the page_alloc and page_free event handlers to prevent out-of-bound
memory accesses.

> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index daf2272c73372..33585e353efe5 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -826,6 +826,16 @@ static int evsel__process_page_alloc_event(struct perf_sample *sample)
> .migrate_type = migrate_type,
> };
>
> + if (order >= MAX_PAGE_ORDER) {
> + pr_debug("Out-of-bounds order %u\n", order);
> + return -1;

Will returning -1 here abort the entire perf session? In the perf events
subsystem, returning a negative value from an event callback causes
perf_session__process_events() to stop processing the rest of the perf.data
file.

Since kernels can be configured with higher max zone orders (for example,
via CONFIG_ARCH_FORCE_MAX_ORDER on arm64), would this strict abort behavior
prevent perf kmem from analyzing valid traces from those configurations?

Could this return 0 instead to safely skip the unsupported sample while
continuing to process the rest of the valid trace data?

> + }
> +
> + if (migrate_type >= MAX_MIGRATE_TYPES) {
> + pr_debug("Out-of-bounds migratetype %u\n", migrate_type);
> + return -1;

Similarly, if a newer kernel release adds new migrate types, will this also
abort the session? Would returning 0 be safer here as well?

> + }
> +
> if (use_pfn)

[ ... ]

> @@ -892,6 +902,11 @@ static int evsel__process_page_free_event(struct perf_sample *sample)
> .order = order,
> };
>
> + if (order >= MAX_PAGE_ORDER) {
> + pr_debug("Out-of-bounds order %u\n", order);
> + return -1;
> + }
> +

Does this order check have the same abort issue for the page free events?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260413041143.1736055-1-irogers@google.com?part=28