From: Joelle van Dyne <[email protected]> When `owner` == `mr`, `object_unparent` will crash:
object_unparent(mr) -> object_property_del_child(mr, mr) -> object_finalize_child_property(mr, name, mr) -> object_unref(mr) -> object_finalize(mr) -> object_property_del_all(mr) -> object_finalize_child_property(mr, name, mr) -> object_unref(mr) -> fail on g_assert(obj->ref > 0) However, passing a different `owner` to `memory_region_init` does not work. `memory_region_ref` has an optimization where it takes a ref only on the owner. That means when flatviews are created, it does not take a ref on the region and you can get a UAF from `flatview_destroy` called from RCU. The correct fix therefore is to use `NULL` as the name which will set the `owner` but not the `parent` (which is still NULL). This allows us to use `memory_region_ref` on itself while not having to rely on unparent for cleanup. Signed-off-by: Joelle van Dyne <[email protected]> Reviewed-by: Akihiko Odaki <[email protected]> Reviewed-by: Michael S. Tsirkin <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Message-Id: <[email protected]> --- hw/display/virtio-gpu-virgl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 07f6355ad6..6a83fb63c8 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -120,7 +120,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g, vmr->g = g; mr = &vmr->mr; - memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data); + memory_region_init_ram_ptr(mr, OBJECT(mr), NULL, size, data); memory_region_add_subregion(&b->hostmem, offset, mr); memory_region_set_enabled(mr, true); @@ -186,7 +186,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g, /* memory region owns self res->mr object and frees it by itself */ memory_region_set_enabled(mr, false); memory_region_del_subregion(&b->hostmem, mr); - object_unparent(OBJECT(mr)); + object_unref(OBJECT(mr)); } return 0; -- MST
