4:27 a.m.
On 20.09.2012 17:29, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones(a)redhat.com>
This is just code motion, allowing us to reuse the same function to
parse the <seclabel> from character devices too.
---
src/conf/domain_conf.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35814fb..06b3209 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3253,8 +3253,10 @@ error:
return -1;
}
+/* Parse the <seclabel> from a disk or serial device. */
static int
-virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
+ size_t *nseclabels_rtn,
virSecurityLabelDefPtr *vmSeclabels,
int nvmSeclabels, xmlXPathContextPtr ctxt)
{
@@ -3263,19 +3265,16 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
virSecurityLabelDefPtr vmDef = NULL;
char *model, *relabel, *label;
- if (def == NULL)
- return 0;
-
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
return 0;
- def->nseclabels = n;
- if (VIR_ALLOC_N(def->seclabels, n) < 0) {
+ *nseclabels_rtn = n;
+ if (VIR_ALLOC_N(*seclabels_rtn, n) < 0) {
I know it is not your fault, but this may lead to SIGSEGV. if this fails
then we proceed to error label where we dereference *seclabels_rtn.
I think the correct solution is:
- if VIR_ALLOC_N() fails, virReportOOMError(); return -1;
- set nseclabels_rtn = n; after alloc succeeds.
or check for *seclabels_rtn != NULL within error label.
virReportOOMError();
goto error;
}
for (i = 0; i < n; i++) {
- if (VIR_ALLOC(def->seclabels[i]) < 0) {
+ if (VIR_ALLOC((*seclabels_rtn)[i]) < 0) {
virReportOOMError();
goto error;
}
This is okay, since previous VIR_ALLOC zeroed the allocated memory.
@@ -3292,7 +3291,7 @@
virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
break;
}
}
- def->seclabels[i]->model = model;
+ (*seclabels_rtn)[i]->model = model;
}
/* Can't use overrides if top-level doesn't allow relabeling. */
@@ -3306,9 +3305,9 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
relabel = virXMLPropString(list[i], "relabel");
if (relabel != NULL) {
if (STREQ(relabel, "yes")) {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
} else if (STREQ(relabel, "no")) {
- def->seclabels[i]->norelabel = true;
+ (*seclabels_rtn)[i]->norelabel = true;
} else {
virReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"),
@@ -3318,19 +3317,19 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
}
VIR_FREE(relabel);
} else {
- def->seclabels[i]->norelabel = false;
+ (*seclabels_rtn)[i]->norelabel = false;
}
ctxt->node = list[i];
label = virXPathStringLimit("string(./label)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
- def->seclabels[i]->label = label;
+ (*seclabels_rtn)[i]->label = label;
- if (label && def->seclabels[i]->norelabel) {
+ if (label && (*seclabels_rtn)[i]->norelabel) {
virReportError(VIR_ERR_XML_ERROR,
_("Cannot specify a label if relabelling is "
"turned off. model=%s"),
- NULLSTR(def->seclabels[i]->model));
+ NULLSTR((*seclabels_rtn)[i]->model));
goto error;
}
}
@@ -3339,10 +3338,11 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
error:
for (i = 0; i < n; i++) {
- virSecurityDeviceLabelDefFree(def->seclabels[i]);
+ virSecurityDeviceLabelDefFree((*seclabels_rtn)[i]);
}
- VIR_FREE(def->seclabels);
+ VIR_FREE(*seclabels_rtn);
VIR_FREE(list);
+ *nseclabels_rtn = 0;
return -1;
}
@@ -3834,7 +3834,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (sourceNode) {
xmlNodePtr saved_node = ctxt->node;
ctxt->node = sourceNode;
- if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
+ &def->nseclabels,
+ vmSeclabels,
nvmSeclabels,
ctxt) < 0)
goto error;
ACK with the issue fixed.
Michal