Re: [libvirt] [PATCH] storage: avoid null deref and leak on failure

On 05/03/2011 01:58 PM, Eric Blake wrote: > Detected by clang. NULL deref added in commit 343a27a (Mar 11), > but leak of voldef present since commit 2cd9b2d (Apr 09). > > * src/storage/storage_driver.c (storageVolumeCreateXML): Don't > leak voldef or dereference null volobj. > --- > src/storage/storage_driver.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 1ea5d12..19c7d63 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -1319,8 +1319,10 @@ storageVolumeCreateXML(virStoragePoolPtr obj, > pool->volumes.objs[pool->volumes.count++] = voldef; > volobj = virGetStorageVol(obj->conn, pool->def->name, voldef->name, > voldef->key); > + if (!volobj) > + goto cleanup; At this point, voldef has been added into pool->volumes.objs[], but voldef hasn't been NULLed out yet, so there is an extra pointer to it. If you goto cleanup, you will end up calling virStorageVolDefFree(voldef), so that object will get freed, but pool->volumes.objs still has a pointer to it. You either need to NULL out voldef, or remove it from pool->volumes.objs[] (ie, just decrement the count). I'm too tired to figure out which is more appropriate :-)