ivshmem: fix memory backend leak

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Drop the original reference from object_new() once backend is attached to its parent.

Signed-off-by Igor Mammedov <[email protected]>

---
 hw/misc/ivshmem.c | 1 +
 1 file changed, 1 insertion(+)

hw/misc/ivshmem.c +1 -1

index f88910e..ecfd10a 100644
@@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
     object_property_set_bool(obj, true, "share", &error_abort);
     object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
                               &error_abort);
+    object_unref(obj);
     user_creatable_complete(obj, &error_abort);
     s->hostmem = MEMORY_BACKEND(obj);
 }
--

2.7.4

Quoted 8 lines · Igor Mammedov

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Quoted 4 lines

Drop the original reference from object_new() once backend is attached to its parent.

Signed-off-by Igor Mammedov <[email protected]>

I would rather have the unref in finalize, but that is ok too.

Reviewed-by Marc-André Lureau <[email protected]>

Quoted 20 lines

---
 hw/misc/ivshmem.c | 1 +
 1 file changed, 1 insertion(+)

hw/misc/ivshmem.c +1 -1

index f88910e..ecfd10a 100644
@@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
     object_property_set_bool(obj, true, "share", &error_abort);
     object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
                               &error_abort);
+    object_unref(obj);
     user_creatable_complete(obj, &error_abort);
     s->hostmem = MEMORY_BACKEND(obj);
 }
--

2.7.4

Quoted 9 lines · On Thu, 1 Nov 2018 15:02:04 +0400 Marc-André Lureau…

Quoted 8 lines · Igor Mammedov

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Quoted 3 lines

Quoted 3 lines

Drop the original reference from object_new() once backend is attached to its parent.

Quoted 3 lines

Quoted 1 line

Signed-off-by Igor Mammedov <[email protected]>

I would rather have the unref in finalize, but that is ok too.

I followed the pattern we use else where, i.e. drop reference as soon as we set the parent (virtio-rng/cpus) within the same scope as object_new().

There is no point in keeping reference until finalize time since backend is kept alive as child and is destroyed well after all nonexistent ivshmem::unrealize/finilize() are finished when generic Object is being destroyed.

Quoted 1 line

Reviewed-by Marc-André Lureau <[email protected]>

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't

destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Drop the original reference from object_new() once backend is attached to its parent.

Signed-off-by Igor Mammedov <[email protected]>

---
  hw/misc/ivshmem.c | 1 +
  1 file changed, 1 insertion(+)

hw/misc/ivshmem.c +1 -0

index f88910e..ecfd10a 100644
@@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
      object_property_set_bool(obj, true, "share", &error_abort);
      object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
                                &error_abort);
+    object_unref(obj);
      user_creatable_complete(obj, &error_abort);
      s->hostmem = MEMORY_BACKEND(obj);
  }

Quoted 3 lines · Igor Mammedov

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't

^ "destroyed"

Quoted 26 lines

destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Drop the original reference from object_new() once backend is attached to its parent.

Signed-off-by Igor Mammedov <[email protected]>

---
  hw/misc/ivshmem.c | 1 +
  1 file changed, 1 insertion(+)

hw/misc/ivshmem.c +1 -0

index f88910e..ecfd10a 100644
@@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
      object_property_set_bool(obj, true, "share", &error_abort);
      object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
                                &error_abort);
+    object_unref(obj);
      user_creatable_complete(obj, &error_abort);
      s->hostmem = MEMORY_BACKEND(obj);
  }

object_new() returns a new backend with refcount == 1 and then later object_property_add_child() increases refcount to 2 So when ivshmem is desroyed, the backend it has created isn't destroyed along with it as children cleanup will bring backend's refcount only to 1, which leaks backend including resources it is using.

Drop the original reference from object_new() once backend is attached to its parent.

Signed-off-by Igor Mammedov <[email protected]>

---
 hw/misc/ivshmem.c | 1 +
 1 file changed, 1 insertion(+)

hw/misc/ivshmem.c +0 -0

index f88910e..ecfd10a 100644