From: Sean Christopherson <seanjc@google.com>
To: Sean Christopherson <seanjc@google.com>,
Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
David Matlack <dmatlack@google.com>,
Pasha Tatashin <tatashin@google.com>,
Michael Krebs <mkrebs@google.com>
Subject: [PATCH 1/2] KVM: x86: Mark target gfn of emulated atomic instruction as dirty
Date: Wed, 14 Feb 2024 17:00:03 -0800 [thread overview]
Message-ID: <20240215010004.1456078-2-seanjc@google.com> (raw)
In-Reply-To: <20240215010004.1456078-1-seanjc@google.com>
When emulating an atomic access on behalf of the guest, mark the target
gfn dirty if the CMPXCHG by KVM is attempted and doesn't fault. This
fixes a bug where KVM effectively corrupts guest memory during live
migration by writing to guest memory without informing userspace that the
page is dirty.
Marking the page dirty got unintentionally dropped when KVM's emulated
CMPXCHG was converted to do a user access. Before that, KVM explicitly
mapped the guest page into kernel memory, and marked the page dirty during
the unmap phase.
Mark the page dirty even if the CMPXCHG fails, as the old data is written
back on failure, i.e. the page is still written. The value written is
guaranteed to be the same because the operation is atomic, but KVM's ABI
is that all writes are dirty logged regardless of the value written. And
more importantly, that's what KVM did before the buggy commit.
Huge kudos to the folks on the Cc list (and many others), who did all the
actual work of triaging and debugging.
Fixes: 1c2361f667f3 ("KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses")
Cc: stable@vger.kernel.org
Cc: David Matlack <dmatlack@google.com>
Cc: Pasha Tatashin <tatashin@google.com>
Cc: Michael Krebs <mkrebs@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b66c45e7f6f8..3ec9781d6122 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8009,6 +8009,16 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
if (r < 0)
return X86EMUL_UNHANDLEABLE;
+
+ /*
+ * Mark the page dirty _before_ checking whether or not the CMPXCHG was
+ * successful, as the old value is written back on failure. Note, for
+ * live migration, this is unnecessarily conservative as CMPXCHG writes
+ * back the original value and the access is atomic, but KVM's ABI is
+ * that all writes are dirty logged, regardless of the value written.
+ */
+ kvm_vcpu_mark_page_dirty(vcpu, gpa_to_gfn(gpa));
+
if (r)
return X86EMUL_CMPXCHG_FAILED;
--
2.43.0.687.g38aa6559b0-goog
next prev parent reply other threads:[~2024-02-15 1:00 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-15 1:00 [PATCH 0/2] KVM: x86: Fix dirty logging of emulated atomics Sean Christopherson
2024-02-15 1:00 ` Sean Christopherson [this message]
2024-02-15 17:13 ` [PATCH 1/2] KVM: x86: Mark target gfn of emulated atomic instruction as dirty Jim Mattson
2024-02-15 17:57 ` David Matlack
2024-02-15 18:45 ` Sean Christopherson
2024-02-16 17:10 ` Sean Christopherson
2024-02-16 17:14 ` David Matlack
2024-02-15 1:00 ` [PATCH 2/2] KVM: selftests: Test forced instruction emulation in dirty log test (x86 only) Sean Christopherson
2024-02-15 8:21 ` Oliver Upton
2024-02-15 18:50 ` Sean Christopherson
2024-02-15 20:13 ` Oliver Upton
2024-02-15 21:33 ` Sean Christopherson
2024-02-15 23:27 ` Oliver Upton
2024-02-16 0:26 ` Sean Christopherson
2024-02-16 15:55 ` Oliver Upton
2024-02-16 17:03 ` Sean Christopherson
2024-02-17 1:02 ` [PATCH 0/2] KVM: x86: Fix dirty logging of emulated atomics Sean Christopherson
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=20240215010004.1456078-2-seanjc@google.com \
--to=seanjc@google.com \
--cc=dmatlack@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkrebs@google.com \
--cc=pbonzini@redhat.com \
--cc=tatashin@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox