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

Avi Kivity
2008-03-27T08:06:52Z

Andrea Arcangeli wrote:
> On Wed, Mar 26, 2008 at 08:22:31PM +0100, Andrea Arcangeli wrote:
>
>> what happens if invalidate_page runs after rmap_remove is returned
>> (the spte isn't visible anymore by the rmap code and in turn by
>> invalidate_page) but before the set_shadow_pte(nonpresent) runs.
>>
>
> Thinking some more the mmu_lock is meant to prevent this. So
> invalidate_page should wait. As long as the kvm tlb flush happens
> inside the mmu lock we should be safe.
>
> Fixing it with mmu notifiers is the higher performance way too. This
> would be the patch if we decide to do that.
>
>

Well, obviously mmu notifiers is the future and we should code for that,
instead of increasing code complexity.

> Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 95c12bc..80cf172 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -550,6 +550,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
> sp = page_header(__pa(spte));
> page = spte_to_page(*spte);
> mark_page_accessed(page);
> + BUG_ON(page_count(page) <= 1);
> if (is_writeble_pte(*spte))
> kvm_release_page_dirty(page);
> else
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 30bf832..a49987c 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -741,6 +741,10 @@ static struct vm_operations_struct kvm_vcpu_vm_ops = {
> static int kvm_vcpu_mmap(struct file *file, struct vm_area_struct *vma)
> {
> vma->vm_ops = &kvm_vcpu_vm_ops;
> +#ifndef CONFIG_MMU_NOTIFIER
> + /* prevent the VM to release pages under sptes mappings */
> + vma->vm_flags |= VM_LOCKED;
> +#endif
> return 0;
> }
>
>

That's sad, but I guess the only safe and simple option is to queue this
for 2.6.25 and remove it in 2.6.26.

--
error compiling committee.c: too many arguments to function