linux-kernel - [PATCH 2/4] KVM: x86: Inject UD when fetching from MMIO

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240923141810.76331-3-iorlov@amazon.com>
Date: Mon, 23 Sep 2024 14:18:08 +0000
From: Ivan Orlov <iorlov@...zon.com>
To: <hpa@...or.com>, <bp@...en8.de>, <dave.hansen@...ux.intel.com>,
	<mingo@...hat.com>, <pbonzini@...hat.com>, <seanjc@...gle.com>,
	<shuah@...nel.org>, <tglx@...utronix.de>
CC: Ivan Orlov <iorlov@...zon.com>, <jalliste@...zon.com>,
	<nh-open-source@...zon.com>, <kvm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
	<x86@...nel.org>
Subject: [PATCH 2/4] KVM: x86: Inject UD when fetching from MMIO

Currently, we simply return a KVM internal error with suberror =
KVM_INTERNAL_ERROR_EMULATION if the guest tries to fetch instruction
from MMIO range as we simply can't decode it.

I believe it is not the best thing to do, considering that

  1) we don't give enough information to VMM about the issue we faced
  2) the issue is triggered by the guest itself, so it is not the KVM
     "internal" error.

Inject the #UD into the guest instead and resume it's execution without
giving an error to VMM, as it would be if we can't find a valid
instruction at MMIO address.

Signed-off-by: Ivan Orlov <iorlov@...zon.com>
---
 arch/x86/kvm/emulate.c     | 3 +++
 arch/x86/kvm/kvm_emulate.h | 1 +
 arch/x86/kvm/x86.c         | 7 ++++++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e72aed25d721..d610c47fa1f4 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4742,10 +4742,13 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
 	ctxt->fetch.end = ctxt->fetch.data + insn_len;
 	ctxt->opcode_len = 1;
 	ctxt->intercept = x86_intercept_none;
+	ctxt->is_mmio_fetch = false;
 	if (insn_len > 0)
 		memcpy(ctxt->fetch.data, insn, insn_len);
 	else {
 		rc = __do_insn_fetch_bytes(ctxt, 1);
+		if (rc == X86EMUL_IO_NEEDED)
+			ctxt->is_mmio_fetch = true;
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 	}
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 55a18e2f2dcd..46c0d1111ec1 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -362,6 +362,7 @@ struct x86_emulate_ctxt {
 	u8 seg_override;
 	u64 d;
 	unsigned long _eip;
+	bool is_mmio_fetch;
 
 	/* Here begins the usercopy section. */
 	struct operand src;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c983c8e434b8..4fb57280ec7b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8857,7 +8857,12 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
 
 	kvm_queue_exception(vcpu, UD_VECTOR);
 
-	if (!is_guest_mode(vcpu) && kvm_x86_call(get_cpl)(vcpu) == 0) {
+	/*
+	 * Don't return an internal error if the emulation error is caused by a fetch from MMIO
+	 * address. Injecting a #UD should be enough.
+	 */
+	if (!is_guest_mode(vcpu) && kvm_x86_call(get_cpl)(vcpu) == 0 &&
+	    !vcpu->arch.emulate_ctxt->is_mmio_fetch) {
 		prepare_emulation_ctxt_failure_exit(vcpu);
 		return 0;
 	}
-- 
2.43.0