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

Ingo Molnar
2007-01-03T13:37:14Z

* 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);