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...
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/network/bridge_driver.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a007388..d885aa9 100644
--- 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;
@@ -4593,17 +4593,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 +4689,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;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list