diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f9f3d3d17e..31d4463a47 100644 --- 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 6609d4f300..72be51e20c 100644 --- 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 bab8608cf8..34f2af31b2 100644 --- 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 @@ -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; }