[libvirt] [PATCH] nwfilter: Don't crash when trying to add an nwfilter that's already being removed
https://bugzilla.redhat.com/show_bug.cgi?id=1686927
When trying to create a nwfilter binding via
nwfilterBindingCreateXML() we may encounter a crash. The sequence
of functions called is as follows:
1) nwfilterBindingCreateXML() parses the XML and calls
virNWFilterBindingObjListAdd() which calls
virNWFilterBindingObjListAddLocked()
2) Here, @binding is not found because binding->remove is set.
3) Therefore, controls continue with creating new @binding,
setting its def to the one from 1) and adding it to the hash
table.
4) This fails, because the binding is still in the hash table
(duplicate key is detected).
5) The control jumps to 'error' label where
virNWFilterBindingObjEndAPI() is called which frees the binding
definition passed.
6) Error is propagated to the caller, which calls
virNWFilterBindingDefFree() over the definition again.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/virnwfilterbindingobjlist.c | 11 ++++++-----
src/conf/virnwfilterbindingobjlist.h | 2 +-
src/nwfilter/nwfilter_driver.c | 5 ++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c
index 06ccbf53af..87189da642 100644
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -164,23 +164,24 @@ virNWFilterBindingObjListAddObjLocked(virNWFilterBindingObjListPtr
bindings,
*/
static virNWFilterBindingObjPtr
virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings,
- virNWFilterBindingDefPtr def)
+ virNWFilterBindingDefPtr *def)
{
virNWFilterBindingObjPtr binding;
/* See if a binding with matching portdev already exists */
if ((binding = virNWFilterBindingObjListFindByPortDevLocked(
- bindings, def->portdevname))) {
+ bindings, (*def)->portdevname))) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("binding '%s' already exists"),
- def->portdevname);
+ (*def)->portdevname);
goto error;
}
if (!(binding = virNWFilterBindingObjNew()))
goto error;
- virNWFilterBindingObjSetDef(binding, def);
+ virNWFilterBindingObjSetDef(binding, *def);
+ *def = NULL;
if (virNWFilterBindingObjListAddObjLocked(bindings, binding) < 0)
goto error;
@@ -195,7 +196,7 @@ virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr
bindings,
virNWFilterBindingObjPtr
virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings,
- virNWFilterBindingDefPtr def)
+ virNWFilterBindingDefPtr *def)
{
virNWFilterBindingObjPtr ret;
diff --git a/src/conf/virnwfilterbindingobjlist.h b/src/conf/virnwfilterbindingobjlist.h
index b0fb90f667..4047453634 100644
--- a/src/conf/virnwfilterbindingobjlist.h
+++ b/src/conf/virnwfilterbindingobjlist.h
@@ -35,7 +35,7 @@ virNWFilterBindingObjListFindByPortDev(virNWFilterBindingObjListPtr
bindings,
virNWFilterBindingObjPtr
virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings,
- virNWFilterBindingDefPtr def);
+ virNWFilterBindingDefPtr *def);
void
virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings,
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index fdfc6f48fa..8c2e987b5d 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -759,7 +759,7 @@ nwfilterBindingCreateXML(virConnectPtr conn,
goto cleanup;
obj = virNWFilterBindingObjListAdd(driver->bindings,
- def);
+ &def);
if (!obj)
goto cleanup;
@@ -775,8 +775,7 @@ nwfilterBindingCreateXML(virConnectPtr conn,
virNWFilterBindingObjSave(obj, driver->bindingDir);
cleanup:
- if (!obj)
- virNWFilterBindingDefFree(def);
+ virNWFilterBindingDefFree(def);
virNWFilterBindingObjEndAPI(&obj);
return ret;
--
2.19.2