[libvirt] [PATCH] Fix segmentation fault when accessing default qemu machine type

[Yudai Yamagishi]

> Yudai, would you be comfortable with updating our test suite to test
> this corner case?
Okay, I've taken look at the test suite and I can update it.
However, my time is pretty limited right now so it may take a little while
to get the update done. If you need this ASAP, maybe someone else
should do this. If not, I'll do it in my spare time.

Meanwhile, is it possible for you to have this patch merged?

Best Regards,
Yudai Yamagishi

2013/12/17 Yudai Yamagishi <yummy at sfc.wide.ad.jp>:
> I'm not familiar with the libvirt test suite but let me see if I can
> enhance it a little bit.
>
> Best Regards
> Yudai Yamagishi
>
> 2013/12/17 Jiri Denemark <jdenemar at redhat.com>:
>> On Tue, Dec 17, 2013 at 17:33:19 +0900, Yudai Yamagishi wrote:
>>> From: Yudai Yamagish <yummy at sfc.wide.ad.jp>
>>>
>>> This patch fixes a segmentation fault when creating new virtual machines using QEMU.
>>> The segmentation fault is caused by commit f41830680e40d3ec845cefd25419bd87414b9ccf
>>> and commit cbb6ec42e2447d7920b30d66923b2a2b2670133b.
>>>
>>> In virQEMUCapsProbeQMPMachineTypes, when copying machines to qemuCaps, "none" is skipped.
>>> Therefore, the value of i and "qemuCaps->nmachineTypes - 1" do not always match.
>>> However, defIdx value (used to call virQEMUCapsSetDefaultMachine) is set using the value in i
>>> when the array elements are in qemuCaps->nmachineTypes - 1.
>>> So, when libvirt tries to create virtual machines using the default machine type,
>>> qemuCaps->machineTypes[defIdx] is accessed and since the defIdx is NULL, it results in segmentation fault.
>>>
>>> Signed-off-by: Yudai Yamagishi <yummy at sfc.wide.ad.jp>
>>> ---
>>>  src/qemu/qemu_capabilities.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>>> index 5e9c65e..5def55c 100644
>>> --- a/src/qemu/qemu_capabilities.c
>>> +++ b/src/qemu/qemu_capabilities.c
>>> @@ -2150,7 +2150,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
>>>                         machines[i]->name) < 0)
>>>              goto cleanup;
>>>          if (machines[i]->isDefault)
>>> -            defIdx = i;
>>> +            defIdx = qemuCaps->nmachineTypes - 1;
>>>          qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes - 1] =
>>>              machines[i]->maxCpus;
>>>      }
>>
>> Oops, good catch. The reason we haven't seen it yet is the order of
>> machine types returned by your QEMU is apparently different than the
>> order in which anyone else gets them :-) If "none" comes after the
>> default machine type, everything works as expected. Also if the default
>> machine is not the last one, libvirt would just select a wrong machine
>> type as the default one instead of segfaulting.
>>
>> Yudai, would you be comfortable with updating our test suite to test
>> this corner case? We already have qemucapabilitiestest.c but it only
>> tests QEMU capabilities even though the data files already contain
>> machine types. So the test would need to be enhanced to actually check
>> machine types decoded from the data files. And we would need a data file
>> where "none" is not the last machine type in the list.
>>
>> Jirka