linux-kernel - Re: [PATCH] KVM: x86: Suppress MMIO that is triggered during task switch emulation

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: <ZpTwddPr+4X6CCof@linux.bj.intel.com>
Date: Mon, 15 Jul 2024 17:48:37 +0800
From: Tao Su <tao1.su@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	syzbot+2fb9f8ed752c01bc9a3f@...kaller.appspotmail.com
Subject: Re: [PATCH] KVM: x86: Suppress MMIO that is triggered during task
 switch emulation

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;

[...]