util: report all address range errors in virSocketAddrGetRange()

There are now many more reasons that virSocketAddrGetRange() could
fail, so it is much more informative to report the error there instead
of in the caller. (one of the two callers was previously assuming
success, which is almost surely safe based on the parsing that has
already happened to the config by that time, but it still is nicer to
account for an error "just in case")

Part of fix for: https://bugzilla.redhat.com/show_bug.cgi?id=985653

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f9f3d3d17e33ba7419ffd9871c71b271503dbc4c..31d4463a475b842ba80d042c443846ab8529fed7 100644 (file)
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -861,12 +861,8 @@ virSocketAddrRangeParseXML(const char *networkName,
 
     /* do a sanity check of the range */
     if (virSocketAddrGetRange(&range->start, &range->end, &ipdef->address,
-                              virNetworkIpDefPrefix(ipdef)) < 0) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       _("Invalid dhcp range '%s' to '%s' in network '%s'"),
-                       start, end, networkName);
+                              virNetworkIpDefPrefix(ipdef)) < 0)
         goto cleanup;
-    }
 
     ret = 0;
 

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6609d4f30068b4ab4e0a6789dfdf6bf6212f6614..72be51e20c20b7b135d97a3f81c19f703f4ad746 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1181,6 +1181,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 
     while (ipdef) {
         for (r = 0; r < ipdef->nranges; r++) {
+            int thisRange;
+
             if (!(saddr = virSocketAddrFormat(&ipdef->ranges[r].start)) ||
                 !(eaddr = virSocketAddrFormat(&ipdef->ranges[r].end)))
                 goto cleanup;
@@ -1189,10 +1191,13 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
                               saddr, eaddr);
             VIR_FREE(saddr);
             VIR_FREE(eaddr);
-            nbleases += virSocketAddrGetRange(&ipdef->ranges[r].start,
+            thisRange = virSocketAddrGetRange(&ipdef->ranges[r].start,
                                               &ipdef->ranges[r].end,
                                               &ipdef->address,
                                               virNetworkIpDefPrefix(ipdef));
+            if (thisRange < 0)
+                goto cleanup;
+            nbleases += thisRange;
         }
 
         /*

diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index bab8608cf87d19571e84da3d075a0855863f3c92..34f2af31b2e7d6f03daaec98034f61a06ff0f319 100644 (file)
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -26,6 +26,7 @@
 #include "virsocketaddr.h"
 #include "virerror.h"
 #include "virstring.h"
+#include "viralloc.h"
 
 #include <netdb.h>
 
@@ -623,32 +624,63 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end,
     int ret = 0;
     size_t i;
     virSocketAddr netmask;
+    char *startStr = NULL, *endStr = NULL, *netStr = NULL;
 
-    if (start == NULL || end == NULL || network == NULL)
-        return -1;
+    if (start == NULL || end == NULL || network == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("NULL argument - %p %p %p"), start, end, network);
+        goto error;
+    }
+
+    startStr = virSocketAddrFormat(start);
+    endStr = virSocketAddrFormat(end);
+    netStr = virSocketAddrFormat(network);
+    if (!(startStr && endStr && netStr))
+        goto error; /*error already reported */
 
     if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end) ||
-        VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network))
-        return -1;
+        VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("mismatch of address family in "
+                         "range %s - %s for network %s"),
+                       startStr, endStr, netStr);
+        goto error;
+    }
 
     if (prefix < 0 ||
-        virSocketAddrPrefixToNetmask(prefix, &netmask, VIR_SOCKET_ADDR_FAMILY(network)) < 0)
-        return -1;
+        virSocketAddrPrefixToNetmask(prefix, &netmask,
+                                     VIR_SOCKET_ADDR_FAMILY(network)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("bad prefix %d for network %s when "
+                         " checking range %s - %s"),
+                       prefix, netStr, startStr, endStr);
+        goto error;
+    }
 
     /* both start and end of range need to be in the same network as
      * "network"
      */
     if (virSocketAddrCheckNetmask(start, network, &netmask) <= 0 ||
-        virSocketAddrCheckNetmask(end, network, &netmask) <= 0)
-        return -1;
+        virSocketAddrCheckNetmask(end, network, &netmask) <= 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("range %s - %s is not entirely within "
+                         "network %s/%d"),
+                       startStr, endStr, netStr, prefix);
+        goto error;
+    }
 
     if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) {
         virSocketAddrIPv4 t1, t2;
         virSocketAddr netaddr, broadcast;
 
         if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 ||
-            virSocketAddrMask(network, &netmask, &netaddr) < 0)
-           return -1;
+            virSocketAddrMask(network, &netmask, &netaddr) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to construct broadcast or network "
+                             "address for network %s/%d"),
+                           netStr, prefix);
+            goto error;
+        }
 
         /* Don't allow the start of the range to be the network
          * address (usually "...0") or the end of the range to be the
@@ -657,47 +689,104 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end,
          * other combined checks) (IPv6 doesn't have broadcast and
          * network addresses, so this check is only done for IPv4)
          */
-        if (virSocketAddrEqual(start, &netaddr) ||
-            virSocketAddrEqual(end, &broadcast))
-            return -1;
+        if (virSocketAddrEqual(start, &netaddr)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("start of range %s - %s in network %s/%d "
+                             "is the network address"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
+
+        if (virSocketAddrEqual(end, &broadcast)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("end of range %s - %s in network %s/%d "
+                             "is the broadcast address"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
 
         if ((virSocketAddrGetIPv4Addr(start, &t1) < 0) ||
-            (virSocketAddrGetIPv4Addr(end, &t2) < 0))
-            return -1;
+            (virSocketAddrGetIPv4Addr(end, &t2) < 0)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to get IPv4 address "
+                             "for start or end of range %s - %s "
+                             "in network %s/%d"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
 
         /* legacy check that everything except the last two bytes are
          * the same
          */
         for (i = 0; i < 2; i++) {
-            if (t1[i] != t2[i])
-                return -1;
+            if (t1[i] != t2[i]) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("range %s - %s is too large (> 65535) "
+                             "in network %s/%d"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+            }
         }
         ret = (t2[2] - t1[2]) * 256 + (t2[3] - t1[3]);
-        if (ret < 0)
-            return -1;
+        if (ret < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("range %s - %s is reversed "
+                             "in network %s/%d"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
         ret++;
     } else if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET6)) {
         virSocketAddrIPv6 t1, t2;
 
         if ((virSocketAddrGetIPv6Addr(start, &t1) < 0) ||
-            (virSocketAddrGetIPv6Addr(end, &t2) < 0))
-            return -1;
+            (virSocketAddrGetIPv6Addr(end, &t2) < 0)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to get IPv6 address "
+                             "for start or end of range %s - %s "
+                             "in network %s/%d"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
 
         /* legacy check that everything except the last two bytes are
          * the same
          */
         for (i = 0; i < 7; i++) {
-            if (t1[i] != t2[i])
-                return -1;
+            if (t1[i] != t2[i]) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("range %s - %s is too large (> 65535) "
+                                 "in network %s/%d"),
+                               startStr, endStr, netStr, prefix);
+                goto error;
+            }
         }
         ret = t2[7] - t1[7];
-        if (ret < 0)
-            return -1;
+        if (ret < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("range %s - %s start larger than end "
+                             "in network %s/%d"),
+                           startStr, endStr, netStr, prefix);
+            goto error;
+        }
         ret++;
     } else {
-        return -1;
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unsupported address family "
+                         "for range %s - %s "
+                         "in network %s/%d, must be ipv4 or ipv6"),
+                       startStr, endStr, netStr, prefix);
+        goto error;
     }
+ cleanup:
+    VIR_FREE(startStr);
+    VIR_FREE(endStr);
+    VIR_FREE(netStr);
     return ret;
+
+ error:
+    ret = -1;
+    goto cleanup;
 }