Sheng Yang wrote:
> On Monday 13 April 2009 16:50:40 Jan Kiszka wrote:
>> Sheng Yang wrote:
>>> On Saturday 11 April 2009 17:48:04 Jan Kiszka wrote:
>>>> This nice little buglet complicates a smarter slot management in qemu
>>>> user space just "slightly". Sigh...
>>>>
>>>> -------->
>>>>
>>>> When checking for overlapping slots on registration of a new one, kvm
>>>> currently also considers zero-length (ie. deleted) slots and rejects
>>>> requests incorrectly. This finally denies user space from joining slots.
>>>> Fix the check by skipping deleted slots.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>>>> ---
>>>>
>>>> virt/kvm/kvm_main.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>>>> index 363af32..18f06d2 100644
>>>> --- a/virt/kvm/kvm_main.c
>>>> +++ b/virt/kvm/kvm_main.c
>>>> @@ -1117,7 +1117,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
>>>> for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
>>>> struct kvm_memory_slot *s = &kvm->memslots[i];
>>>>
>>>> - if (s == memslot)
>>>> + if (s == memslot || !s->npages)
>>>> continue;
>>>> if (!((base_gfn + npages <= s->base_gfn) ||
>>>> (base_gfn >= s->base_gfn + s->npages)))
>>> Is it necessary to preserve a valid base_gfn/flags/etc for a zeroed slot?
>>> Seems kvm_free_physmem_slot didn't clean them.
>> It is not necessary as long as we ignore such slots (as this patch does).
>
> What I think is, if they are invalid and unnecessary to keep, it's better to
> clean them rather than add a additional check, for it should covered by
> current check.
I think it is cleaner to add an explicit check for "slot unused"
(!npages) than re-initializing it with "mostly harmless" values. I've no
problem with zeroing them, but the test here should stay.
BTW, I was hoping to find a way to initialize deleted slots with safe
values from user space to work around this bug, but I found none. :(
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
