linux-kernel - Re: [PATCH v3 09/27] KVM: VMX: Do not use MAX_POSSIBLE_PASSTHROUGH

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: <e7f6e7c2-272a-4527-ba50-08167564e787@zytor.com>
Date: Tue, 26 Nov 2024 11:22:45 -0800
From: Xin Li <xin@...or.com>
To: Borislav Petkov <bp@...en8.de>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, seanjc@...gle.com, pbonzini@...hat.com,
        corbet@....net, tglx@...utronix.de, mingo@...hat.com,
        dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
        luto@...nel.org, peterz@...radead.org, andrew.cooper3@...rix.com
Subject: Re: [PATCH v3 09/27] KVM: VMX: Do not use
 MAX_POSSIBLE_PASSTHROUGH_MSRS in array definition

On 11/26/2024 10:02 AM, Borislav Petkov wrote:
> On Mon, Sep 30, 2024 at 10:00:52PM -0700, Xin Li (Intel) wrote:
>> No need to use MAX_POSSIBLE_PASSTHROUGH_MSRS in the definition of array
>> vmx_possible_passthrough_msrs, as the macro name indicates the _possible_
>> maximum size of passthrough MSRs.
>>
>> Use ARRAY_SIZE instead of MAX_POSSIBLE_PASSTHROUGH_MSRS when the size of
>> the array is needed and add a BUILD_BUG_ON to make sure the actual array
>> size does not exceed the possible maximum size of passthrough MSRs.
> 
> This commit message needs to talk about the why - not the what. Latter should
> be visible from the diff itself.

I should not write such a changelog...

> What you're not talking about is the sneaked increase of
> MAX_POSSIBLE_PASSTHROUGH_MSRS to 64. Something you *should* mention because
> the array is full and blablabla...

It's still far from full in a bitmap on x86-64, but just that the
existing use of MAX_POSSIBLE_PASSTHROUGH_MSRS tastes bad.


A better one?

Per the definition, a bitmap on x86-64 is an array of 'unsigned long',
and is at least 64-bit long.

#define DECLARE_BITMAP(name,bits) \
	unsigned long name[BITS_TO_LONGS(bits)]

It's not accurate and error-prone to use a hard-coded possible size of
a bitmap, Use ARRAY_SIZE with an overflow build check instead.

> 
>> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
>> index e0d76d2460ef..e7409f8f28b1 100644
>> --- a/arch/x86/kvm/vmx/vmx.h
>> +++ b/arch/x86/kvm/vmx/vmx.h
>> @@ -356,7 +356,7 @@ struct vcpu_vmx {
>>   	struct lbr_desc lbr_desc;
>>   
>>   	/* Save desired MSR intercept (read: pass-through) state */
>> -#define MAX_POSSIBLE_PASSTHROUGH_MSRS	16
>> +#define MAX_POSSIBLE_PASSTHROUGH_MSRS	64
> 						^^^
>