All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	Robert Dinse <nanook@eskimo.com>
Subject: [PATCH v2 6/8] KVM: x86: Bug the VM if the emulator accesses a non-existent GPR
Date: Thu, 26 May 2022 21:08:15 +0000	[thread overview]
Message-ID: <20220526210817.3428868-7-seanjc@google.com> (raw)
In-Reply-To: <20220526210817.3428868-1-seanjc@google.com>

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>
---
 arch/x86/kvm/emulate.c     |  4 ++--
 arch/x86/kvm/kvm_emulate.h | 10 ++++++++++
 arch/x86/kvm/x86.c         |  9 +++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 77161f57c8d3..70a8e0cd9fdc 100644
--- 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 034c845b3c63..89246446d6aa 100644
--- 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 7460b9a77d9a..e60badfbbc42 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7887,7 +7887,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,
-- 
2.36.1.255.ge46751e96f-goog


  parent reply	other threads:[~2022-05-26 21:25 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26 21:08 [PATCH v2 0/8] KVM: x86: Emulator _regs fixes and cleanups Sean Christopherson
2022-05-26 21:08 ` [PATCH v2 1/8] KVM: x86: Grab regs_dirty in local 'unsigned long' Sean Christopherson
2022-05-26 21:08 ` [PATCH v2 2/8] KVM: x86: Harden _regs accesses to guard against buggy input Sean Christopherson
2022-05-26 21:08 ` [PATCH v2 3/8] KVM: x86: Omit VCPU_REGS_RIP from emulator's _regs array Sean Christopherson
2022-05-31 18:04   ` Kees Cook
2022-05-26 21:08 ` [PATCH v2 4/8] KVM: x86: Use 16-bit fields to track dirty/valid emulator GPRs Sean Christopherson
2022-05-26 21:08 ` [PATCH v2 5/8] KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM Sean Christopherson
2022-05-31 18:04   ` Kees Cook
2022-05-26 21:08 ` Sean Christopherson [this message]
2022-05-31 18:06   ` [PATCH v2 6/8] KVM: x86: Bug the VM if the emulator accesses a non-existent GPR Kees Cook
2022-06-01  8:29   ` Vitaly Kuznetsov
2022-06-02 16:01     ` Sean Christopherson
2022-05-26 21:08 ` [PATCH v2 7/8] KVM: x86: Bug the VM if the emulator generates a bogus exception vector Sean Christopherson
2022-05-31 18:06   ` Kees Cook
2022-06-01  8:31   ` Vitaly Kuznetsov
2022-05-26 21:08 ` [PATCH v2 8/8] KVM: x86: Bug the VM on an out-of-bounds data read Sean Christopherson
2022-05-31 18:06   ` Kees Cook
2022-06-01  8:39   ` Vitaly Kuznetsov
2022-05-26 23:14 ` [PATCH v2 0/8] KVM: x86: Emulator _regs fixes and cleanups Robert Dinse
2022-06-08 12:23 ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220526210817.3428868-7-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nanook@eskimo.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.