incorrect reference-counting related to gtk::Widget::parent()

[collinmay]

Bug description

There seems to be some kind of reference-counting mishap near gtk::Widget::parent() that only happens under specific circumstances that I do not yet fully understand. Sample code is included below. It segfaults on my system. If this.parent() is replaced by this.property::<gtk::Widget>("parent"), it does not segfault and seems to behave correctly. If the model is provided during construction of the ColumnView instead of set later, it also doesn't crash.

use gtk::prelude::*;
use gtk::subclass::prelude::*;
use gtk::glib;
use gtk::gio;

glib::wrapper! {
    pub struct CustomWidget(ObjectSubclass<imp::CustomWidget>)
        @extends gtk::Widget,
        @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget;
}

mod imp {
    use super::*;

    #[derive(Default)]
    pub struct CustomWidget {
    }

    #[glib::object_subclass]
    impl ObjectSubclass for CustomWidget {
        const NAME: &'static str = "CustomWidget";
        type Type = super::CustomWidget;
        type ParentType = gtk::Widget;
    }

    impl ObjectImpl for CustomWidget {
        fn constructed(&self) {
            self.parent_constructed();

            println!("custom widget constructed");
            
            self.obj().connect_parent_notify(|this| {
                println!("custom widget reparented");
                let _ = this.parent(); // <-- bug happens here
            });
        }
    }

    impl WidgetImpl for CustomWidget {
    }
}

fn main() {
    gtk::init().unwrap();
    CustomWidget::ensure_type();

    let template = br#"
<?xml version='1.0' encoding='UTF-8'?>
<interface>
  <template class="GtkListItem">
    <property name="child">
      <object class="CustomWidget">
      </object>
    </property>
  </template>
</interface>
"#;

    let store = gio::ListStore::new::<glib::Object>();
    store.append(&glib::Object::new::<glib::Object>());
    let model = gtk::NoSelection::new(Some(store));
    let cv = gtk::ColumnView::new(Option::<gtk::SelectionModel>::None);
    
    cv.append_column(
        &gtk::ColumnViewColumn::builder()
            .expand(true)
            .resizable(true)
            .title("test")
            .factory(&gtk::BuilderListItemFactory::from_bytes(
                gtk::BuilderScope::NONE,
                &glib::Bytes::from_static(template)))
            .build());

    let window = gtk::Window::new();
    window.set_child(Some(&cv));
    cv.set_model(Some(&model));
}

Investigation/backtraces

By setting a breakpoint on println!("custom widget reparented");, I can pull out the address of the parent by looking in the backtrace:

(gdb) break 33
Breakpoint 2 at 0x5555555e3903: file src/main.rs, line 33.
(gdb) c
Continuing.
custom widget constructed

Thread 1 "gtk-rs-parent-u" hit Breakpoint 2, gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure#0} (this=0x7fffffffb9e0) at src/main.rs:33
33	                println!("custom widget reparented");
(gdb) bt
#0  gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure#0} (this=0x7fffffffba20) at src/main.rs:33
#1  0x00005555555e5ae2 in gtk4::auto::widget::WidgetExt::connect_parent_notify::notify_parent_trampoline<gtk_rs_parent_uaf_demo::CustomWidget, gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure_env#0}>
    (this=0x555555736340, _param_spec=0x5555556bf990, f=0x1) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gtk4-0.9.6/src/auto/widget.rs:2461
#2  0x00007ffff6f8c82a in g_closure_invoke () at /usr/lib/libgobject-2.0.so.0
#3  0x00007ffff6fbd565 in ??? () at /usr/lib/libgobject-2.0.so.0
#4  0x00007ffff6fadca9 in ??? () at /usr/lib/libgobject-2.0.so.0
#5  0x00007ffff6fadf32 in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#6  0x00007ffff6fadff4 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#7  0x00007ffff6f98d16 in ??? () at /usr/lib/libgobject-2.0.so.0
#8  0x00005555555e00ab in glib::subclass::object::ObjectImplExt::parent_dispatch_properties_changed<gtk_rs_parent_uaf_demo::imp::CustomWidget> (self=0x5555557361f0, pspecs=...)
    at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/subclass/object.rs:302
#9  0x00005555555dfdd7 in glib::subclass::object::ObjectImpl::dispatch_properties_changed<gtk_rs_parent_uaf_demo::imp::CustomWidget> (self=0x5555557361f0, pspecs=...)
    at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/subclass/object.rs:85
#10 0x00005555555e53c5 in glib::subclass::object::dispatch_properties_changed<gtk_rs_parent_uaf_demo::imp::CustomWidget> (obj=0x555555736340, n_pspecs=1, pspecs=0x7fffffffc0d0)
    at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/subclass/object.rs:152
#11 0x00007ffff6f9c544 in g_object_notify_by_pspec () at /usr/lib/libgobject-2.0.so.0
#12 0x00007ffff759bd40 in gtk_widget_reposition_after (widget=0x555555736340, parent=0x555555743120, previous_sibling=0x0) at ../gtk/gtkwidget.c:6105
#13 0x00007ffff759bfd1 in gtk_widget_set_parent (widget=0x555555736340, parent=0x555555743120) at ../gtk/gtkwidget.c:6166
(gdb) up 13
#13 0x00007ffff759bfd1 in gtk_widget_set_parent (widget=0x555555738360, parent=0x555555745040) at ../gtk/gtkwidget.c:6166
6166	  gtk_widget_reposition_after (widget,
(gdb) p *parent
$2 = {parent_instance = {g_type_instance = {g_class = Python Exception <class 'gdb.error'>: No type named TypeNode.
}, ref_count = 1, qdata = 0x2}, priv = 0x555555742fd0}

I set a watchpoint on the reference count. It is incremented from 1 to 2 by the call to parent().

(gdb) watch -l parent->parent_instance.ref_count
Hardware watchpoint 3: -location parent->parent_instance.ref_count
(gdb) c
Continuing.
custom widget reparented

Thread 1 "gtk-rs-parent-u" hit Hardware watchpoint 2: -location parent->parent_instance.ref_count

Old value = 1
New value = 2
0x00007ffff6f97840 in ?? () from /usr/lib/libgobject-2.0.so.0
(gdb) bt
#0  0x00007ffff6f97840 in ??? () at /usr/lib/libgobject-2.0.so.0
#1  0x00007ffff6f993dc in g_object_ref () at /usr/lib/libgobject-2.0.so.0
#2  0x00007ffff6f9c58f in g_object_ref_sink () at /usr/lib/libgobject-2.0.so.0
#3  0x00005555555f715e in glib::object::{impl#13}::from_glib_none (ptr=0x555555743120) at src/object.rs:475
#4  0x00005555555fb35a in glib::translate::from_glib_none<*mut gobject_sys::GObject, glib::object::ObjectRef> (ptr=0x555555743120) at src/translate.rs:1630
#5  0x00005555555eefdc in gtk4::auto::widget::{impl#30}::from_glib_none (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/object.rs:897
#6  0x00005555555ee19a in glib::translate::from_glib_none<*mut gtk4_sys::GtkWidget, gtk4::auto::widget::Widget> (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1630
#7  0x00005555555ee643 in glib::translate::{impl#62}::from_glib_none<*mut gtk4_sys::GtkWidget, gtk4::auto::widget::Widget> (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1657
#8  0x00005555555ee1ab in glib::translate::from_glib_none<*mut gtk4_sys::GtkWidget, core::option::Option<gtk4::auto::widget::Widget>> (ptr=0x555555743120)
    at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1630
#9  0x00005555555ed054 in gtk4::auto::widget::WidgetExt::parent<gtk_rs_parent_uaf_demo::CustomWidget> (self=0x7fffffffba20) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gtk4-0.9.6/src/auto/widget.rs:662
#10 0x00005555555e1e1a in gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure#0} (this=0x7fffffffba20) at src/main.rs:34

Then from 2 back to 1 in the same call to parent(). This seems incorrect to me.

(gdb) c
Continuing.

Thread 1 "gtk-rs-parent-u" hit Hardware watchpoint 2: -location parent->parent_instance.ref_count

Old value = 2
New value = 1
0x00007ffff6f9c0f1 in g_object_unref () from /usr/lib/libgobject-2.0.so.0
(gdb) bt
#0  0x00007ffff6f9c0f1 in g_object_unref () at /usr/lib/libgobject-2.0.so.0
#1  0x00007ffff6f9c609 in g_object_ref_sink () at /usr/lib/libgobject-2.0.so.0
#2  0x00005555555f715e in glib::object::{impl#13}::from_glib_none (ptr=0x555555743120) at src/object.rs:475
#3  0x00005555555fb35a in glib::translate::from_glib_none<*mut gobject_sys::GObject, glib::object::ObjectRef> (ptr=0x555555743120) at src/translate.rs:1630
#4  0x00005555555eefdc in gtk4::auto::widget::{impl#30}::from_glib_none (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/object.rs:897
#5  0x00005555555ee19a in glib::translate::from_glib_none<*mut gtk4_sys::GtkWidget, gtk4::auto::widget::Widget> (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1630
#6  0x00005555555ee643 in glib::translate::{impl#62}::from_glib_none<*mut gtk4_sys::GtkWidget, gtk4::auto::widget::Widget> (ptr=0x555555743120) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1657
#7  0x00005555555ee1ab in glib::translate::from_glib_none<*mut gtk4_sys::GtkWidget, core::option::Option<gtk4::auto::widget::Widget>> (ptr=0x555555743120)
    at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glib-0.20.9/src/translate.rs:1630
#8  0x00005555555ed054 in gtk4::auto::widget::WidgetExt::parent<gtk_rs_parent_uaf_demo::CustomWidget> (self=0x7fffffffba20) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gtk4-0.9.6/src/auto/widget.rs:662
#9  0x00005555555e1e1a in gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure#0} (this=0x7fffffffba20) at src/main.rs:34

Then from 1 to 0 at the end of the closure, causing the parent object to be freed early and UAF'd.

(gdb) c
Continuing.

Thread 1 "gtk-rs-parent-u" hit Hardware watchpoint 2: -location parent->parent_instance.ref_count

Old value = 1
New value = 0
0x00007ffff6f9c1a9 in g_object_unref () from /usr/lib/libgobject-2.0.so.0
(gdb) bt
#0  0x00007ffff6f9c1a9 in g_object_unref () at /usr/lib/libgobject-2.0.so.0
#1  0x00005555555f70d7 in glib::object::{impl#4}::drop (self=0x7fffffffb9e0) at src/object.rs:394
#2  0x00005555555f4d0a in core::ptr::drop_in_place<glib::object::ObjectRef> () at /home/mayco/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:574
#3  0x00005555555f520b in core::ptr::drop_in_place<glib::object::TypedObjectRef<*mut core::ffi::c_void, ()>> () at /home/mayco/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:574
#4  0x00005555555eebcb in core::ptr::drop_in_place<gtk4::auto::widget::Widget> () at /home/mayco/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:574
#5  0x00005555555ddd66 in core::ptr::drop_in_place<core::option::Option<gtk4::auto::widget::Widget>> () at /home/mayco/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:574
#6  0x00005555555e1e29 in gtk_rs_parent_uaf_demo::imp::{impl#0}::constructed::{closure#0} (this=0x7fffffffba20) at src/main.rs:34

GDB backtrace of the segfault (unhelpful, since that's not where the bug actually occurs, but here for completeness):

#0  0x00007ffff758f23c in _gtk_widget_get_visible (widget=0x0) at ../gtk/gtkwidgetprivate.h:391
#1  0x00007ffff759bd53 in gtk_widget_reposition_after (widget=0x555555736340, parent=0x555555743120, previous_sibling=0x0) at ../gtk/gtkwidget.c:6108
#2  0x00007ffff759bfd1 in gtk_widget_set_parent (widget=0x555555736340, parent=0x555555743120) at ../gtk/gtkwidget.c:6166
#3  0x00007ffff75ebc98 in gtk_column_view_cell_widget_set_child (self=0x555555743120, child=0x555555736340) at ../gtk/gtkcolumnviewcellwidget.c:420
#4  0x00007ffff7360c0f in gtk_column_view_cell_set_child (self=0x5555557b9b60, child=0x555555736340) at ../gtk/gtkcolumnviewcell.c:321
#5  0x00007ffff73606f8 in gtk_column_view_cell_set_property (object=0x5555557b9b60, property_id=1, value=0x5555556f8c00, pspec=0x5555557b98c0) at ../gtk/gtkcolumnviewcell.c:134
#6  0x00007ffff6f9df0b in ??? () at /usr/lib/libgobject-2.0.so.0
#7  0x00007ffff6fa0ff3 in g_object_setv () at /usr/lib/libgobject-2.0.so.0
#8  0x00007ffff732c094 in _gtk_builder_apply_properties (builder=0x5555557ba230, info=0x5555557ba880, error=0x7fffffffc600) at ../gtk/gtkbuilder.c:1130
#9  0x00007ffff7333707 in builder_construct (data=0x7fffffffc740, object_info=0x5555557ba880, error=0x7fffffffc600) at ../gtk/gtkbuilderparser.c:484
#10 0x00007ffff73376d9 in end_element (context=0x7fffffffc768, element_name=0x5555556f1605 "template", user_data=0x7fffffffc740, error=0x7fffffffc600) at ../gtk/gtkbuilderparser.c:1950
#11 0x00007ffff7332e2b in proxy_end_element (gm_context=0x0, element_name=0x5555556f1605 "template", user_data=0x7fffffffc768, error=0x7fffffffc600) at ../gtk/gtkbuilderparser.c:104
#12 0x00007ffff75e3fdf in replay_end_element (context=0x7fffffffc768, tree=0x7fffffffc660, strings=0x5555556f15f5 "class", error=0x7fffffffc858) at ../gtk/gtkbuilderprecompile.c:660
#13 0x00007ffff75e420c in _gtk_buildable_parser_replay_precompiled (context=0x7fffffffc768, data=0x5555556f1652 "", data_len=124, error=0x7fffffffc858) at ../gtk/gtkbuilderprecompile.c:742
#14 0x00007ffff7333073 in gtk_buildable_parse_context_parse (context=0x7fffffffc768, text=0x5555556f15f0 "GBU", text_len=124, error=0x7fffffffc858) at ../gtk/gtkbuilderparser.c:191
#15 0x00007ffff7338569 in _gtk_builder_parser_parse_buffer (builder=0x5555557ba230, filename=0x5555557ba740 "<GtkListItem template>", buffer=0x5555556f15f0 "GBU", length=124, requested_objs=0x0, error=0x7fffffffc858) at ../gtk/gtkbuilderparser.c:2205
#16 0x00007ffff732d0e4 in gtk_builder_extend_with_template (builder=0x5555557ba230, object=0x5555557b9b60, template_type=Python Exception <class 'gdb.error'>: No type named TypeNode.
, buffer=0x5555556f15f0 "GBU", length=124, error=0x7fffffffc8e0) at ../gtk/gtkbuilder.c:1562
#17 0x00007ffff733204d in gtk_builder_list_item_factory_setup (factory=0x55555570b740, item=0x5555557b9b60, bind=0, func=0x7ffff74375fa <gtk_list_factory_widget_setup_func>, data=0x555555743120) at ../gtk/gtkbuilderlistitemfactory.c:109
#18 0x00007ffff743c3fc in gtk_list_item_factory_setup (self=0x55555570b740, item=0x5555557b9b60, bind=0, func=0x7ffff74375fa <gtk_list_factory_widget_setup_func>, data=0x555555743120) at ../gtk/gtklistitemfactory.c:137
#19 0x00007ffff74376ab in gtk_list_factory_widget_setup_factory (self=0x555555743120) at ../gtk/gtklistfactorywidget.c:108
#20 0x00007ffff743888b in gtk_list_factory_widget_set_factory (self=0x555555743120, factory=0x55555570b740) at ../gtk/gtklistfactorywidget.c:550
#21 0x00007ffff7437a9b in gtk_list_factory_widget_set_property (object=0x555555743120, property_id=2, value=0x7fffffffcd60, pspec=0x5555557484a0) at ../gtk/gtklistfactorywidget.c:224
#22 0x00007ffff6f9df0b in ??? () at /usr/lib/libgobject-2.0.so.0
#23 0x00007ffff6f9e893 in ??? () at /usr/lib/libgobject-2.0.so.0
#24 0x00007ffff6fa0afb in g_object_new_valist () at /usr/lib/libgobject-2.0.so.0
#25 0x00007ffff6fa0eb0 in g_object_new () at /usr/lib/libgobject-2.0.so.0
#26 0x00007ffff75ebad4 in gtk_column_view_cell_widget_new (column=0x55555571ae30, inert=0) at ../gtk/gtkcolumnviewcellwidget.c:367
#27 0x00007ffff735aa87 in gtk_column_list_view_create_list_widget (base=0x555555703790) at ../gtk/gtkcolumnview.c:195
#28 0x00007ffff742ba84 in gtk_list_base_create_list_widget_func (widget=0x555555703790) at ../gtk/gtklistbase.c:1985
#29 0x00007ffff743f1fa in gtk_list_item_manager_ensure_items (self=0x555555711660, change=0x7fffffffd1d0, update_start=4294967295, update_diff=0) at ../gtk/gtklistitemmanager.c:1374
#30 0x00007ffff74409f3 in gtk_list_item_tracker_set_position (self=0x555555711660, tracker=0x555555720790, position=0, n_before=2, n_after=202) at ../gtk/gtklistitemmanager.c:1938
#31 0x00007ffff742c486 in gtk_list_base_set_anchor (self=0x555555703790, anchor_pos=0, anchor_align_across=0, anchor_side_across=GTK_PACK_START, anchor_align_along=0, anchor_side_along=GTK_PACK_START) at ../gtk/gtklistbase.c:2240
#32 0x00007ffff742c66c in gtk_list_base_set_model (self=0x555555703790, model=0x5555556a36c0) at ../gtk/gtklistbase.c:2317
#33 0x00007ffff7444992 in gtk_list_view_set_model (self=0x555555703790, model=0x5555556a36c0) at ../gtk/gtklistview.c:1081
#34 0x00007ffff735e469 in gtk_column_view_set_model (self=0x55555568fa50, model=0x5555556a36c0) at ../gtk/gtkcolumnview.c:1561
#35 0x00005555555de8a2 in gtk4::auto::column_view::ColumnView::set_model<gtk4::auto::no_selection::NoSelection> (self=0x7fffffffd420, model=...) at /home/mayco/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gtk4-0.9.6/src/auto/column_view.rs:224
#36 0x00005555555e2af5 in gtk_rs_parent_uaf_demo::main () at src/main.rs:76

[sdroege]

I don't have time to look in detail now but the problem seems to be that the parent is a floating reference at that point (it is not owned by any window or other widget yet). Receiving a floating reference from a (transfer none) context (gtk_widget_get_parent()'s return value) then takes ownership of that floating reference (that's how the rules are), and that then steals the reference from whoever is actually owning it wrongly at that point (GtkBuilder?)

It should be possible to cause the exact same crash in e.g. the Python bindings.

[collinmay]

Thanks, I think I've figured out what's going on here and reported it upstream. It's unrelated to the builder and I was able to come up with a more succinct reproducer that doesn't use the builder. Basically, the GtkColumnViewCellWidget constructor sets the factory property on itself too early while the only reference to it is still flotaing, which enables us to steal the floating reference before it is sunken where it should be (when the cell widget is added as a row widget child).

[collinmay]

Several upstream GTK maintainers seem to be of the opinion that the problem is that bindings are sinking floating references from transfer-none functions. After coming up with this reproducer, I'm inclined to agree with them that this is a problematic rule to have.

use gtk::prelude::*;
use gtk::subclass::prelude::*;
use gtk::glib;

glib::wrapper! {
    pub struct CustomWidget(ObjectSubclass<imp::CustomWidget>)
        @extends gtk::Box,
        @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget, gtk::Widget;
}

mod imp {
    use super::*;

    #[derive(Default)]
    pub struct CustomWidget { }

    #[glib::object_subclass]
    impl ObjectSubclass for CustomWidget {
        const NAME: &'static str = "CustomWidget";
        type Type = super::CustomWidget;
        type ParentType = gtk::Box;
    }

    impl ObjectImpl for CustomWidget {
        fn constructed(&self) {
            self.parent_constructed();
            let l = gtk::Label::new(None);
            self.obj().append(&l);
            let _ = l.parent();
        }
    }

    impl WidgetImpl for CustomWidget { }
    impl BoxImpl for CustomWidget { }
}

fn main() {
    gtk::init().unwrap();
    glib::Object::new::<CustomWidget>();
}

[sdroege]

Commented over there

[awused]

This is probably the same issue I reported in #1560

[bilelmoussaoui]

I guess this seems to be more of an upstream issue or can we do something about it here? given that upstream closed it