5:41 a.m.
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.
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/xenconfig/xen_sxpr.c | 7 +++++++
4 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1763305..c9488cf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11563,6 +11563,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;
@@ -13251,6 +13257,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;
@@ -13352,6 +13365,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;
@@ -13473,13 +13492,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;
@@ -13847,6 +13867,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 7eb5a7d..eae2809 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10106,8 +10106,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/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");
--
1.9.3
5:41 a.m.
New subject: [libvirt] [PATCH 2/2] util: Remove virBitmapIsAllClear from virBitmapParse
This patch 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. This should
rather be checked at individual spots.
---
src/util/virbitmap.c | 3 ---
1 file changed, 3 deletions(-)
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:
--
1.9.3
12:12 a.m.
New subject: [libvirt] [PATCH 2/2] util: Remove virBitmapIsAllClear from virBitmapParse
On Fri, Apr 10, 2015 at 12:41:13 +0200, Erik Skultety wrote:
This patch 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. This should
rather be checked at individual spots.
This should be squashed in the previous patch. Additionally it would be
great if you could also add tests to virbitmaptest to test this
particular case so that somebody doesn't change it in the future.
Peter
12:10 a.m.
On Fri, Apr 10, 2015 at 12:41:12 +0200, Erik Skultety wrote:
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.
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/xenconfig/xen_sxpr.c | 7 +++++++
4 files changed, 60 insertions(+), 10 deletions(-)
I've "git grep"'d a few uses of this function and found a few places
that would also need a chceck. Few examples are the XML parser for
networks, the use in qemuProcessStart() to parse the automatic
placemenet and so on ..
The rest looks good though.
Peter
7:01 a.m.
On 04/13/2015 07:10 AM, Peter Krempa wrote:
On Fri, Apr 10, 2015 at 12:41:12 +0200, Erik Skultety wrote:
> 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.
>
> 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/xenconfig/xen_sxpr.c | 7 +++++++
> 4 files changed, 60 insertions(+), 10 deletions(-)
I've "git grep"'d a few uses of this function and found a few places
that would also need a chceck. Few examples are the XML parser for
networks, the use in qemuProcessStart() to parse the automatic
placemenet and so on ..
The rest looks good though.
Peter
At the moment, there's really no need to check the bitmap 'class_id' in
network.conf, because it's a status XML anyway, however even if user
changed our status XML and an error occurred, it would be user's
responsibility, the only exception to this would be a daemon crash which
isn't this case. Moreover, if you parse the bitmap "0,^0" the empty
bitmap wouldn't have almost any effect, because by default first 3 bits
are always set during network object creation.
To the qemuProcessStart, we call numad to get a suggestion for the
nodeset and the only problem would be if numad returned empty string,
however this would be handled by virBitmapParse itself successfully.
The other occurrences are tests mostly, v1 included this check for XEN
parser, I'm not sure about the 'Parallels' though.
Anyway, I squashed the second patch into this one as you suggested and
modified an existing test in virbitmaptest.c for v2. (I thought checking
this specific case wasn't worth having a separate test, so I modified
bitmap-overlap test, instead of just testing the bitmap parsing.)
Erik
3511
days inactive
3514
days old
4 comments
2 participants
participants (2)
-
Erik Skultety -
Peter Krempa