On 05/03/2011 11:18 PM, Laine Stump wrote: > 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 :-) Good catch. I think decrementing the count is correct. v2 coming up. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list