File libvirt-Fix-parsing-of-bond-interface-XML.patch of Package libvirt

From 7051d7e2dd9330a4553e77c7aba4750b274f6ff8 Mon Sep 17 00:00:00 2001
Message-Id: <7051d7e2dd9330a4553e77c7aba4750b274f6ff8@dist-git>
From: Jim Fehlig <jfehlig@suse.com>
Date: Thu, 8 May 2014 10:28:42 -0400
Subject: [PATCH] Fix parsing of bond interface XML

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1067062

Noticed that parsing bond interface XML containing the miimon element
fails

  <interface type="bond" name="bond0">
    ...
    <bond mode="active-backup">
      <miimon freq="100" carrier="netif"/>
      ...
    </bond>
  </interface>

This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.

While fixing this bug, cleanup the function to use virXPathInt instead
of virXPathULong, and store the result directly instead of using a tmp
variable.  Using virXPathInt actually fixes a potential silent
truncation bug noted by Eric Blake.

Also, there is no cleanup in the error label.  Remove the label,
returning failure where failure occurs and success if the end of the
function is reached.
(cherry picked from commit 5ba077dcd0775d5fc5b8d3691c0a68b46e44ff9e)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/conf/interface_conf.c | 63 ++++++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 31b9219..bb4dd7a 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -572,81 +572,72 @@ error:
 static int
 virInterfaceDefParseBond(virInterfaceDefPtr def,
                          xmlXPathContextPtr ctxt) {
-    int ret = -1;
-    unsigned long tmp;
+    int res;
 
     def->data.bond.mode = virInterfaceDefParseBondMode(ctxt);
     if (def->data.bond.mode < 0)
-        goto error;
+        return -1;
 
-    ret = virInterfaceDefParseBondItfs(def, ctxt);
-    if (ret != 0)
-       goto error;
+    if (virInterfaceDefParseBondItfs(def, ctxt) != 0)
+        return -1;
 
     if (virXPathNode("./miimon[1]", ctxt) != NULL) {
         def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII;
 
-        ret = virXPathULong("string(./miimon/@freq)", ctxt, &tmp);
-        if ((ret == -2) || (ret == -1)) {
+        res = virXPathInt("string(./miimon/@freq)", ctxt,
+                          &def->data.bond.frequency);
+        if ((res == -2) || (res == -1)) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon freq missing or invalid"));
-            goto error;
+            return -1;
         }
-        def->data.bond.frequency = (int) tmp;
 
-        ret = virXPathULong("string(./miimon/@downdelay)", ctxt, &tmp);
-        if (ret == -2) {
+        res = virXPathInt("string(./miimon/@downdelay)", ctxt,
+                          &def->data.bond.downdelay);
+        if (res == -2) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon downdelay invalid"));
-            goto error;
-        } else if (ret == 0) {
-            def->data.bond.downdelay = (int) tmp;
+            return -1;
         }
 
-        ret = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp);
-        if (ret == -2) {
+        res = virXPathInt("string(./miimon/@updelay)", ctxt,
+                          &def->data.bond.updelay);
+        if (res == -2) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon updelay invalid"));
-            goto error;
-        } else if (ret == 0) {
-            def->data.bond.updelay = (int) tmp;
+            return -1;
         }
 
         def->data.bond.carrier = virInterfaceDefParseBondMiiCarrier(ctxt);
-        if (def->data.bond.carrier < 0) {
-            ret = -1;
-            goto error;
-        }
+        if (def->data.bond.carrier < 0)
+            return -1;
 
     } else if (virXPathNode("./arpmon[1]", ctxt) != NULL) {
 
         def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP;
 
-        ret = virXPathULong("string(./arpmon/@interval)", ctxt, &tmp);
-        if ((ret == -2) || (ret == -1)) {
+        res = virXPathInt("string(./arpmon/@interval)", ctxt,
+                          &def->data.bond.interval);
+        if ((res == -2) || (res == -1)) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface arpmon interval missing or invalid"));
-            goto error;
+            return -1;
         }
-        def->data.bond.interval = (int) tmp;
 
         def->data.bond.target =
             virXPathString("string(./arpmon/@target)", ctxt);
         if (def->data.bond.target == NULL) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface arpmon target missing"));
-            ret = -1;
-            goto error;
+            return -1;
         }
 
         def->data.bond.validate = virInterfaceDefParseBondArpValid(ctxt);
-        if (def->data.bond.validate < 0) {
-            ret = -1;
-            goto error;
-        }
+        if (def->data.bond.validate < 0)
+            return -1;
     }
-error:
-    return ret;
+
+    return 0;
 }
 
 static int
-- 
1.9.3