This patch adds checks for empty bitmaps right after the calls of virBitmapParse. These only include spots where set API's are called and where domain's XML is parsed. Also, it partially reverts commit 983f5a which added a check for invalid nodeset "0,^0" into virBitmapParse function. This change broke the logic, as an empty bitmap should not cause an error. https://bugzilla.redhat.com/show_bug.cgi?id=1210545 --- src/conf/domain_conf.c | 35 +++++++++++++++++++++++++++++++---- src/conf/numa_conf.c | 23 +++++++++++++++++++---- src/qemu/qemu_driver.c | 5 +++-- src/util/virbitmap.c | 3 --- src/xenconfig/xen_sxpr.c | 7 +++++++ tests/virbitmaptest.c | 13 ++++++++++--- 6 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 823e003..f7f68ba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11577,6 +11577,12 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, if (virBitmapParse(nodemask, 0, &def->sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(def->sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodemask': %s"), nodemask); + goto cleanup; + } } ret = 0; @@ -13265,6 +13271,13 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpuset': %s"), + tmp); + goto error; + } + cleanup: VIR_FREE(tmp); ctxt->node = oldnode; @@ -13366,6 +13379,12 @@ virDomainHugepagesParseXML(xmlNodePtr node, if (virBitmapParse(nodeset, 0, &hugepage->nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(hugepage->nodemask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), nodeset); + goto cleanup; + } } ret = 0; @@ -13487,13 +13506,14 @@ virDomainThreadSchedParse(xmlNodePtr node, goto error; } - if (!virBitmapParse(tmp, 0, &sp->ids, - VIR_DOMAIN_CPUMASK_LEN) || - virBitmapIsAllClear(sp->ids) || + if (virBitmapParse(tmp, 0, &sp->ids, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto error; + + if (virBitmapIsAllClear(sp->ids) || virBitmapNextSetBit(sp->ids, -1) < minid || virBitmapLastSetBit(sp->ids) > maxid) { - virReportError(VIR_ERR_XML_ERROR, + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of '%s': %s"), name, tmp); goto error; @@ -13861,6 +13881,13 @@ virDomainDefParseXML(xmlDocPtr xml, if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpuset': %s"), tmp); + goto error; + } + VIR_FREE(tmp); } } diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 8a0f686..7ad3f66 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -178,6 +178,12 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa, if (virBitmapParse(tmp, 0, &mem_node->nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; + + if (virBitmapIsAllClear(mem_node->nodeset)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), tmp); + goto cleanup; + } VIR_FREE(tmp); } @@ -233,10 +239,19 @@ virDomainNumatuneParseXML(virDomainNumaPtr numa, } VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "nodeset")) && - virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - VIR_FREE(tmp); + tmp = virXMLPropString(node, "nodeset"); + if (tmp) { + if (virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; + + if (virBitmapIsAllClear(nodeset)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodeset': %s"), tmp); + goto cleanup; + } + + VIR_FREE(tmp); + } } if (virDomainNumatuneSet(numa, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f37a11e..cbb6e1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10134,8 +10134,9 @@ qemuDomainSetNumaParameters(virDomainPtr dom, goto endjob; if (virBitmapIsAllClear(nodeset)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Invalid nodeset for numatune")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("Invalid nodeset of 'numatune': %s"), + param->value.s); goto endjob; } } diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5322bce..bf905ab 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -416,9 +416,6 @@ virBitmapParse(const char *str, } } - if (virBitmapIsAllClear(*bitmap)) - goto error; - return virBitmapCountBits(*bitmap); error: diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 5a170d3..d77abf3 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1165,6 +1165,13 @@ xenParseSxpr(const struct sexpr *root, if (virBitmapParse(cpus, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; + + if (virBitmapIsAllClear(def->cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'cpumask': %s"), + cpus); + goto error; + } } def->maxvcpus = sexpr_int(root, "domain/vcpus"); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index f247275..9a84e4c 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -524,16 +524,23 @@ static int test10(const void *opaque ATTRIBUTE_UNUSED) { int ret = -1; - virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL; + virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL; if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 || virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 || - virBitmapParse("15", 0, &b3, 20) < 0) + virBitmapParse("15", 0, &b3, 20) < 0 || + virBitmapParse("0,^0", 0, &b4, 20) < 0) + goto cleanup; + + if (!virBitmapIsAllClear(b4)) goto cleanup; if (virBitmapOverlaps(b1, b2) || + virBitmapOverlaps(b1, b4) || virBitmapOverlaps(b2, b3) || - !virBitmapOverlaps(b1, b3)) + virBitmapOverlaps(b2, b4) || + !virBitmapOverlaps(b1, b3) || + virBitmapOverlaps(b3, b4)) goto cleanup; ret = 0; -- 1.9.3