[libvirt] [PATCH 0/2] qemu: add range check for numa memory placement mode
Peter Krempa (2): qemu: Clean up qemuDomainSetNumaParameters qemu: range check numa memory placement mode src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) -- 1.8.5.2
Add whitespace to separate logical code blocks, reformat error messages and clean up code flow. This patch changes error handling in some cases where the the loop would be continued to jump to cleanup instead and error out rather than modify the domain any further. --- src/qemu/qemu_driver.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 45d11cd..819ad7f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8237,6 +8237,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParamsValidate(params, nparams, VIR_DOMAIN_NUMA_MODE, VIR_TYPED_PARAM_INT, @@ -8263,43 +8264,40 @@ qemuDomainSetNumaParameters(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup cpuset controller is not mounted")); + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cgroup cpuset controller is not mounted")); goto cleanup; } } - ret = 0; for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) { + int mode = param->value.i; + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && - vm->def->numatune.memory.mode != params[i].value.i) { + vm->def->numatune.memory.mode != mode) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("can't change numa mode for running domain")); - ret = -1; goto cleanup; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - persistentDef->numatune.memory.mode = params[i].value.i; - } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + persistentDef->numatune.memory.mode = mode; + } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { virBitmapPtr nodeset = NULL; - if (virBitmapParse(params[i].value.s, - 0, &nodeset, + if (virBitmapParse(param->value.s, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) { - ret = -1; - continue; + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (qemuDomainSetNumaParamsLive(vm, caps, nodeset) < 0) { virBitmapFree(nodeset); - ret = -1; - continue; + goto cleanup; } /* update vm->def here so that dumpxml can read the new @@ -8328,9 +8326,11 @@ qemuDomainSetNumaParameters(virDomainPtr dom, persistentDef->numatune.memory.placement_mode = VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO; if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) - ret = -1; + goto cleanup; } + ret = 0; + cleanup: if (vm) virObjectUnlock(vm); -- 1.8.5.2
https://bugzilla.redhat.com/show_bug.cgi?id=1047234 Add a range check for supported numa memory placement modes provided by the user before setting them in the domain definition. Without the check the user is able to provide a (yet) unknown mode which is then stored in the domain definition. This potentially causes a NULL dereference when the defintion is formatted into the XML. To reproduce run: virsh numatune DOMNAME --mode 6 --nodeset 0 The XML will then contain: <numatune> <memory mode='(null)' nodeset='0'/> </numatune> With this fix, the command fails: error: Unable to change numa parameters error: invalid argument: unsupported numa_mode: '6' --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 819ad7f..7e45ffc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8276,6 +8276,14 @@ qemuDomainSetNumaParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) { int mode = param->value.i; + if (mode >= VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST || + mode < VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_DEFAULT) + { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported numa_mode: '%d'"), mode); + goto cleanup; + } + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && vm->def->numatune.memory.mode != mode) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", -- 1.8.5.2
On 01/06/2014 08:33 AM, Peter Krempa wrote:
Peter Krempa (2): qemu: Clean up qemuDomainSetNumaParameters qemu: range check numa memory placement mode
src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)
ACK series. John
On 01/06/14 15:56, John Ferlan wrote:
On 01/06/2014 08:33 AM, Peter Krempa wrote:
Peter Krempa (2): qemu: Clean up qemuDomainSetNumaParameters qemu: range check numa memory placement mode
src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)
ACK series.
Pushed; Thanks.
John
Peter
participants (2)
-
John Ferlan -
Peter Krempa