[libvirt] [PATCH 8/8] secret: Handle object list removal and deletion properly
Rather than rely on virSecretObjEndAPI to make the final virObjectUnref
after the call virSecretObjListRemove, be more explicit by calling
virObjectUnref and setting @obj to NULL for secretUndefine and in
the error path of secretDefineXML.
This also fixes a leak during virSecretLoad if the virSecretLoadValue
fails the code jumps to cleanup without setting @ret = obj, thus calling
virSecretObjListRemove which only accounts for the object reference
related to adding the object to the list during virSecretObjListAdd,
but does not account for the reference to the object itself as the
return of @ret would be NULL so the caller wouldn't call virSecretObjEndAPI
on the object recently added thus reducing the refcnt to zero. Thus
cleaning up the virSecretLoadValue error path to make it clearer what
needs to be done on failure.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 14 ++++++--------
src/secret/secret_driver.c | 9 +++++++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index ca13cf8..26646dd 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -918,7 +918,6 @@ virSecretLoad(virSecretObjListPtr secrets,
{
virSecretDefPtr def = NULL;
virSecretObjPtr obj = NULL;
- virSecretObjPtr ret = NULL;
if (!(def = virSecretDefParseFile(path)))
goto cleanup;
@@ -930,16 +929,15 @@ virSecretLoad(virSecretObjListPtr secrets,
goto cleanup;
def = NULL;
- if (virSecretLoadValue(obj) < 0)
- goto cleanup;
-
- ret = obj;
- obj = NULL;
+ if (virSecretLoadValue(obj) < 0) {
+ virSecretObjListRemove(secrets, obj);
+ virObjectUnref(obj);
+ obj = NULL;
+ }
cleanup:
- virSecretObjListRemove(secrets, obj);
virSecretDefFree(def);
- return ret;
+ return obj;
}
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 32cd8bb..14483fd 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -268,10 +268,13 @@ secretDefineXML(virConnectPtr conn,
* the backup. The current def will be handled below.
* Otherwise, this is a new secret, thus remove it.
*/
- if (backup)
+ if (backup) {
virSecretObjSetDef(obj, backup);
- else
+ } else {
virSecretObjListRemove(driver->secrets, obj);
+ virObjectUnref(obj);
+ obj = NULL;
+ }
cleanup:
virSecretDefFree(def);
@@ -411,6 +414,8 @@ secretUndefine(virSecretPtr secret)
virSecretObjDeleteData(obj);
virSecretObjListRemove(driver->secrets, obj);
+ virObjectUnref(obj);
+ obj = NULL;
ret = 0;
--
2.9.4