[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix pool define crash



Chris Lalancette wrote:
> Cole Robinson wrote:
>> There's a null dereference in the storage driver when defining a pool.
>> Attached patch fixes it for me.
>>
>> diff --git a/src/storage_driver.c b/src/storage_driver.c
>> index 2432a9a..ac5e443 100644
>> --- a/src/storage_driver.c
>> +++ b/src/storage_driver.c
>> @@ -546,7 +546,7 @@ storagePoolDefine(virConnectPtr conn,
>>          goto cleanup;
>>      def = NULL;
>>  
>> -    if (virStoragePoolObjSaveDef(conn, driver, pool, def) < 0) {
>> +    if (virStoragePoolObjSaveDef(conn, driver, pool, pool->def) < 0) {
>>          virStoragePoolObjRemove(&driver->pools, pool);
>>          goto cleanup;
>>      }
>>
>>
> 
> Hm, I definitely see what you are getting at, and why the crash is there, but
> I'm not sure this is totally correct.  virStoragePoolObjAssignDef() only assigns
> this def to pool->def iff the storage pool is not running.  That means that if
> the pool is running, and we make this change, for running pools you would always
> get the old def, not the updated one.  I think we need to move the "def = NULL"
> further down, but that of course will require other changes so we still have the
> unified cleanup target.
> 

Ahh, I didn't realize the AssignDef caveat. Thanks for pointing that
out. Something like this should work then?

- Cole
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 2432a9a..330317c 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -544,12 +544,13 @@ storagePoolDefine(virConnectPtr conn,
 
     if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def)))
         goto cleanup;
-    def = NULL;
 
     if (virStoragePoolObjSaveDef(conn, driver, pool, def) < 0) {
         virStoragePoolObjRemove(&driver->pools, pool);
+        def = NULL;
         goto cleanup;
     }
+    def = NULL;
 
     ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
 

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]