[PATCH 0/2] improve qom debugging and fix a dangling MemoryRegion in rutabaga

Roman Kiryanov posted 2 patches 3 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260427154441.1918536-1-rkir@google.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
hw/display/virtio-gpu-rutabaga.c | 1 +
qom/object.c                     | 3 +++
2 files changed, 4 insertions(+)
[PATCH 0/2] improve qom debugging and fix a dangling MemoryRegion in rutabaga
Posted by Roman Kiryanov 3 weeks, 6 days ago
Hi, this series of patches fixes a dangling MemoryRegion
in rutabaga and also brings the debugging changes I used
to locate it.

Roman Kiryanov (2):
  qom: improve use-after-free debugging
  display: rutabaga: unparent MemoryRegions in unmap

 hw/display/virtio-gpu-rutabaga.c | 1 +
 qom/object.c                     | 3 +++
 2 files changed, 4 insertions(+)

-- 
2.54.0.rc2.544.gc7ae2d5bb8-goog
[PATCH v2 0/2] improve qom debugging and fix a dangling MemoryRegion in rutabaga
Posted by Roman Kiryanov 3 weeks, 5 days ago
Hi, this series of patches fixes a dangling MemoryRegion
in rutabaga and also brings the debugging changes I used
to locate it.

Changes in v2:
- Updated clearing of obj->properties using g_clear_pointer.
- Moved clearing of obj->class into object_deinit.
- obj->class is checked to be NULL before obj->free(obj).

Roman Kiryanov (2):
  qom: improve use-after-free debugging
  display: rutabaga: unparent MemoryRegions in unmap

 hw/display/virtio-gpu-rutabaga.c | 1 +
 qom/object.c                     | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.54.0.545.g6539524ca2-goog
[PATCH v2 1/2] qom: improve use-after-free debugging
Posted by Roman Kiryanov 3 weeks, 5 days ago
This patch invalidates dead objects so their usage will
lead to more predictable results (crashes or asserts).

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
Changes in v2:
- Updated clearing of obj->properties using g_clear_pointer.
- Moved clearing of obj->class into object_deinit.
- obj->class is checked to be NULL before obj->free(obj).

 qom/object.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index f981e27044..b8ffb00976 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -612,7 +612,7 @@ static void object_property_del_all(Object *obj)
         }
     } while (released);
 
-    g_hash_table_unref(obj->properties);
+    g_clear_pointer(&obj->properties, g_hash_table_unref);
 }
 
 static void object_property_del_child(Object *obj, Object *child)
@@ -658,6 +658,9 @@ static void object_deinit(Object *obj, TypeImpl *type)
     if (type_has_parent(type)) {
         object_deinit(obj, type_get_parent(type));
     }
+
+    g_assert(obj->properties == NULL);
+    obj->class = NULL;
 }
 
 static void object_finalize(void *data)
@@ -670,6 +673,7 @@ static void object_finalize(void *data)
 
     g_assert(obj->ref == 0);
     g_assert(obj->parent == NULL);
+    g_assert(obj->class == NULL);
     if (obj->free) {
         obj->free(obj);
     }
-- 
2.54.0.545.g6539524ca2-goog
Re: [PATCH v2 1/2] qom: improve use-after-free debugging
Posted by Philippe Mathieu-Daudé 2 weeks ago
On 28/4/26 18:45, Roman Kiryanov wrote:
> This patch invalidates dead objects so their usage will
> lead to more predictable results (crashes or asserts).
> 
> Signed-off-by: Roman Kiryanov <rkir@google.com>
> ---
> Changes in v2:
> - Updated clearing of obj->properties using g_clear_pointer.
> - Moved clearing of obj->class into object_deinit.
> - obj->class is checked to be NULL before obj->free(obj).
> 
>   qom/object.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 2/2] display: rutabaga: unparent MemoryRegions in unmap
Posted by Roman Kiryanov 3 weeks, 5 days ago
The virtio-gpu-rutabaga-device instance holds a
hash table of child objects (`Object::properties`)
and a memory region is added there every time
`memory_region_init_ram_ptr` is called.

The `unmap_blob` call invalidates a `MemoryRegion`
but does not remove it from the device, which makes
pointers to the region dangling and eventually causes
a crash when those pointers are dereferenced.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 hw/display/virtio-gpu-rutabaga.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index ebb6c783fb..5cd9b19336 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -737,6 +737,7 @@ rutabaga_cmd_resource_unmap_blob(VirtIOGPU *g,
 
         MemoryRegion *mr = &(vr->memory_regions[slot].mr);
         memory_region_del_subregion(&vb->hostmem, mr);
+        object_unparent(OBJECT(mr));
 
         vr->memory_regions[slot].resource_id = 0;
         vr->memory_regions[slot].used = 0;
-- 
2.54.0.545.g6539524ca2-goog