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

[Jiri Denemark]

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