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

[Yudai Yamagishi]

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