Peter Maydell <[email protected]> writes:

> On Wed, 4 Feb 2026 at 19:04, Michael S. Tsirkin <[email protected]> wrote:
>>
>> 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);
>
> This looks very odd. The owner of an MR should be the
> device that that MR belongs to, not the MR itself,
> and usually not NULL either. The name should be something
> useful for people looking at the HMP info output about
> memory layouts.
>
> If there's a use-after-free issue then I suspect that the right
> fix must be somewhere else, not here.

The blobs really cause issues for our MemoryRegion code because they are
transient and aren't easily cleaned up with RCU because of the dance we
have to do between qemu and virglrenderer for the underlying memory.
There have been multiple attempts to clean this up and so far I don't
think we've managed to reach a solid solution.

>
> thanks
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to