network: Resolve Coverity FORWARD_NULL

The following is a long winded way to say this patch is avoiding a
false positive.

Coverity complains that calling networkPlugBandwidth() could eventually
end up with a NULL dereference on iface->bandwidth because in the
networkAllocateActualDevice there's a check of 'iface->bandwidth'
before deciding to try to use the 'portgroup' if it exists or to not
perferm the virNetDevBandwidthCopy if 'bandwidth' is not NULL.

Later in networkPlugBandwidth the 'iface->bandwidth' is sourced from
virDomainNetGetActualBandwidth - which would be either iface->bandwidth
or (preferably) iface->data.network.actual->bandwidth which would have
been filled in from either 'iface->bandwidth' or 'portgroup->bandwidth'
back in networkAllocateActualDevice

There *is* a check in networkCheckBandwidth for the result of the
virDomainNetGetActualBandwidth being NULL and a return 1 based on
that which would cause networkPlugBandwidth to exit properly and thus
never hit the condition that Coverity complains about.

However, since Coverity checks all paths - it somehow believes that
a return of 0 by networkCheckBandwidth in this condition would end
up causing the possible NULL dereference. The "fix" to silence Coverity
is to not have networkCheckBandwidth also call virDomainNetGetActualBandwidth
in order to get the ifaceBand, but rather have it accept it as an argument
which causes Coverity to "see" that it's the exit condition of 1 that won't
have the possible NULL dereference.  Since we're passing that, I added the
passing of iface->mac rather than passing iface as well. This just hopefully
makes sure someone doesn't undo this in the future...

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a007388ab7814aa79d20544970ccd587be292fbb..c37bcac81898902cf3240b6b8e12b3f9f189e585 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3820,7 +3820,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
         (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) ||
         (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE)) {
         /* for these forward types, the actual net type really *is*
-         *NETWORK; we just keep the info from the portgroup in
+         * NETWORK; we just keep the info from the portgroup in
          * iface->data.network.actual
          */
         iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK;
@@ -4584,7 +4584,8 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
 /**
  * networkCheckBandwidth:
  * @net: network QoS
- * @iface: interface QoS
+ * @ifaceBand: interface QoS (may be NULL if no QoS)
+ * @ifaceMac: interface MAC (used in error messages for identification)
  * @new_rate: new rate for non guaranteed class
  *
  * Returns: -1 if plugging would overcommit network QoS
@@ -4593,17 +4594,17 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
  */
 static int
 networkCheckBandwidth(virNetworkObjPtr net,
-                      virDomainNetDefPtr iface,
+                      virNetDevBandwidthPtr ifaceBand,
+                      virMacAddr ifaceMac,
                       unsigned long long *new_rate)
 {
     int ret = -1;
     virNetDevBandwidthPtr netBand = net->def->bandwidth;
-    virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
     unsigned long long tmp_floor_sum = net->floor_sum;
     unsigned long long tmp_new_rate = 0;
     char ifmac[VIR_MAC_STRING_BUFLEN];
 
-    virMacAddrFormat(&iface->mac, ifmac);
+    virMacAddrFormat(&ifaceMac, ifmac);
 
     if (ifaceBand && ifaceBand->in && ifaceBand->in->floor &&
         !(netBand && netBand->in)) {
@@ -4689,7 +4690,8 @@ networkPlugBandwidth(virNetworkObjPtr net,
     char ifmac[VIR_MAC_STRING_BUFLEN];
     virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
 
-    if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) {
+    if ((plug_ret = networkCheckBandwidth(net, ifaceBand,
+                                          iface->mac, &new_rate)) < 0) {
         /* helper reported error */
         goto cleanup;
     }