Skip to content

Commit 910c57d

Browse files
committed
KVM: x86: Mark target gfn of emulated atomic instruction as dirty
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: 1c2361f ("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> base-commit: 6769ea8 Reviewed-by: Jim Mattson <jmattson@google.com> Link: https://lore.kernel.org/r/20240215010004.1456078-2-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 9895cee commit 910c57d

File tree

1 file changed

+10
-0
lines changed

1 file changed

+10
-0
lines changed

arch/x86/kvm/x86.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8007,6 +8007,16 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
80078007

80088008
if (r < 0)
80098009
return X86EMUL_UNHANDLEABLE;
8010+
8011+
/*
8012+
* Mark the page dirty _before_ checking whether or not the CMPXCHG was
8013+
* successful, as the old value is written back on failure. Note, for
8014+
* live migration, this is unnecessarily conservative as CMPXCHG writes
8015+
* back the original value and the access is atomic, but KVM's ABI is
8016+
* that all writes are dirty logged, regardless of the value written.
8017+
*/
8018+
kvm_vcpu_mark_page_dirty(vcpu, gpa_to_gfn(gpa));
8019+
80108020
if (r)
80118021
return X86EMUL_CMPXCHG_FAILED;
80128022

0 commit comments

Comments
 (0)