All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Avi Kivity <avi@qumranet.com>, "Richard W.M. Jones" <rjones@redhat.com>
Cc: kvm-devel <kvm@vger.kernel.org>
Subject: KVM: MMU: nuke shadowed pgtable pages and pte's on memslot destruction
Date: Thu, 3 Jul 2008 22:06:18 -0300	[thread overview]
Message-ID: <20080704010618.GA25834@dmt.cnet> (raw)


During RH6.2 graphical installation the following oops is triggered:

 BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
 IP: [<ffffffffa00bf172>] :kvm:gfn_to_rmap+0x3e/0x61
 Pid: 4559, comm: qemu-system-x86 Not tainted
 RIP: 0010:[<ffffffffa00bf172>]  [<ffffffffa00bf172>] :kvm:gfn_to_rmap+0x3e/0x61

 Process qemu-system-x86 (pid: 4559, threadinfo ffff810220920000, task
ffff81022e82b2f0)
 Stack:  0000000000217a4b ffff81021a48e9d8 ffff81021a48fa80
ffffffffa00bf240
  ffff810220ab82f0 ffff81021a48e9d8 ffff81021a48fa80 000000000000013b
  ffff81022288c000 ffffffffa00bf3b7 000000000003aed3 ffff81022288c000
 Call Trace:
  [<ffffffffa00bf240>] ? :kvm:rmap_remove+0xab/0x19d
  [<ffffffffa00bf3b7>] ? :kvm:kvm_mmu_zap_page+0x85/0x26e 
  [<ffffffffa00bf8a5>] ? :kvm:kvm_mmu_zap_all+0x2b/0x46 
  [<ffffffffa00bb37c>] ? :kvm:kvm_arch_vm_ioctl+0x262/0x575
  [<ffffffffa00b6fb8>] ? :kvm:kvm_read_guest+0x3f/0x7d
  [<ffffffffa00c19c9>] ? :kvm:paging32_walk_addr+0xac/0x262
  [<ffffffffa00b6d7c>] ? :kvm:gfn_to_hva+0x9/0x5d
  [<ffffffffa00b6f44>] ? :kvm:kvm_read_guest_page+0x11/0x46

The problem is that KVM allows shadow pagetable entries that
point to a removed memslot to exist. In this case the cirrus vram
mapping was removed, and the NULL dereference happened during
kvm_set_memory_alias()'s zap_all_pages().

Since a malicious guest could have a thread's root table inside the
to-be-removed memslot, also guarantee there this is not the case before
removal.

This behaviour will also be useful for memory hotplugging.

As a side note, RH 6.2 graphical installation still won't function (due
to cirrus emulation bugs).

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


Index: kvm/arch/x86/kvm/x86.c
===================================================================
--- kvm.orig/arch/x86/kvm/x86.c
+++ kvm/arch/x86/kvm/x86.c
@@ -4040,6 +4040,12 @@ int kvm_arch_set_memory_region(struct kv
 	return 0;
 }
 
+int kvm_arch_destroy_memory_region(struct kvm *kvm, int slot)
+{
+	kvm_mmu_zap_all(kvm);
+	return kvm_mmu_slot_has_shadowed_page(kvm, slot);
+}
+
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE
Index: kvm/include/linux/kvm_host.h
===================================================================
--- kvm.orig/include/linux/kvm_host.h
+++ kvm/include/linux/kvm_host.h
@@ -168,6 +168,7 @@ int kvm_arch_set_memory_region(struct kv
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+int kvm_arch_destroy_memory_region(struct kvm *kvm, int slot);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
 unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn);
Index: kvm/virt/kvm/kvm_main.c
===================================================================
--- kvm.orig/virt/kvm/kvm_main.c
+++ kvm/virt/kvm/kvm_main.c
@@ -405,6 +405,12 @@ int __kvm_set_memory_region(struct kvm *
 	if (mem->slot >= kvm->nmemslots)
 		kvm->nmemslots = mem->slot + 1;
 
+	if (!npages) {
+		r = kvm_arch_destroy_memory_region(kvm, mem->slot);
+		if (r)
+			goto out_free;
+	}
+
 	*memslot = new;
 
 	r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
Index: kvm/arch/ia64/kvm/kvm-ia64.c
===================================================================
--- kvm.orig/arch/ia64/kvm/kvm-ia64.c
+++ kvm/arch/ia64/kvm/kvm-ia64.c
@@ -1455,6 +1455,10 @@ int kvm_arch_set_memory_region(struct kv
 	return 0;
 }
 
+int kvm_arch_destroy_memory_region(struct kvm *kvm, int slot)
+{
+	return 0;
+}
 
 long kvm_arch_dev_ioctl(struct file *filp,
 		unsigned int ioctl, unsigned long arg)
Index: kvm/arch/powerpc/kvm/powerpc.c
===================================================================
--- kvm.orig/arch/powerpc/kvm/powerpc.c
+++ kvm/arch/powerpc/kvm/powerpc.c
@@ -170,6 +170,11 @@ int kvm_arch_set_memory_region(struct kv
 	return 0;
 }
 
+int kvm_arch_destroy_memory_region(struct kvm *kvm, int slot)
+{
+	return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 {
 	struct kvm_vcpu *vcpu;
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -675,6 +675,11 @@ int kvm_arch_set_memory_region(struct kv
 	return 0;
 }
 
+int kvm_arch_destroy_memory_region(struct kvm *kvm, int slot)
+{
+	return 0;
+}
+
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
 {
 	return gfn;
Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -1955,6 +1955,22 @@ void kvm_mmu_slot_remove_write_access(st
 	}
 }
 
+int kvm_mmu_slot_has_shadowed_page(struct kvm *kvm, int slot)
+{
+	struct kvm_mmu_page *sp;
+	int ret = 0;
+
+	spin_lock(&kvm->mmu_lock);
+	list_for_each_entry(sp, &kvm->arch.active_mmu_pages, link) {
+		if (test_bit(slot, &sp->slot_bitmap)) {
+			ret = -EINVAL;
+			break;
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+	return ret;
+}
+
 void kvm_mmu_zap_all(struct kvm *kvm)
 {
 	struct kvm_mmu_page *sp, *node;
Index: kvm/include/asm-x86/kvm_host.h
===================================================================
--- kvm.orig/include/asm-x86/kvm_host.h
+++ kvm/include/asm-x86/kvm_host.h
@@ -457,6 +457,7 @@ void kvm_mmu_set_mask_ptes(u64 user_mask
 
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
+int kvm_mmu_slot_has_shadowed_page(struct kvm *kvm, int slot);
 void kvm_mmu_zap_all(struct kvm *kvm);
 unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
 void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages);

             reply	other threads:[~2008-07-04  1:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-04  1:06 Marcelo Tosatti [this message]
2008-07-05 17:25 ` KVM: MMU: nuke shadowed pgtable pages and pte's on memslot destruction Avi Kivity
2008-07-05 19:23   ` Marcelo Tosatti
2008-07-05 21:15     ` Avi Kivity
2008-07-07 17:31       ` Marcelo Tosatti
2008-07-07 19:58         ` Marcelo Tosatti
2008-07-10 14:42           ` Avi Kivity
2008-07-10 18:58             ` Hollis Blanchard
2008-07-10 23:49               ` Marcelo Tosatti
2008-07-11 14:48                 ` Avi Kivity
2008-07-21 21:03                 ` Hollis Blanchard
2008-07-21 21:34                   ` Marcelo Tosatti
2008-07-21 22:22                     ` Hollis Blanchard
2008-07-22  5:18                   ` Avi Kivity
2008-07-10 23:54             ` Marcelo Tosatti
2008-07-11 15:09               ` Avi Kivity

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=20080704010618.GA25834@dmt.cnet \
    --to=mtosatti@redhat.com \
    --cc=avi@qumranet.com \
    --cc=kvm@vger.kernel.org \
    --cc=rjones@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.