Explicitly suppress userspace emulated MMIO exits that are triggered when
emulating a task switch as KVM doesn't support userspace MMIO during
complex (multi-step) emulation.  Silently ignoring the exit request can
result in the WARN_ON_ONCE(vcpu->mmio_needed) firing if KVM exits to
userspace for some other reason prior to purging mmio_needed.

See commit 0dc902267cb3 ("KVM: x86: Suppress pending MMIO write exits if
emulator detects exception") for more details on KVM's limitations with
respect to emulated MMIO during complex emulator flows.

Reported-by: syzbot+2fb9f8ed752c01bc9a3f@syzkaller.appspotmail.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

This is from a syzkaller report on a Google-internal kernel, but it repros
on upstream (obviously).  There are unfortunately an absurd number of upstream
reports with "WARNING in kvm_arch_vcpu_ioctl_run" as the title, so I haven't
been able to hunt down an upstream report.

 arch/x86/kvm/x86.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 994743266480..47bd8a9fdb21 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11803,7 +11803,13 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index,
 
 	ret = emulator_task_switch(ctxt, tss_selector, idt_index, reason,
 				   has_error_code, error_code);
-	if (ret) {
+
+	/*
+	 * Report an error userspace if MMIO is needed, as KVM doesn't support
+	 * MMIO during a task switch (or any other complex operation).
+	 */
+	if (ret || vcpu->mmio_needed) {
+		vcpu->mmio_needed = false;
 		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
 		vcpu->run->internal.ndata = 0;

base-commit: 771df9ffadb8204e61d3e98f36c5067102aab78f
-- 
2.45.2.993.g49e7a77208-goog

Queued, thanks.

Paolo

On Fri, Jul 12, 2024 at 07:48:41AM -0700, Sean Christopherson wrote:

[...]

> See commit 0dc902267cb3 ("KVM: x86: Suppress pending MMIO write exits if
> emulator detects exception") for more details on KVM's limitations with
> respect to emulated MMIO during complex emulator flows.
> 

I try to understand the changelog of commit 0dc902267cb3 but I’m confused with
the MMIO read. The commit said, "For MMIO reads, KVM immediately exits to
userspace upon detecting MMIO as userspace provides the to-be-read value in a
buffer, and so KVM can safely (more or less) restart the instruction from the
beginning." But in read_emulated(), mc->end is adjusted after checking rc,
i.e., although the value will be saved in the buffer, mc->end is not adjusted
after existing to userspace.

Maybe this would really support a buffer for multiple MMIO read instructions
(e.g. POPA)?

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 5d4c86133453..841d5b6f21b0 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1367,8 +1367,11 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt,
 
 	rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
 				      &ctxt->exception);
-	if (rc != X86EMUL_CONTINUE)
+	if (rc != X86EMUL_CONTINUE) {
+		if (rc == X86EMUL_IO_NEEDED)
+			mc->end += size;
 		return rc;
+	}
 
 	mc->end += size;

[...]