virnwfilterbindingobj: Introduce and use virNWFilterBindingObjStealDef

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.

The solution is to unset binding->def in case of failure so it's
not freed in step 5).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c
index 23978d4207fecaf09b88ea6562381afcf5f7254d..68afb9c4340da82664ab4fedc6c552165456fff3 100644 (file)
--- a/src/conf/virnwfilterbindingobj.c
+++ b/src/conf/virnwfilterbindingobj.c
@@ -88,6 +88,16 @@ virNWFilterBindingObjSetDef(virNWFilterBindingObjPtr obj,
 }
 
 
+virNWFilterBindingDefPtr
+virNWFilterBindingObjStealDef(virNWFilterBindingObjPtr obj)
+{
+    virNWFilterBindingDefPtr def;
+
+    VIR_STEAL_PTR(def, obj->def);
+    return def;
+}
+
+
 bool
 virNWFilterBindingObjGetRemoving(virNWFilterBindingObjPtr obj)
 {

diff --git a/src/conf/virnwfilterbindingobj.h b/src/conf/virnwfilterbindingobj.h
index 8e5fbee35f7d0a2ecf53d2c4da374a98b744eea2..b26bb3c8ecd21a15dc48b525988f7047a13723ca 100644 (file)
--- a/src/conf/virnwfilterbindingobj.h
+++ b/src/conf/virnwfilterbindingobj.h
@@ -39,6 +39,9 @@ void
 virNWFilterBindingObjSetDef(virNWFilterBindingObjPtr obj,
                             virNWFilterBindingDefPtr def);
 
+virNWFilterBindingDefPtr
+virNWFilterBindingObjStealDef(virNWFilterBindingObjPtr obj);
+
 bool
 virNWFilterBindingObjGetRemoving(virNWFilterBindingObjPtr obj);
 

diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c
index 06ccbf53afdfffe4649e340451d1612a9ad9e1fe..4ee2c1b194b855c884784780e8bd352443ade4cf 100644 (file)
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -167,6 +167,7 @@ virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings,
                                    virNWFilterBindingDefPtr def)
 {
     virNWFilterBindingObjPtr binding;
+    bool stealDef = false;
 
     /* See if a binding with matching portdev already exists */
     if ((binding = virNWFilterBindingObjListFindByPortDevLocked(
@@ -181,6 +182,7 @@ virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings,
         goto error;
 
     virNWFilterBindingObjSetDef(binding, def);
+    stealDef = true;
 
     if (virNWFilterBindingObjListAddObjLocked(bindings, binding) < 0)
         goto error;
@@ -188,6 +190,8 @@ virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings,
     return binding;
 
  error:
+    if (stealDef)
+        virNWFilterBindingObjStealDef(binding);
     virNWFilterBindingObjEndAPI(&binding);
     return NULL;
 }

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 26f10bd47fcec61d5c781b8609f8c05917d3b3a5..a33f9e61b147eb0cd883d6805127a32ea7e963f8 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1101,6 +1101,7 @@ virNWFilterBindingObjParseFile;
 virNWFilterBindingObjSave;
 virNWFilterBindingObjSetDef;
 virNWFilterBindingObjSetRemoving;
+virNWFilterBindingObjStealDef;
 
 
 # conf/virnwfilterbindingobjlist.h