On Thu, Sep 28, 2017 at 03:26:49PM -0400, John Ferlan wrote:
Flag when virNWFilterIPAddrMapAddIPAddr to allow deletion - keep
@inetaddr around to message after virNWFilterInstantiateFilterLate
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/nwfilter/nwfilter_learnipaddr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 5b95f0e61..567897221 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -610,6 +610,7 @@ learnIPAddressThread(void *arg)
sa.data.inet4.sin_family = AF_INET;
sa.data.inet4.sin_addr.s_addr = vmaddr;
char *inetaddr;
+ bool failmap = false;
The name looks confusing. I'd prefer the name to document either what happened (adding_failed, address_consumed) or what should happen (free_addr). Or rather dropping the bool completely, log the IP address before the AddIpAddr call and remove it from the other VIR_DEBUG message.
/* It is necessary to unlock interface here to avoid updateMutex and
* interface ordering deadlocks. Otherwise we are going to
@@ -625,6 +626,7 @@ learnIPAddressThread(void *arg)
if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
VIR_ERROR(_("Failed to add IP address %s to IP address "
"cache for interface %s"), inetaddr, req->ifname);
+ failmap = true;
This assumes that virNWFilterIPAddrMapAddIPAddr consumes inetaddr only if it returns zero. However in a fraction of the unlikely cases when this function call can fail (all of them on OOM), it can consume it even though it returns -1 -- if virNWFilterVarValueCreateSimple succeeds, but virNWFilterHashTablePut fails.
}
ret = virNWFilterInstantiateFilterLate(req->driver,
@@ -637,7 +639,8 @@ learnIPAddressThread(void *arg)
req->filterparams);
VIR_DEBUG("Result from applying firewall rules on "
"%s with IP addr %s : %d", req->ifname, inetaddr, ret);
At this point, inetaddr belongs to the hash table protected by the lock that AddIPAddr already released. Can we safely access it here? Jan
- VIR_FREE(inetaddr);
+ if (failmap)
+ VIR_FREE(inetaddr);
}
} else {
if (showError)
--
2.13.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
