Re: [PATCH RESEND v7 1/2] perf pmu intel: Generalize SNC cpumask adjustment for multiple platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] An integer overflow in `uncore_cha_snc()` can result in a negative Sub-NUMA Clustering (SNC) node index. This bypasses the upper-bound check against `MAX_SNCS` and causes subsequent out-of-bounds array accesses.
--

commit be394e8b7ddbe730c3fc40fb608337fe19c7b33b
Author: Chun-Tse Shao <ctshao@google.com>

perf pmu intel: Generalize SNC cpumask adjustment for multiple platforms

This commit generalizes the Sub-NUMA Clustering (SNC) logic previously
specific to Granite Rapids to support multiple Intel platforms. It uses
CPUID to determine SNC support and caches various PMU configurations
to improve thread safety.

> diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> index 7c9d238922a6c..9b00d5720fb7a 100644
> --- a/tools/perf/arch/x86/util/pmu.c
> +++ b/tools/perf/arch/x86/util/pmu.c

[ ... ]

> @@ -121,46 +142,73 @@ static int uncore_cha_snc(struct perf_pmu *pmu)
>  		return 0;
>  	}
>  	chas_per_node = num_cha / snc_nodes;
> +	if (chas_per_node == 0) {
> +		pr_warning("Unexpected: chas_per_node is 0 (num_cha=%d, snc_nodes=%d)\n",
> +			   num_cha, snc_nodes);
> +		return 0;
> +	}
>  	cha_snc = cha_num / chas_per_node;
>  
>  	/* Range check cha_snc. for unexpected out of bounds. */
>  	return cha_snc >= MAX_SNCS ? 0 : cha_snc;

[Severity: Critical]
This is a pre-existing issue, but does this range check safely handle very
large values for cha_num?

Since cha_num is parsed as an unsigned int via sscanf() from the PMU name,
and cha_snc is a signed int, a sufficiently large cha_num could cause the
division result to overflow into a negative value.

If cha_snc becomes negative, the cha_snc >= MAX_SNCS check evaluates to
false, allowing the function to return a negative index.

Could this negative index then cause out-of-bounds array accesses when used
later in uncore_cha_imc_compute_cpu_adjust()?

uncore_cha_imc_compute_cpu_adjust() {
    static bool checked_cpu_adjust[MAX_SNCS];
    ...
    if (checked_cpu_adjust[pmu_snc])
    ...
}

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