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);
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list