From 9319676595a2da8022327119e89c8b13f934835e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 27 Jul 2020 08:45:00 -0400 Subject: selftests: kvm: do not set guest mode flag Setting KVM_STATE_NESTED_GUEST_MODE enables various consistency checks on VMCS12 and therefore causes KVM_SET_NESTED_STATE to fail spuriously with -EINVAL. Do not set the flag so that we're sure to cover the conditions included by the test, and cover the case where VMCS12 is set and KVM_SET_NESTED_STATE is called with invalid VMCS12 contents. Signed-off-by: Paolo Bonzini --- .../selftests/kvm/x86_64/vmx_set_nested_state_test.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c index 54cdefdfb49d..d14a34f1b018 100644 --- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c @@ -76,10 +76,8 @@ void set_default_state(struct kvm_nested_state *state) void set_default_vmx_state(struct kvm_nested_state *state, int size) { memset(state, 0, size); - state->flags = KVM_STATE_NESTED_GUEST_MODE | - KVM_STATE_NESTED_RUN_PENDING; if (have_evmcs) - state->flags |= KVM_STATE_NESTED_EVMCS; + state->flags = KVM_STATE_NESTED_EVMCS; state->format = 0; state->size = size; state->hdr.vmx.vmxon_pa = 0x1000; @@ -190,17 +188,20 @@ void test_vmx_nested_state(struct kvm_vm *vm) state->size = sizeof(*state); test_nested_state(vm, state); + /* + * KVM_SET_NESTED_STATE succeeds with invalid VMCS + * contents but L2 not running. + */ + set_default_vmx_state(state, state_sz); + state->flags = 0; + test_nested_state(vm, state); + /* vmxon_pa cannot be the same address as vmcs_pa. */ set_default_vmx_state(state, state_sz); state->hdr.vmx.vmxon_pa = 0; state->hdr.vmx.vmcs12_pa = 0; test_nested_state_expect_einval(vm, state); - /* The revision id for vmcs12 must be VMCS12_REVISION. */ - set_default_vmx_state(state, state_sz); - set_revision_id_for_vmcs12(state, 0); - test_nested_state_expect_einval(vm, state); - /* * Test that if we leave nesting the state reflects that when we get * it again. -- cgit From 0f02bd0ade9a552492463c0159abbe26c4d92b40 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 27 Jul 2020 09:00:37 -0400 Subject: KVM: nVMX: check for required but missing VMCS12 in KVM_SET_NESTED_STATE A missing VMCS12 was not causing -EINVAL (it was just read with copy_from_user, so it is not a security issue, but it is still wrong). Test for VMCS12 validity and reject the nested state if a VMCS12 is required but not present. Signed-off-by: Paolo Bonzini --- arch/x86/kvm/vmx/nested.c | 13 ++++++++++--- arch/x86/kvm/vmx/nested.h | 5 +++++ .../selftests/kvm/x86_64/vmx_set_nested_state_test.c | 12 +++++++++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index d4a4cec034d0..6a0e32a7418c 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -6108,9 +6108,16 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, if (ret) return ret; - /* Empty 'VMXON' state is permitted */ - if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) - return 0; + /* Empty 'VMXON' state is permitted if no VMCS loaded */ + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) { + /* See vmx_has_valid_vmcs12. */ + if ((kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE) || + (kvm_state->flags & KVM_STATE_NESTED_EVMCS) || + (kvm_state->hdr.vmx.vmcs12_pa != -1ull)) + return -EINVAL; + else + return 0; + } if (kvm_state->hdr.vmx.vmcs12_pa != -1ull) { if (kvm_state->hdr.vmx.vmcs12_pa == kvm_state->hdr.vmx.vmxon_pa || diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h index 758bccc26cf9..197148d76b8f 100644 --- a/arch/x86/kvm/vmx/nested.h +++ b/arch/x86/kvm/vmx/nested.h @@ -47,6 +47,11 @@ static inline struct vmcs12 *get_shadow_vmcs12(struct kvm_vcpu *vcpu) return to_vmx(vcpu)->nested.cached_shadow_vmcs12; } +/* + * Note: the same condition is checked against the state provided by userspace + * in vmx_set_nested_state; if it is satisfied, the nested state must include + * the VMCS12. + */ static inline int vmx_has_valid_vmcs12(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c index d14a34f1b018..94f28a657569 100644 --- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c @@ -183,9 +183,19 @@ void test_vmx_nested_state(struct kvm_vm *vm) state->hdr.vmx.smm.flags = KVM_STATE_NESTED_SMM_GUEST_MODE; test_nested_state_expect_einval(vm, state); - /* Size must be large enough to fit kvm_nested_state and vmcs12. */ + /* + * Size must be large enough to fit kvm_nested_state and vmcs12 + * if VMCS12 physical address is set + */ + set_default_vmx_state(state, state_sz); + state->size = sizeof(*state); + state->flags = 0; + test_nested_state_expect_einval(vm, state); + set_default_vmx_state(state, state_sz); state->size = sizeof(*state); + state->flags = 0; + state->hdr.vmx.vmcs12_pa = -1; test_nested_state(vm, state); /* -- cgit From 5e105c88ab4859bc1aedd29e8d2f55e599427035 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 27 Jul 2020 08:55:09 -0400 Subject: KVM: nVMX: check for invalid hdr.vmx.flags hdr.vmx.flags is meant for future extensions to the ABI, rejecting invalid flags is necessary to avoid broken half-loads of the nVMX state. Signed-off-by: Paolo Bonzini --- arch/x86/kvm/vmx/nested.c | 3 +++ .../selftests/kvm/x86_64/vmx_set_nested_state_test.c | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 6a0e32a7418c..11e4df560018 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -6079,6 +6079,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, ~(KVM_STATE_NESTED_SMM_GUEST_MODE | KVM_STATE_NESTED_SMM_VMXON)) return -EINVAL; + if (kvm_state->hdr.vmx.flags & ~KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE) + return -EINVAL; + /* * SMM temporarily disables VMX, so we cannot be in guest mode, * nor can VMLAUNCH/VMRESUME be pending. Outside SMM, SMM flags diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c index 94f28a657569..d59f3eb67c8f 100644 --- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c @@ -146,6 +146,11 @@ void test_vmx_nested_state(struct kvm_vm *vm) state->hdr.vmx.smm.flags = 1; test_nested_state_expect_einval(vm, state); + /* Invalid flags are rejected. */ + set_default_vmx_state(state, state_sz); + state->hdr.vmx.flags = ~0; + test_nested_state_expect_einval(vm, state); + /* It is invalid to have vmxon_pa == -1ull and vmcs_pa != -1ull. */ set_default_vmx_state(state, state_sz); state->hdr.vmx.vmxon_pa = -1ull; @@ -206,6 +211,14 @@ void test_vmx_nested_state(struct kvm_vm *vm) state->flags = 0; test_nested_state(vm, state); + /* Invalid flags are rejected, even if no VMCS loaded. */ + set_default_vmx_state(state, state_sz); + state->size = sizeof(*state); + state->flags = 0; + state->hdr.vmx.vmcs12_pa = -1; + state->hdr.vmx.flags = ~0; + test_nested_state_expect_einval(vm, state); + /* vmxon_pa cannot be the same address as vmcs_pa. */ set_default_vmx_state(state, state_sz); state->hdr.vmx.vmxon_pa = 0; -- cgit From bf4086b1a1efa3d3a2c17582e00bbd2176dfe177 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Wed, 22 Jul 2020 17:22:31 +0100 Subject: KVM: arm64: Prevent vcpu_has_ptrauth from generating OOL functions So far, vcpu_has_ptrauth() is implemented in terms of system_supports_*_auth() calls, which are declared "inline". In some specific conditions (clang and SCS), the "inline" very much turns into an "out of line", which leads to a fireworks when this predicate is evaluated on a non-VHE system (right at the beginning of __hyp_handle_ptrauth). Instead, make sure vcpu_has_ptrauth gets expanded inline by directly using the cpus_have_final_cap() helpers, which are __always_inline, generate much better code, and are the only thing that make sense when running at EL2 on a nVHE system. Fixes: 29eb5a3c57f7 ("KVM: arm64: Handle PtrAuth traps early") Reported-by: Nathan Chancellor Reported-by: Nick Desaulniers Signed-off-by: Marc Zyngier Tested-by: Nathan Chancellor Reviewed-by: Nathan Chancellor Link: https://lore.kernel.org/r/20200722162231.3689767-1-maz@kernel.org --- arch/arm64/include/asm/kvm_host.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index c3e6fcc664b1..e21d4a01372f 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -380,9 +380,14 @@ struct kvm_vcpu_arch { #define vcpu_has_sve(vcpu) (system_supports_sve() && \ ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE)) -#define vcpu_has_ptrauth(vcpu) ((system_supports_address_auth() || \ - system_supports_generic_auth()) && \ - ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_PTRAUTH)) +#ifdef CONFIG_ARM64_PTR_AUTH +#define vcpu_has_ptrauth(vcpu) \ + ((cpus_have_final_cap(ARM64_HAS_ADDRESS_AUTH) || \ + cpus_have_final_cap(ARM64_HAS_GENERIC_AUTH)) && \ + (vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_PTRAUTH) +#else +#define vcpu_has_ptrauth(vcpu) false +#endif #define vcpu_gp_regs(v) (&(v)->arch.ctxt.gp_regs) -- cgit From b757b47a2fcba584d4a32fd7ee68faca510ab96f Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Thu, 23 Jul 2020 11:17:14 +0100 Subject: KVM: arm64: Don't inherit exec permission across page-table levels If a stage-2 page-table contains an executable, read-only mapping at the pte level (e.g. due to dirty logging being enabled), a subsequent write fault to the same page which tries to install a larger block mapping (e.g. due to dirty logging having been disabled) will erroneously inherit the exec permission and consequently skip I-cache invalidation for the rest of the block. Ensure that exec permission is only inherited by write faults when the new mapping is of the same size as the existing one. A subsequent instruction abort will result in I-cache invalidation for the entire block mapping. Signed-off-by: Will Deacon Signed-off-by: Marc Zyngier Tested-by: Quentin Perret Reviewed-by: Quentin Perret Cc: Marc Zyngier Cc: Link: https://lore.kernel.org/r/20200723101714.15873-1-will@kernel.org --- arch/arm64/kvm/mmu.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 8c0035cab6b6..31058e6e7c2a 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -1326,7 +1326,7 @@ static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr, return true; } -static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr) +static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr, unsigned long sz) { pud_t *pudp; pmd_t *pmdp; @@ -1338,11 +1338,11 @@ static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr) return false; if (pudp) - return kvm_s2pud_exec(pudp); + return sz <= PUD_SIZE && kvm_s2pud_exec(pudp); else if (pmdp) - return kvm_s2pmd_exec(pmdp); + return sz <= PMD_SIZE && kvm_s2pmd_exec(pmdp); else - return kvm_s2pte_exec(ptep); + return sz == PAGE_SIZE && kvm_s2pte_exec(ptep); } static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache, @@ -1958,7 +1958,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, * execute permissions, and we preserve whatever we have. */ needs_exec = exec_fault || - (fault_status == FSC_PERM && stage2_is_exec(kvm, fault_ipa)); + (fault_status == FSC_PERM && + stage2_is_exec(kvm, fault_ipa, vma_pagesize)); if (vma_pagesize == PUD_SIZE) { pud_t new_pud = kvm_pfn_pud(pfn, mem_type); -- cgit From d2286ba7d574ba3103a421a2f9ec17cb5b0d87a1 Mon Sep 17 00:00:00 2001 From: Wanpeng Li Date: Fri, 31 Jul 2020 11:12:19 +0800 Subject: KVM: LAPIC: Prevent setting the tscdeadline timer if the lapic is hw disabled Prevent setting the tscdeadline timer if the lapic is hw disabled. Fixes: bce87cce88 (KVM: x86: consolidate different ways to test for in-kernel LAPIC) Cc: Signed-off-by: Wanpeng Li Message-Id: <1596165141-28874-1-git-send-email-wanpengli@tencent.com> Signed-off-by: Paolo Bonzini --- arch/x86/kvm/lapic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 5bf72fc86a8e..4ce2ddd26c0b 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2195,7 +2195,7 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data) { struct kvm_lapic *apic = vcpu->arch.apic; - if (!lapic_in_kernel(vcpu) || apic_lvtt_oneshot(apic) || + if (!kvm_apic_present(vcpu) || apic_lvtt_oneshot(apic) || apic_lvtt_period(apic)) return; -- cgit From 830f01b089b12bbe93bd55f2d62837253012a30e Mon Sep 17 00:00:00 2001 From: Wanpeng Li Date: Fri, 31 Jul 2020 11:12:21 +0800 Subject: KVM: SVM: Fix disable pause loop exit/pause filtering capability on SVM 'Commit 8566ac8b8e7c ("KVM: SVM: Implement pause loop exit logic in SVM")' drops disable pause loop exit/pause filtering capability completely, I guess it is a merge fault by Radim since disable vmexits capabilities and pause loop exit for SVM patchsets are merged at the same time. This patch reintroduces the disable pause loop exit/pause filtering capability support. Reported-by: Haiwei Li Tested-by: Haiwei Li Fixes: 8566ac8b ("KVM: SVM: Implement pause loop exit logic in SVM") Signed-off-by: Wanpeng Li Message-Id: <1596165141-28874-3-git-send-email-wanpengli@tencent.com> Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini --- arch/x86/kvm/svm/svm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index c0da4dd78ac5..5bbf76189afa 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -1090,7 +1090,7 @@ static void init_vmcb(struct vcpu_svm *svm) svm->nested.vmcb = 0; svm->vcpu.arch.hflags = 0; - if (pause_filter_count) { + if (!kvm_pause_in_guest(svm->vcpu.kvm)) { control->pause_filter_count = pause_filter_count; if (pause_filter_thresh) control->pause_filter_thresh = pause_filter_thresh; @@ -2693,7 +2693,7 @@ static int pause_interception(struct vcpu_svm *svm) struct kvm_vcpu *vcpu = &svm->vcpu; bool in_kernel = (svm_get_cpl(vcpu) == 0); - if (pause_filter_thresh) + if (!kvm_pause_in_guest(vcpu->kvm)) grow_ple_window(vcpu); kvm_vcpu_on_spin(vcpu, in_kernel); @@ -3780,7 +3780,7 @@ static void svm_handle_exit_irqoff(struct kvm_vcpu *vcpu) static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu) { - if (pause_filter_thresh) + if (!kvm_pause_in_guest(vcpu->kvm)) shrink_ple_window(vcpu); } @@ -3958,6 +3958,9 @@ static void svm_vm_destroy(struct kvm *kvm) static int svm_vm_init(struct kvm *kvm) { + if (!pause_filter_count || !pause_filter_thresh) + kvm->arch.pause_in_guest = true; + if (avic) { int ret = avic_vm_init(kvm); if (ret) -- cgit