Re: [PATCH 10/13] perf header: Sanity check HEADER_HYBRID_TOPOLOGY

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Low] The MAX_PMU_MAPPINGS macro is defined locally inside process_pmu_mappings() but reused in process_hybrid_topology().
--

commit b5af27fec41475de3166cc72abe94ddc329689cc
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_HYBRID_TOPOLOGY

This commit adds an upper bound check on nr_nodes in
process_hybrid_topology() to harden against malformed perf.data files by
reusing the MAX_PMU_MAPPINGS limit.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3448,6 +3448,18 @@ static int process_hybrid_topology(struct feat_fd *ff,
>  	if (do_read_u32(ff, &nr))
>  		return -1;
>  
> +	if (nr > MAX_PMU_MAPPINGS) {

This isn't a bug, but could this cause build failures if the functions are
reordered in the future?

MAX_PMU_MAPPINGS is currently defined locally inside process_pmu_mappings():

tools/perf/util/header.c:process_pmu_mappings() {
    ...
#define MAX_PMU_MAPPINGS 4096
	if (pmu_num > MAX_PMU_MAPPINGS) {
    ...
}

While the C preprocessor doesn't respect function block scope, allowing it
to be used here, this relies on process_pmu_mappings() always appearing
lexically before process_hybrid_topology() in this file.

Should this macro be hoisted to file scope to prevent accidental breakage
during future refactoring?

> +		pr_err("Invalid HEADER_HYBRID_TOPOLOGY: nr_nodes (%u) > %u\n",
> +		       nr, MAX_PMU_MAPPINGS);
> +		return -1;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410004000.148138-1-acme@kernel.org?part=10