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>
This commit is contained in:
Michal Privoznik 2019-03-20 09:59:48 +01:00
parent 971872ca27
commit 8c08a99745
4 changed files with 18 additions and 0 deletions

View File

@ -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)
{

View File

@ -39,6 +39,9 @@ void
virNWFilterBindingObjSetDef(virNWFilterBindingObjPtr obj,
virNWFilterBindingDefPtr def);
virNWFilterBindingDefPtr
virNWFilterBindingObjStealDef(virNWFilterBindingObjPtr obj);
bool
virNWFilterBindingObjGetRemoving(virNWFilterBindingObjPtr obj);

View File

@ -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;
}

View File

@ -1101,6 +1101,7 @@ virNWFilterBindingObjParseFile;
virNWFilterBindingObjSave;
virNWFilterBindingObjSetDef;
virNWFilterBindingObjSetRemoving;
virNWFilterBindingObjStealDef;
# conf/virnwfilterbindingobjlist.h