If virStoragePoolObjNew() fails to create new volume object list
then virObjectUnref() is called and since refcounter is 1 then
virStoragePoolObjDispose() is called which in turn calls
virStoragePoolObjClearVols() which in turn dereferences
obj->volumes.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/virstorageobj.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 1d6c9d1937..1d5c88f50b 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -365,8 +365,10 @@ virStoragePoolObjDispose(void *opaque)
     if (!obj)
         return;
 
-    virStoragePoolObjClearVols(obj);
-    virObjectUnref(obj->volumes);
+    if (obj->volumes) {
+        virStoragePoolObjClearVols(obj);
+        virObjectUnref(obj->volumes);
+    }
 
     virStoragePoolDefFree(obj->def);
     virStoragePoolDefFree(obj->newDef);
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, May 14, 2019 at 11:24:09AM +0200, Michal Privoznik wrote:
> If virStoragePoolObjNew() fails to create new volume object list
> then virObjectUnref() is called and since refcounter is 1 then
> virStoragePoolObjDispose() is called which in turn calls
> virStoragePoolObjClearVols() which in turn dereferences
> obj->volumes.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virstorageobj.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index 1d6c9d1937..1d5c88f50b 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -365,8 +365,10 @@ virStoragePoolObjDispose(void *opaque)
>      if (!obj)
>          return;
>
> -    virStoragePoolObjClearVols(obj);
> -    virObjectUnref(obj->volumes);
> +    if (obj->volumes) {
> +        virStoragePoolObjClearVols(obj);
> +        virObjectUnref(obj->volumes);

I think the check is better suited to live inside virStoragePoolObjClearVols as
there are multiple callers to virStoragePoolObjClearVols, just to be on the
safer side.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list