linux-kernel - Re: [PATCH] KVM: nVMX: cleanup and fix host 64-bit mode checks

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: <1543003a-9a2f-2a52-444a-d55bde6b8e2f@redhat.com>
Date:   Thu, 26 Sep 2019 10:42:38 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Krish Sadhukhan <krish.sadhukhan@...cle.com>,
        Jim Mattson <jmattson@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        kvm list <kvm@...r.kernel.org>,
        Sean Christopherson <sean.j.christopherson@...el.com>
Subject: Re: [PATCH] KVM: nVMX: cleanup and fix host 64-bit mode checks

On 26/09/19 01:55, Krish Sadhukhan wrote:
> 
> 
> On 09/25/2019 09:47 AM, Jim Mattson wrote:
>> On Wed, Sep 25, 2019 at 9:34 AM Paolo Bonzini <pbonzini@...hat.com>
>> wrote:
>>> KVM was incorrectly checking vmcs12->host_ia32_efer even if the "load
>>> IA32_EFER" exit control was reset.  Also, some checks were not using
>>> the new CC macro for tracing.
>>>
>>> Cleanup everything so that the vCPU's 64-bit mode is determined
>>> directly from EFER_LMA and the VMCS checks are based on that, which
>>> matches section 26.2.4 of the SDM.
>>>
>>> Cc: Sean Christopherson <sean.j.christopherson@...el.com>
>>> Cc: Jim Mattson <jmattson@...gle.com>
>>> Cc: Krish Sadhukhan <krish.sadhukhan@...cle.com>
>>> Fixes: 5845038c111db27902bc220a4f70070fe945871c
>>> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
>>> ---
>>>   arch/x86/kvm/vmx/nested.c | 53
>>> ++++++++++++++++++++---------------------------
>>>   1 file changed, 22 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>>> index 70d59d9304f2..e108847f6cf8 100644
>>> --- a/arch/x86/kvm/vmx/nested.c
>>> +++ b/arch/x86/kvm/vmx/nested.c
>>> @@ -2664,8 +2664,26 @@ static int nested_vmx_check_host_state(struct
>>> kvm_vcpu *vcpu,
>>>              CC(!kvm_pat_valid(vmcs12->host_ia32_pat)))
>>>                  return -EINVAL;
>>>
>>> -       ia32e = (vmcs12->vm_exit_controls &
>>> -                VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
>>> +#ifdef CONFIG_X86_64
>>> +       ia32e = !!(vcpu->arch.efer & EFER_LMA);
>>> +#else
>>> +       if (CC(vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE))
>>> +               return -EINVAL;
>> This check is redundant, since it is checked in the else block below.
> 
> Should we be re-using is_long_mode() instead of duplicating the code ?

Of course!  I have already pushed the patch, but I will send a follow up.

Paolo