Re: KVM: MMU: add KVM_ZAP_GFN ioctl

Andrea Arcangeli
2008-03-26T12:31:13Z

On Mon, Mar 24, 2008 at 07:54:27AM +0100, Andrea Arcangeli wrote:
> I'd more accurately describe the race as this:
>
>
> CPU0 CPU1
>
> spte = rmap_next(kvm, rmapp, NULL);
> while (spte) {
> BUG_ON(!spte);
> BUG_ON(!(*spte & PT_PRESENT_MASK));
> rmap_printk("rmap_nuke: spte %p %llx\n", spte, *spte);
> page_count = 2
> <---------- try_to_unmap_one()
> page_count = 1
> rmap_remove(kvm, spte); <- page is freed, race window opened
> set_shadow_pte(spte, shadow_trap_nonpresent_pte);
> nuked = 1;
> spte = rmap_next(kvm, rmapp, spte);
> }
> if (nuked)
> kvm_flush_remote_tlbs(kvm); <- race window closed

As I suspected, it seems this race can happen even when sptes are
removed from the cache. The VM may move the page_count to swapcount
while rmap_remove runs. Nobody has ever reproduced it, but it has to
be fixed nevertheless.

My mmu notifier patch closed this race condition against the linux VM,
by doing the last free with the right ordering under the PG_lock, in
turn ensuring when the VM removes the page from anonymous memory, the
tlb of the secondary mmu is flushed before the final put_page.

We discussed with Avi to do the right ordering all the time in
rmap_remove so there's no dependency on the Linux VM locking when
calling mmu notifier methods, and to be more robust in general (so
unmap all sptes, queue all the pages to unpin in a global array inside
the kvm_arch, flush the tlb once, put_page all the pages in the
array). As Avi pointed out we can't use page->lru for the queue.

Then perhaps your KVM ioctl will just work safe.

Marcelo, if you're interested to fix this let us know, so we don't
duplicate effort, or Avi or me can do the fix on rmap_remove.

Thanks!
Andrea

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.