linux-kernel - Re: [kvm-unit-tests patch v6 05/18] x86: pmu: Enlarge cnt[] length to 48 in check_counters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cf013079-ad8a-4b07-bbcf-6f35d1126a92@linux.intel.com>
Date: Tue, 18 Feb 2025 17:24:40 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Jim Mattson <jmattson@...gle.com>,
 Mingwei Zhang <mizhang@...gle.com>, Xiong Zhang <xiong.y.zhang@...el.com>,
 Zhenyu Wang <zhenyuw@...ux.intel.com>, Like Xu <like.xu.linux@...il.com>,
 Jinrong Liang <cloudliang@...cent.com>, Yongwei Ma <yongwei.ma@...el.com>,
 Dapeng Mi <dapeng1.mi@...el.com>
Subject: Re: [kvm-unit-tests patch v6 05/18] x86: pmu: Enlarge cnt[] length to
 48 in check_counters_many()


On 2/15/2025 5:06 AM, Sean Christopherson wrote:
> On Sat, Sep 14, 2024, Dapeng Mi wrote:
>> Considering there are already 8 GP counters and 4 fixed counters on
>> latest Intel processors, like Sapphire Rapids. The original cnt[] array
>> length 10 is definitely not enough to cover all supported PMU counters on
>> these new processors even through currently KVM only supports 3 fixed
>> counters at most. This would cause out of bound memory access and may trigger
>> false alarm on PMU counter validation
>>
>> It's probably more and more GP and fixed counters are introduced in the
>> future and then directly extends the cnt[] array length to 48 once and
>> for all. Base on the layout of IA32_PERF_GLOBAL_CTRL and
>> IA32_PERF_GLOBAL_STATUS, 48 looks enough in near feature.
>>
>> Reviewed-by: Jim Mattson <jmattson@...gle.com>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
>> ---
>>  x86/pmu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index a0268db8..b4de2680 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -255,7 +255,7 @@ static void check_fixed_counters(void)
>>  
>>  static void check_counters_many(void)
>>  {
>> -	pmu_counter_t cnt[10];
>> +	pmu_counter_t cnt[48];
> ARGH.  Since the *entire* purpose of increasing the size is to guard against
> buffer overflow, add an assert that the loop doesn't overflow.

This is not only for ensuring no buffer overflow. As the commit message
says,  the counter number has already exceeded 10, such as SPR has 12
counters (8 GP + 4 fixed), and there would be more counters in later
platfroms. The aim of enlarging the array size is to ensure these counters
can be enabled and verified simultaneously.  48 may be too large and 32
should be fair enough. Thanks.


>
>>  	int i, n;
>>  
>>  	for (i = 0, n = 0; n < pmu.nr_gp_counters; i++) {
>> -- 
>> 2.40.1
>>