From db2aff6adaa405f0bc998c7bd3158fe43805ee60 Mon Sep 17 00:00:00 2001

From: "Richard W.M. Jones" <rjones@redhat.com>

Date: Thu, 20 Sep 2012 14:58:12 +0100

Subject: [PATCH 1/2] Make virSecurityDeviceLabelDefParseXML into generic

 device <seclabel> parser.

This is just code motion, allowing us to reuse the same function to

parse the <seclabel> from character devices too.

However it also fixes a possible segfault in the original code if

VIR_ALLOC_N returns an error and the cleanup code (at the error:

label) tries to iterate over the unallocated array (thanks Michal

Privoznik for spotting this).

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

---

 src/conf/domain_conf.c | 43 +++++++++++++++++++++++++------------------

 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 947cc7a..26c2042 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -3258,29 +3258,30 @@ error:

     return -1;

 }

+/* Parse the <seclabel> from a disk or character device. */

 static int

-virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,

+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,

+                                  size_t *nseclabels_rtn,

                                   virSecurityLabelDefPtr *vmSeclabels,

                                   int nvmSeclabels, xmlXPathContextPtr ctxt)

 {

+    virSecurityDeviceLabelDefPtr *seclabels;

+    size_t nseclabels = 0;

     int n, i, j;

     xmlNodePtr *list = NULL;

     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) {

+    if (VIR_ALLOC_N(seclabels, n) < 0) {

         virReportOOMError();

         goto error;

     }

+    nseclabels = n;

     for (i = 0; i < n; i++) {

-        if (VIR_ALLOC(def->seclabels[i]) < 0) {

+        if (VIR_ALLOC(seclabels[i]) < 0) {

             virReportOOMError();

             goto error;

         }

@@ -3297,7 +3298,7 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,

                     break;

                 }

             }

-            def->seclabels[i]->model = model;

+            seclabels[i]->model = model;

         }

         /* Can't use overrides if top-level doesn't allow relabeling.  */

@@ -3311,9 +3312,9 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,

         relabel = virXMLPropString(list[i], "relabel");

         if (relabel != NULL) {

             if (STREQ(relabel, "yes")) {

-                def->seclabels[i]->norelabel = false;

+                seclabels[i]->norelabel = false;

             } else if (STREQ(relabel, "no")) {

-                def->seclabels[i]->norelabel = true;

+                seclabels[i]->norelabel = true;

             } else {

                 virReportError(VIR_ERR_XML_ERROR,

                                _("invalid security relabel value %s"),

@@ -3323,30 +3324,34 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,

             }

             VIR_FREE(relabel);

         } else {

-            def->seclabels[i]->norelabel = false;

+            seclabels[i]->norelabel = false;

         }

         ctxt->node = list[i];

         label = virXPathStringLimit("string(./label)",

                                     VIR_SECURITY_LABEL_BUFLEN-1, ctxt);

-        def->seclabels[i]->label = label;

+        seclabels[i]->label = label;

-        if (label && def->seclabels[i]->norelabel) {

+        if (label && seclabels[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[i]->model));

             goto error;

         }

     }

     VIR_FREE(list);

+

+    *nseclabels_rtn = nseclabels;

+    *seclabels_rtn = seclabels;

+

     return 0;

 error:

-    for (i = 0; i < n; i++) {

-        virSecurityDeviceLabelDefFree(def->seclabels[i]);

+    for (i = 0; i < nseclabels; i++) {

+        virSecurityDeviceLabelDefFree(seclabels[i]);

     }

-    VIR_FREE(def->seclabels);

+    VIR_FREE(seclabels);

     VIR_FREE(list);

     return -1;

 }

@@ -3839,7 +3844,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;

-- 

1.7.11.4