Commit efbeec70 authored Dec 27, 2014 by Paolo Bonzini

kvm: fix sorting of memslots with base_gfn == 0



Before commit 0e60b079 (kvm: change memslot sorting rule from size
to GFN, 2014-12-01), the memslots' sorting key was npages, meaning
that a valid memslot couldn't have its sorting key equal to zero.
On the other hand, a valid memslot can have base_gfn == 0, and invalid
memslots are identified by base_gfn == npages == 0.

Because of this, commit 0e60b079 broke the invariant that invalid
memslots are at the end of the mslots array.  When a memslot with
base_gfn == 0 was created, any invalid memslot before it were left
in place.

This can be fixed by changing the insertion to use a ">=" comparison
instead of "<=", but some care is needed to avoid breaking the case
of deleting a memslot; see the comment in update_memslots.

Thanks to Tiejun Chen for posting an initial patch for this bug.

Reported-by: Jamie Heilman <jamie@audible.transient.net>
Reported-by: Andy Lutomirski <luto@amacapital.net>
Tested-by: Jamie Heilman <jamie@audible.transient.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

parent a629df7e

virt/kvm/kvm_main.c

+17 −5

Original line number	Original line	Diff line number	Diff line
	@@ -687,12 +687,24 @@ static void update_memslots(struct kvm_memslots *slots,
	slots->id_to_index[mslots[i].id] = i;		slots->id_to_index[mslots[i].id] = i;
	i++;		i++;
	}		}

			/*
			* The ">=" is needed when creating a slot with base_gfn == 0,
			* so that it moves before all those with base_gfn == npages == 0.
			*
			* On the other hand, if new->npages is zero, the above loop has
			* already left i pointing to the beginning of the empty part of
			* mslots, and the ">=" would move the hole backwards in this
			* case---which is wrong. So skip the loop when deleting a slot.
			*/
			if (new->npages) {
	while (i > 0 &&		while (i > 0 &&
	new->base_gfn > mslots[i - 1].base_gfn) {		new->base_gfn >= mslots[i - 1].base_gfn) {
	mslots[i] = mslots[i - 1];		mslots[i] = mslots[i - 1];
	slots->id_to_index[mslots[i].id] = i;		slots->id_to_index[mslots[i].id] = i;
	i--;		i--;
	}		}
			}

	mslots[i] = *new;		mslots[i] = *new;
	slots->id_to_index[mslots[i].id] = i;		slots->id_to_index[mslots[i].id] = i;