Skip to content

Commit 52e322e

Browse files
sean-jcbonzini
authored andcommitted
KVM: x86/mmu: BUG() in rmap helpers iff CONFIG_BUG_ON_DATA_CORRUPTION=y
Introduce KVM_BUG_ON_DATA_CORRUPTION() and use it in the low-level rmap helpers to convert the existing BUG()s to WARN_ON_ONCE() when the kernel is built with CONFIG_BUG_ON_DATA_CORRUPTION=n, i.e. does NOT want to BUG() on corruption of host kernel data structures. Environments that don't have infrastructure to automatically capture crash dumps, i.e. aren't likely to enable CONFIG_BUG_ON_DATA_CORRUPTION=y, are typically better served overall by WARN-and-continue behavior (for the kernel, the VM is dead regardless), as a BUG() while holding mmu_lock all but guarantees the _best_ case scenario is a panic(). Make the BUG()s conditional instead of removing/replacing them entirely as there's a non-zero chance (though by no means a guarantee) that the damage isn't contained to the target VM, e.g. if no rmap is found for a SPTE then KVM may be double-zapping the SPTE, i.e. has already freed the memory the SPTE pointed at and thus KVM is reading/writing memory that KVM no longer owns. Link: https://lore.kernel.org/all/20221129191237.31447-1-mizhang@google.com Suggested-by: Mingwei Zhang <mizhang@google.com> Cc: David Matlack <dmatlack@google.com> Cc: Jim Mattson <jmattson@google.com> Reviewed-by: Mingwei Zhang <mizhang@google.com> Link: https://lore.kernel.org/r/20230729004722.1056172-13-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
1 parent 069f30c commit 52e322e

2 files changed

Lines changed: 29 additions & 11 deletions

File tree

arch/x86/kvm/mmu/mmu.c

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ static void pte_list_desc_remove_entry(struct kvm *kvm,
973973
* when adding an entry and the previous head is full, and heads are
974974
* removed (this flow) when they become empty.
975975
*/
976-
BUG_ON(j < 0);
976+
KVM_BUG_ON_DATA_CORRUPTION(j < 0, kvm);
977977

978978
/*
979979
* Replace the to-be-freed SPTE with the last valid entry from the head
@@ -1004,14 +1004,13 @@ static void pte_list_remove(struct kvm *kvm, u64 *spte,
10041004
struct pte_list_desc *desc;
10051005
int i;
10061006

1007-
if (!rmap_head->val) {
1008-
pr_err("%s: %p 0->BUG\n", __func__, spte);
1009-
BUG();
1010-
} else if (!(rmap_head->val & 1)) {
1011-
if ((u64 *)rmap_head->val != spte) {
1012-
pr_err("%s: %p 1->BUG\n", __func__, spte);
1013-
BUG();
1014-
}
1007+
if (KVM_BUG_ON_DATA_CORRUPTION(!rmap_head->val, kvm))
1008+
return;
1009+
1010+
if (!(rmap_head->val & 1)) {
1011+
if (KVM_BUG_ON_DATA_CORRUPTION((u64 *)rmap_head->val != spte, kvm))
1012+
return;
1013+
10151014
rmap_head->val = 0;
10161015
} else {
10171016
desc = (struct pte_list_desc *)(rmap_head->val & ~1ul);
@@ -1025,8 +1024,8 @@ static void pte_list_remove(struct kvm *kvm, u64 *spte,
10251024
}
10261025
desc = desc->more;
10271026
}
1028-
pr_err("%s: %p many->many\n", __func__, spte);
1029-
BUG();
1027+
1028+
KVM_BUG_ON_DATA_CORRUPTION(true, kvm);
10301029
}
10311030
}
10321031

include/linux/kvm_host.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,25 @@ static inline void kvm_vm_bugged(struct kvm *kvm)
867867
unlikely(__ret); \
868868
})
869869

870+
/*
871+
* Note, "data corruption" refers to corruption of host kernel data structures,
872+
* not guest data. Guest data corruption, suspected or confirmed, that is tied
873+
* and contained to a single VM should *never* BUG() and potentially panic the
874+
* host, i.e. use this variant of KVM_BUG() if and only if a KVM data structure
875+
* is corrupted and that corruption can have a cascading effect to other parts
876+
* of the hosts and/or to other VMs.
877+
*/
878+
#define KVM_BUG_ON_DATA_CORRUPTION(cond, kvm) \
879+
({ \
880+
bool __ret = !!(cond); \
881+
\
882+
if (IS_ENABLED(CONFIG_BUG_ON_DATA_CORRUPTION)) \
883+
BUG_ON(__ret); \
884+
else if (WARN_ON_ONCE(__ret && !(kvm)->vm_bugged)) \
885+
kvm_vm_bugged(kvm); \
886+
unlikely(__ret); \
887+
})
888+
870889
static inline void kvm_vcpu_srcu_read_lock(struct kvm_vcpu *vcpu)
871890
{
872891
#ifdef CONFIG_PROVE_RCU

0 commit comments

Comments
 (0)