On Tue, Dec 23, 2025 at 9:32 PM Akihiko Odaki
<[email protected]> wrote:
>
> On 2025/12/24 3:40, 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` is not
> > enough. `memory_region_ref` has an optimization where it takes a ref
> > only on the owner. It specifically warns against calling unparent on
> > the memory region. So we initialize the memory region first and then
> > patch in the owner with itself.
>
> Patching outside system/memory.c can be fragile.
>
> I think an object is being a child of itself, which doesn't make sense.
> This can be avoided by passing NULL as name. The object will be an
> orphan so it will have to be freed with object_unref() instead of
> object_unparent().
I didn't want to break anything unintentionally and wanted to be safe
by making the change as close to the original logic as possible
(having introduced a UAF in v1 after making a one line change). Maybe
Antonio or Robert can give more insight on the intention of using self
as owner and if making it an orphan is acceptable?

>
> Regards,
> Akihiko Odaki

Reply via email to