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

[Mi, Dapeng]

On 8/28/2026 1:28 PM, Ian Rogers wrote:
> On Tue, Jun 9, 2026 at 11:44 AM Chun-Tse Shao <ctshao@xxxxxxxxxx> wrote:
>> Ping for attention.
> I suspect Arnaldo and Namhyung are waiting for feedback from Intel on
> these patches. If someone could check them it would be appreciated.

I suppose Zide has checked these patches and gave the "reviewed-by" tags. :)


>
> Thanks,
> Ian
>
>> Thanks,
>> CT
>>
>> On Thu, May 28, 2026 at 2:31 PM Chun-Tse Shao <ctshao@xxxxxxxxxx> wrote:
>>> On Wed, May 27, 2026 at 5:11 PM <sashiko-bot@xxxxxxxxxx> wrote:
>>>> 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@xxxxxxxxxx>
>>>>
>>>> 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.
>>> I think it is very unlikely we would see cha_num > INT_MAX.
>>> Since that is a pre-existing issue, I prefer to fix it separately.
>>>
>>> Thanks,
>>> CT
>>>
>>>> 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@xxxxxxxxxx?part=1