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);
next 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox