| 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
| ||
|
Message-ID: <20240927001635.501418-5-seanjc@google.com>
Date: Thu, 26 Sep 2024 17:16:35 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>, Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
Kai Huang <kai.huang@...el.com>, Rick Edgecombe <rick.p.edgecombe@...el.com>,
Yan Zhao <yan.y.zhao@...el.com>
Subject: [PATCH 4/4] Revert "KVM: x86/mmu: Introduce a quirk to control
memslot zap behavior"
Remove KVM_X86_QUIRK_SLOT_ZAP_ALL, as the code is broken for shadow MMUs,
and the underlying premise is dodgy.
As was tried in commit 4e103134b862 ("KVM: x86/mmu: Zap only the relevant
pages when removing a memslot"), all shadow pages, i.e. non-leaf SPTEs,
need to be zapped. All of the accounting for a shadow page is tied to the
memslot, i.e. the shadow page holds a reference to the memslot, for all
intents and purposes. Deleting the memslot without removing all relevant
shadow pages, as is done when KVM_X86_QUIRK_SLOT_ZAP_ALL is disabled,
results in NULL pointer derefs when tearing down the VM.
BUG: kernel NULL pointer dereference, address: 00000000000000b0
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 6085f43067 P4D 608c080067 PUD 608c081067 PMD 0
Oops: Oops: 0000 [#1] SMP NOPTI
CPU: 79 UID: 0 PID: 187063 Comm: set_memory_regi Tainted: G W 6.11.0-smp--24867312d167-cpl #395
Tainted: [W]=WARN
Hardware name: Google Astoria/astoria, BIOS 0.20240617.0-0 06/17/2024
RIP: 0010:__kvm_mmu_prepare_zap_page+0x3a9/0x7b0 [kvm]
Code: <48> 8b 8e b0 00 00 00 48 8b 96 e0 00 00 00 48 c1 e9 09 48 29 c8 8b
RSP: 0018:ff314a25b19f7c28 EFLAGS: 00010212
Call Trace:
<TASK>
kvm_arch_flush_shadow_all+0x7a/0xf0 [kvm]
kvm_mmu_notifier_release+0x6c/0xb0 [kvm]
mmu_notifier_unregister+0x85/0x140
kvm_put_kvm+0x263/0x410 [kvm]
kvm_vm_release+0x21/0x30 [kvm]
__fput+0x8d/0x2c0
__se_sys_close+0x71/0xc0
do_syscall_64+0x83/0x160
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Rather than trying to get things functional for shadow MMUs (which
includes nested TDP), scrap the quirk idea, at least for now. In addition
to the function bug, it's not clear that unconditionally doing a targeted
zap for all non-default VM types is actually desirable. E.g. it's entirely
possible that SEV-ES and SNP VMs would exhibit worse performance than KVM's
current "zap all" behavior, or that it's better to do a targeted zap only
in specific situations, etc.
This reverts commit aa8d1f48d353b0469bff357183ee9df137d15ef0.
Cc: Kai Huang <kai.huang@...el.com>
Cc: Rick Edgecombe <rick.p.edgecombe@...el.com>
Cc: Yan Zhao <yan.y.zhao@...el.com>
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
Documentation/virt/kvm/api.rst | 8 --------
arch/x86/include/asm/kvm_host.h | 3 +--
arch/x86/include/uapi/asm/kvm.h | 1 -
arch/x86/kvm/mmu/mmu.c | 34 +--------------------------------
4 files changed, 2 insertions(+), 44 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e32471977d0a..a4b7dc4a9dda 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8097,14 +8097,6 @@ KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS By default, KVM emulates MONITOR/MWAIT (if
guest CPUID on writes to MISC_ENABLE if
KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT is
disabled.
-
-KVM_X86_QUIRK_SLOT_ZAP_ALL By default, KVM invalidates all SPTEs in
- fast way for memslot deletion when VM type
- is KVM_X86_DEFAULT_VM.
- When this quirk is disabled or when VM type
- is other than KVM_X86_DEFAULT_VM, KVM zaps
- only leaf SPTEs that are within the range of
- the memslot being deleted.
=================================== ============================================
7.32 KVM_CAP_MAX_VCPU_ID
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 6d9f763a7bb9..4738f6f5a794 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2358,8 +2358,7 @@ int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages);
KVM_X86_QUIRK_OUT_7E_INC_RIP | \
KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT | \
KVM_X86_QUIRK_FIX_HYPERCALL_INSN | \
- KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS | \
- KVM_X86_QUIRK_SLOT_ZAP_ALL)
+ KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS)
/*
* KVM previously used a u32 field in kvm_run to indicate the hypercall was
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index a8debbf2f702..bf57a824f722 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -439,7 +439,6 @@ struct kvm_sync_regs {
#define KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT (1 << 4)
#define KVM_X86_QUIRK_FIX_HYPERCALL_INSN (1 << 5)
#define KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS (1 << 6)
-#define KVM_X86_QUIRK_SLOT_ZAP_ALL (1 << 7)
#define KVM_STATE_NESTED_FORMAT_VMX 0
#define KVM_STATE_NESTED_FORMAT_SVM 1
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index e081f785fb23..0d94354bb2f8 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -7049,42 +7049,10 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
kvm_mmu_zap_all(kvm);
}
-/*
- * Zapping leaf SPTEs with memslot range when a memslot is moved/deleted.
- *
- * Zapping non-leaf SPTEs, a.k.a. not-last SPTEs, isn't required, worst
- * case scenario we'll have unused shadow pages lying around until they
- * are recycled due to age or when the VM is destroyed.
- */
-static void kvm_mmu_zap_memslot_leafs(struct kvm *kvm, struct kvm_memory_slot *slot)
-{
- struct kvm_gfn_range range = {
- .slot = slot,
- .start = slot->base_gfn,
- .end = slot->base_gfn + slot->npages,
- .may_block = true,
- };
-
- write_lock(&kvm->mmu_lock);
- if (kvm_unmap_gfn_range(kvm, &range))
- kvm_flush_remote_tlbs_memslot(kvm, slot);
-
- write_unlock(&kvm->mmu_lock);
-}
-
-static inline bool kvm_memslot_flush_zap_all(struct kvm *kvm)
-{
- return kvm->arch.vm_type == KVM_X86_DEFAULT_VM &&
- kvm_check_has_quirk(kvm, KVM_X86_QUIRK_SLOT_ZAP_ALL);
-}
-
void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
struct kvm_memory_slot *slot)
{
- if (kvm_memslot_flush_zap_all(kvm))
- kvm_mmu_zap_all_fast(kvm);
- else
- kvm_mmu_zap_memslot_leafs(kvm, slot);
+ kvm_mmu_zap_all_fast(kvm);
}
void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, u64 gen)
--
2.46.1.824.gd892dcdcdd-goog
Powered by blists - more mailing lists