linux-kernel - Re: [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable

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: <87zhefsugj.fsf@vitty.brq.redhat.com>
Date:   Wed, 22 Jan 2020 10:37:16 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, Jim Mattson <jmattson@...gle.com>,
        linux-kernel@...r.kernel.org, Liran Alon <liran.alon@...cle.com>,
        Roman Kagan <rkagan@...tuozzo.com>
Subject: Re: [PATCH RFC 2/3] x86/kvm/hyper-v: move VMX controls sanitization out of nested_enable_evmcs()

Sean Christopherson <sean.j.christopherson@...el.com> writes:

> On Sat, Jan 18, 2020 at 10:42:31PM +0100, Paolo Bonzini wrote:
>> IMHO the features should stay available in case the guest chooses not to
>> use eVMCS.  A guest that uses eVMCS should know which features it cannot
>> use and not enable them.
>
> Makes sense, wasn't thinking about the scenario where the guest doesn't
> support eVMCS or doesn't want to use it for whatever reason.
>
> Rather than update vmx->nested.msrs or filter vmx_get_msr(), what about
> manually adding eVMCS consistency checks on the disallowed bits and handle
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES as a one-off case by simply
> clearing it from the eVMCS?

Unfortunately, this doesn't work because ... Windows. Not only Hyper-V
enables SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES, it actually expects it
to work (somehow) so when I do

diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index 72359709cdc1..e6c30eec2817 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -361,11 +361,5 @@ int nested_enable_evmcs(struct kvm_vcpu *vcpu,
        if (evmcs_already_enabled)
                return 0;
 
-       vmx->nested.msrs.pinbased_ctls_high &= ~EVMCS1_UNSUPPORTED_PINCTRL;
-       vmx->nested.msrs.entry_ctls_high &= ~EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
-       vmx->nested.msrs.exit_ctls_high &= ~EVMCS1_UNSUPPORTED_VMEXIT_CTRL;
-       vmx->nested.msrs.secondary_ctls_high &= ~EVMCS1_UNSUPPORTED_2NDEXEC;
-       vmx->nested.msrs.vmfunc_controls &= ~EVMCS1_UNSUPPORTED_VMFUNC;
-
        return 0;
 }
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index bb8afe0c5e7f..cd1f5a1c884b 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -1590,7 +1590,7 @@ static int copy_enlightened_to_vmcs12(struct vcpu_vmx *vmx)
                        evmcs->pin_based_vm_exec_control;
                vmcs12->vm_exit_controls = evmcs->vm_exit_controls;
                vmcs12->secondary_vm_exec_control =
-                       evmcs->secondary_vm_exec_control;
+                       evmcs->secondary_vm_exec_control & ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
        }
 
        if (unlikely(!(evmcs->hv_clean_fields &

Hyper-V 2016 with > 1 vCPU fails to boot :-(

-- 
Vitaly