Re: [libvirt] [PATCH v2 3/4] secret: Properly handle @def after virSecretObjAdd in driver

On 07/25/2017 07:36 AM, Pavel Hrdina wrote: > On Fri, Jul 14, 2017 at 10:04:41AM -0400, John Ferlan wrote: >> Since the virSecretObjListAdd technically consumes @def on success, >> the secretDefineXML should set @def = NULL immediately and process >> the remaining calls using a new @objdef variable. We can use use >> VIR_STEAL_PTR since we know the Add function just stores @def in >> obj->def. >> >> This fixes a possible double free of @def if the code jumps to >> restore_backup: and calls virSecretObjListRemove without setting >> def = NULL. In this case, the subsequent call to DefFree would >> succeed and free @def; however, the call to EndAPI would also >> call DefFree because the Unref done would be the last one for >> the @obj meaning the obj->def would be used to call DefFree, >> but it's already been free'd because @def wasn't managed right >> within this error path. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/secret/secret_driver.c | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c >> index 30124b4..77351d8 100644 >> --- a/src/secret/secret_driver.c >> +++ b/src/secret/secret_driver.c >> @@ -210,6 +210,7 @@ secretDefineXML(virConnectPtr conn, >> { >> virSecretPtr ret = NULL; >> virSecretObjPtr obj = NULL; >> + virSecretDefPtr objdef; > > s/objdef/objDef/ Why? I've been using objdef in general and not the camel case one

John > > Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >