[PATCH v3] virtio-gpu-virgl: correct parent for blob memory region

Joelle van Dyne posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260103214400.71694-1-j@getutm.app
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/display/virtio-gpu-virgl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v3] virtio-gpu-virgl: correct parent for blob memory region
Posted by Joelle van Dyne 1 month ago
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 <j@getutm.app>
---
 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 18404be5892..b8a0af702e0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -123,7 +123,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);
 
@@ -189,7 +189,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;
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v3] virtio-gpu-virgl: correct parent for blob memory region
Posted by Akihiko Odaki 1 month ago
On 2026/01/04 6:43, Joelle van Dyne wrote:
> 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 <j@getutm.app>

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>