After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 746602 - Invalid writes through dangling weak pointers in DND code cause crashes
Invalid writes through dangling weak pointers in DND code cause crashes
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.14.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-03-22 10:53 UTC by Tom Hughes
Modified: 2015-03-22 15:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to remove weak pointers when they are no longer needed (2.50 KB, patch)
2015-03-22 10:55 UTC, Tom Hughes
committed Details | Review

Description Tom Hughes 2015-03-22 10:53:31 UTC
Starting with GTK 3.14.8 an appliction (gramps) that makes heavy use of DND was experiencing frequent crashes that appeared to be caused by earlier memory corruption. Investigation with valgrind led to reports like this:

==29736== Invalid write of size 8
==29736==    at 0x7481CE15: g_nullify_pointer (gutils.c:2028 in /usr/lib64/libglib-2.0.so.0.4200.1)
==29736==    by 0x74562BDE: weak_refs_notify (gobject.c:2630 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x74565718: g_object_run_dispose (gobject.c:1076 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x7599B476: gtk_box_forall (gtkbox.c:2558 in /usr/lib64/libgtk-3.so.0.1400.8)
==29736==    by 0x759E299F: gtk_container_destroy (gtkcontainer.c:1409 in /usr/lib64/libgtk-3.so.0.1400.8)
==29736==    by 0x7455EC8E: g_closure_invoke (gclosure.c:768 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x74570E15: signal_emit_unlocked_R (gsignal.c:3669 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x74579180: g_signal_emit_valist (gsignal.c:3309 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x745793AE: g_signal_emit (gsignal.c:3365 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x75BCB2D7: gtk_widget_dispose (gtkwidget.c:11923 in /usr/lib64/libgtk-3.so.0.1400.8)
==29736==    by 0x74565718: g_object_run_dispose (gobject.c:1076 in /usr/lib64/libgobject-2.0.so.0.4200.1)
==29736==    by 0x75BD3998: gtk_window_forall (gtkwindow.c:7957 in /usr/lib64/libgtk-3.so.0.1400.8)
==29736==  Address 0x7f05abc0 is 0 bytes inside an unallocated block of size 48 in arena "client"

As you can see it appears that a weak pointer added with g_object_add_weak_pointer is being cleared because the object it pointed to is being destroyed, but the pointer is no longer valid.

A systematic review of all uses of g_object_add_weak_pointer in GTK led to commit 650c25e06c175c3c35782acbdd5db784372f126c which added weak pointers from GtkDragDestInfo to the destination widget but which does not remove those weak pointers again when the destination widget changes or the GtkDragDestInfo object is destroyed because the drag is complete.

The result is that when the destination widget is eventually destroyed zero is written to an arbitrary bit of memory and eventually something will crash as a result.
Comment 1 Tom Hughes 2015-03-22 10:55:49 UTC
Created attachment 300064 [details] [review]
Patch to remove weak pointers when they are no longer needed

This patch attempts to add appropriate g_object_remove_weak_pointer calls where they are needed.

Running the application where the crashes were observed with this applied appears to be successful so far.
Comment 2 Matthias Clasen 2015-03-22 15:00:57 UTC
Thanks for the detective work! Looks right to me.