On 2025/12/24 16:46, Joelle van Dyne wrote:
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?
The intention here is that the reference counting of memory regions
independent of the device. If you make the device owner, the reference
counter of the device will be used for memory region (see
memory_region_ref()). But this memory region can go away while the
device stays, so its reference counting needs to be done independently.
By making the memory region itself owner, the reference counter of the
memory region itself will be used, just like normal objects.
So the owner needs to be the memory region itself. But it is not
necessary to have the parent/child relationship, and passing NULL as
name only removes the relationship.
Regards,
Akihiko Odaki
