From: Yudai Yamagish <yummy(a)sfc.wide.ad.jp&gt; 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(a)sfc.wide.ad.jp&gt; --- 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; }

Yudai, would you be comfortable with updating our test suite to test this corner case?

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(a)redhat.com&gt;: > On Tue, Dec 17, 2013 at 17:33:19 +0900, Yudai Yamagishi wrote: >> From: Yudai Yamagish <yummy(a)sfc.wide.ad.jp&gt; >> >> 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(a)sfc.wide.ad.jp&gt; >> --- >> 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