Re: [patch] kvm: make cr3 loading more robust

[Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>]

* Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org> wrote:

> ok. How about the patch below then? This only addresses the OOM 
> scenario, not the !memslot case.

the !memslot case is covered by the patch below. Injecting a #GPF is the 
easiest one to do here, although we could do a triple fault too - i just 
dont see the infrastructure for that in KVM, so i went for the easier 
solution ;-)

I have tested this with an intentionally bad cr3 value in a Linux guest, 
and the result is a relatively clean guest abort crash:

  inject_general_protection: rip 0xc012093e
  kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

at the right RIP:

  c012093e:       0f 22 d8                mov    %eax,%cr3

instead of a host crash. Note that i chose to put this into the generic 
cr3 loading function, so that it covers real-mode too. I think we can 
safely ignore a BIOS loading crap into cr3 and after that loading the 
right value into it. (if that ever happens we 1) want to know about it 
2) can push the test down into paging_new_cr3()) Agreed?

	Ingo

Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>

Index: linux/drivers/kvm/kvm_main.c
===================================================================
--- linux.orig/drivers/kvm/kvm_main.c
+++ linux/drivers/kvm/kvm_main.c
@@ -466,7 +466,19 @@ void set_cr3(struct kvm_vcpu *vcpu, unsi
 
 	vcpu->cr3 = cr3;
 	spin_lock(&vcpu->kvm->lock);
-	vcpu->mmu.new_cr3(vcpu);
+	/*
+	 * Does the new cr3 value map to physical memory? (Note, we
+	 * catch an invalid cr3 even in real-mode, because it would
+	 * cause trouble later on when we turn on paging anyway.)
+	 *
+	 * A real CPU would silently accept an invalid cr3 and would
+	 * attempt to use it - with largely undefined (and often hard
+	 * to debug) behavior on the guest side.
+	 */
+	if (unlikely(!gfn_to_memslot(vcpu->kvm, cr3 >> PAGE_SHIFT)))
+		inject_gp(vcpu);
+	else
+		vcpu->mmu.new_cr3(vcpu);
 	spin_unlock(&vcpu->kvm->lock);
 }
 EXPORT_SYMBOL_GPL(set_cr3);

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV