KVM: x86: Bug the VM if the emulator accesses a non-existent GPR

Bug the VM, i.e. kill it, if the emulator accesses a non-existent GPR,
i.e. generates an out-of-bounds GPR index.  Continuing on all but
gaurantees some form of data corruption in the guest, e.g. even if KVM
were to redirect to a dummy register, KVM would be incorrectly read zeros
and drop writes.

Note, bugging the VM doesn't completely prevent data corruption, e.g. the
current round of emulation will complete before the vCPU bails out to
userspace.  But, the very act of killing the guest can also cause data
corruption, e.g. due to lack of file writeback before termination, so
taking on additional complexity to cleanly bail out of the emulator isn't
justified, the goal is purely to stem the bleeding and alert userspace
that something has gone horribly wrong, i.e. to avoid _silent_ data
corruption.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20220526210817.3428868-7-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 77161f57c8d3cb26d7782ca8d672b521652a40d2..70a8e0cd9fdc233ac13af86ce32123e992f32497 100644 (file)
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -247,7 +247,7 @@ enum x86_transfer_type {
 
 static ulong reg_read(struct x86_emulate_ctxt *ctxt, unsigned nr)
 {
-       if (WARN_ON_ONCE(nr >= NR_EMULATOR_GPRS))
+       if (KVM_EMULATOR_BUG_ON(nr >= NR_EMULATOR_GPRS, ctxt))
                nr &= NR_EMULATOR_GPRS - 1;
 
        if (!(ctxt->regs_valid & (1 << nr))) {
@@ -259,7 +259,7 @@ static ulong reg_read(struct x86_emulate_ctxt *ctxt, unsigned nr)
 
 static ulong *reg_write(struct x86_emulate_ctxt *ctxt, unsigned nr)
 {
-       if (WARN_ON_ONCE(nr >= NR_EMULATOR_GPRS))
+       if (KVM_EMULATOR_BUG_ON(nr >= NR_EMULATOR_GPRS, ctxt))
                nr &= NR_EMULATOR_GPRS - 1;
 
        BUILD_BUG_ON(sizeof(ctxt->regs_dirty) * BITS_PER_BYTE < NR_EMULATOR_GPRS);

diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 034c845b3c63d6489a4616ca00e06ca501fd50dc..89246446d6aa9d5254a603a29163f7aa62547896 100644 (file)
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -89,6 +89,7 @@ struct x86_instruction_info {
 #define X86EMUL_INTERCEPTED     6 /* Intercepted by nested VMCB/VMCS */
 
 struct x86_emulate_ops {
+       void (*vm_bugged)(struct x86_emulate_ctxt *ctxt);
        /*
         * read_gpr: read a general purpose register (rax - r15)
         *
@@ -383,6 +384,15 @@ struct x86_emulate_ctxt {
        bool is_branch;
 };
 
+#define KVM_EMULATOR_BUG_ON(cond, ctxt)                \
+({                                             \
+       int __ret = (cond);                     \
+                                               \
+       if (WARN_ON_ONCE(__ret))                \
+               ctxt->ops->vm_bugged(ctxt);     \
+       unlikely(__ret);                        \
+})
+
 /* Repeat String Operation Prefix */
 #define REPE_PREFIX    0xf3
 #define REPNE_PREFIX   0xf2

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e8177d8b378e9031da95efdf8f9f103640f8259b..a0baf49bb00d087a785a12a96ebfaf757d313ed3 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7915,7 +7915,16 @@ static int emulator_set_xcr(struct x86_emulate_ctxt *ctxt, u32 index, u64 xcr)
        return __kvm_set_xcr(emul_to_vcpu(ctxt), index, xcr);
 }
 
+static void emulator_vm_bugged(struct x86_emulate_ctxt *ctxt)
+{
+       struct kvm *kvm = emul_to_vcpu(ctxt)->kvm;
+
+       if (!kvm->vm_bugged)
+               kvm_vm_bugged(kvm);
+}
+
 static const struct x86_emulate_ops emulate_ops = {
+       .vm_bugged           = emulator_vm_bugged,
        .read_gpr            = emulator_read_gpr,
        .write_gpr           = emulator_write_gpr,
        .read_std            = emulator_read_std,