linux-kernel - Re: [PATCH 04/12] KVM: MMU: WARN if PAE roots linger after kvm_mmu

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]

Date:   Fri, 11 Feb 2022 12:18:07 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        vkuznets@...hat.com, mlevitsk@...hat.com, dmatlack@...gle.com
Subject: Re: [PATCH 04/12] KVM: MMU: WARN if PAE roots linger after
 kvm_mmu_unload

On 2/11/22 00:20, Sean Christopherson wrote:
> On Wed, Feb 09, 2022, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
>> ---
>>   arch/x86/kvm/mmu/mmu.c | 17 +++++++++++++----
>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>> index e0c0f0bc2e8b..7b5765ced928 100644
>> --- a/arch/x86/kvm/mmu/mmu.c
>> +++ b/arch/x86/kvm/mmu/mmu.c
>> @@ -5065,12 +5065,21 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
>>   	return r;
>>   }
>>   
>> +static void __kvm_mmu_unload(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
>> +{
>> +	int i;
>> +	kvm_mmu_free_roots(vcpu, mmu, KVM_MMU_ROOTS_ALL);
>> +	WARN_ON(VALID_PAGE(mmu->root_hpa));
>> +	if (mmu->pae_root) {
>> +		for (i = 0; i < 4; ++i)
>> +			WARN_ON(IS_VALID_PAE_ROOT(mmu->pae_root[i]));
>> +	}
> 
> I'm somewhat ambivalent, but if you're at all on the fence, I vote to drop this
> one.  I've always viewed the WARN on root_hpa as gratuitous.
> 
> But, if it helped during development, then why not...

Well, it was not really helping in that the WARN triggered, but rather 
it was ruling out the more blatant violations of invariants.  The one in 
patch 5 triggered a lot, though.

Paolo