Re: [PATCH] KVM: MMU: Fix rmap_remove() race

[Andrea Arcangeli <andrea@qumranet.com>]

On Fri, Mar 28, 2008 at 03:01:13PM +0100, Andrea Arcangeli wrote:
> @@ -271,8 +292,12 @@ int __kvm_set_memory_region(struct kvm *kvm,
>  
>  	r = -EINVAL;
>  	/* General sanity checks */
> +	if (mem->userspace_addr & (PAGE_SIZE - 1))
> +		goto out;
>  	if (mem->memory_size & (PAGE_SIZE - 1))
>  		goto out;
> +	if (mem->userspace_addr + mem->memory_size < mem->userspace_addr)
> +		goto out;

Those above (I suppose they're needed even if not strictly related to
the VM_LOCKED) broke vmx (only my laptop is vmx so I noticed it after
submission sorry!, the testing I did on svm test system before
submission worked fine and kvm was oom killed w/o swapping as
expected). I figured out the above likely broke older userland for the
same reason it broke vmx, so a new patch follows that should works for
vmx and should allow older userspace to still work and in VM_LOCKED
safe mode.

I'll try to make a new mmu notifier patch soon that will allow munmap
on a guest live gphys address space for the first time so ballooning
will be safe. Swapping is already safe and allowed with the current
kvm-swapping patch using the mmu notifiers, thanks to the mmu_lock
taken by invalidate_page that serializes the VM against
"spin_lock(mmu_lock); rmap_remove; tlbflush; spin_unlock(mmu_lock)".

Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 30bf832..8ece406 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -250,6 +250,24 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+#ifndef CONFIG_MMU_NOTIFIER
+static void memslot_mlock(unsigned long start, unsigned long len)
+{
+	struct vm_area_struct * vma;
+	unsigned long end = start+len;
+
+	vma = find_vma(current->mm, start);
+	if (!vma || vma->vm_start > start)
+		return;
+
+	/* go simple and don't split vmas */
+	for (;vma && vma->vm_start < end; vma = vma->vm_next) {
+		vma->vm_flags |= VM_LOCKED;
+		start = vma->vm_end;
+	}
+}
+#endif
+
 /*
  * Allocate some memory and give it an address in the guest physical address
  * space.
@@ -273,6 +291,10 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	/* General sanity checks */
 	if (mem->memory_size & (PAGE_SIZE - 1))
 		goto out;
+	if (user_alloc &&
+	    ((mem->userspace_addr & (PAGE_SIZE - 1)) ||
+	     (mem->userspace_addr + mem->memory_size < mem->userspace_addr)))
+		goto out;
 	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
 		goto out;
 	if (mem->slot >= KVM_MEMORY_SLOTS + KVM_PRIVATE_MEM_SLOTS)
@@ -366,6 +388,10 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		goto out_free;
 	}
 
+#ifndef CONFIG_MMU_NOTIFIER
+	memslot_mlock(mem->userspace_addr, mem->memory_size);
+#endif
+
 	kvm_free_physmem_slot(&old, &new);
 	return 0;
 

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace