linux-kernel - Re: [PATCH 6/7] KVM: nVMX: don't load PDPTRS right after nested state set

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:   Wed, 17 Feb 2021 19:06:32 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Wanpeng Li <wanpengli@...cent.com>,
        Borislav Petkov <bp@...en8.de>, Joerg Roedel <joro@...tes.org>,
        Jim Mattson <jmattson@...gle.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 6/7] KVM: nVMX: don't load PDPTRS right after nested state
 set

On 17/02/21 18:52, Sean Christopherson wrote:
>>
>> Just move the call to nested_vmx_load_cr3 to nested_get_vmcs12_pages
>> to implement this.
>
> I don't love this approach.  KVM_SET_NESTED_STATE will now succeed with a bad
> vmcs12.GUEST_CR3.  At a minimum, GUEST_CR3 should be checked in
> nested_vmx_check_guest_state().  It also feels like vcpu->arch.cr3 should be set
> immediately, e.g. KVM_SET_NESTED_STATE -> KVM_GET_SREGS should reflect L2's CR3
> even if KVM_RUN hasn't been invoked.

Note that KVM_SET_NESTED_STATE does not remove the need to invoke 
KVM_SET_SREGS.  Calling KVM_SET_NESTED_STATE does not necessarily saying 
anything about the value of KVM_GET_SREGS after it.

In particular on SVM it's a "feature" that KVM_SET_NESTED_STATE does not 
include any guest register state; the nested state only includes the 
VMCB12 control state and the L1 save state.  But thinking more about it, 
loading the PDPTRs for the guest CR3 might not be advisable even upon 
KVM_SET_SREGS, and we might want to extend KVM_REQ_GET_NESTED_PAGES to 
cover non-nested PDPTRs as well.

Paolo