Re: [PATCH] perf/x86/amd/uncore: Extend num_umc_pmc to fix UMC discovery

[Sandipan Das <sandipan.das@amd.com>]

On 07-10-2026 21:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] perf/x86/amd: OOB write on active_mask via unvalidated CPUID
> --
> 
> commit 00612d17a5d2f57d4e77167b7e9d6303f0e776ac
> Author: Sandipan Das <sandipan.das@amd.com>
> 
> perf/x86/amd/uncore: Extend num_umc_pmc to fix UMC discovery
> 
> The commit extends the width of the num_umc_pmc bitfield from six bits
> to eight bits to match Zen 5 documentation. This allows discovery of
> Unified Memory Controller (UMC) counters up to 64 in fully-populated
> Zen 6 configurations.
> 
>> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
>> index 1eb13673e889f..d419b26086b22 100644
>> --- a/arch/x86/include/asm/perf_event.h
>> +++ b/arch/x86/include/asm/perf_event.h
>> @@ -293,7 +293,7 @@ union cpuid_0x80000022_ebx {
>>  		/* Number of Data Fabric Counters */
>>  		unsigned int	num_df_pmc:6;
>>  		/* Number of Unified Memory Controller Counters */
>> -		unsigned int	num_umc_pmc:6;
>> +		unsigned int	num_umc_pmc:8;
> 
> [Severity: High]
> Does this expansion allow values up to 255 from CPUID to propagate unchecked?
> 
> When the field was 6 bits, the natural boundary limited it to 63, which fit
> safely inside NUM_COUNTERS_MAX (64). With an 8-bit width, a hypervisor could
> supply a value greater than 64 in EBX.NumPerfCtrUmc.
> 

Uncore counters are generally not visible to guests.

> This value gets propagated down during initialization without validation:
> 
> arch/x86/events/amd/uncore.c:amd_uncore_umc_ctx_init() {
>     ...
>     pmu->num_counters = group_num_pmcs[gid] / group_num_pmus[gid];
>     ...
> }
> 
> If pmu->num_counters exceeds NUM_COUNTERS_MAX, can it cause an
> out-of-bounds write when starting the event?
> 

Not for real hardware since each UMC PMU has at most 4 PMCs.

> arch/x86/events/amd/uncore.c:amd_uncore_umc_start() {
>     ...
>     __set_bit(hwc->idx, ctx->active_mask);
>     ...
> }
> 
> Since ctx->active_mask is sized by BITS_TO_LONGS(NUM_COUNTERS_MAX), writing
> a bit greater than 63 might overflow into the adjacent nr_active and
> hrtimer fields in struct amd_uncore_ctx.
> 
> Should we cap pmu->num_counters against NUM_COUNTERS_MAX during
> initialization to prevent this regression?
> 
>>  	} split;
>>  	unsigned int		full;
>>  };
>