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 747280 - Chrome crashes when opening system (e.g. save file) dialog after a notification window has appeared
Chrome crashes when opening system (e.g. save file) dialog after a notificati...
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: .General
2.24.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-04-03 03:17 UTC by Andy Getzendanner
Modified: 2018-05-02 16:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: NULL-check gdkwin to prevent use-after-free in gdk_window_add_filter (655 bytes, patch)
2015-04-03 03:17 UTC, Andy Getzendanner
none Details | Review
Thunar gdb bt full with gtk 2.24.28-1 (27.71 KB, text/plain)
2015-08-12 16:32 UTC, bugs
  Details
Another possible fix (1.04 KB, patch)
2016-05-12 08:38 UTC, Uli Schlachter
none Details | Review

Description Andy Getzendanner 2015-04-03 03:17:05 UTC
Created attachment 300866 [details] [review]
patch: NULL-check gdkwin to prevent use-after-free in gdk_window_add_filter

I can reliably reproduce this crash when using awesome WM by triggering a notification popup (e.g. email myself in GMail), waiting for it to go away, and then opening a file dialog (e.g. ^S).  I expect disabling Chrome notification popups will be an effective workaround, but I haven't tried that.

The attached patch fixes the problem, which seems to be a use-after-free which occurs because gdk_window_lookup_for_display returns NULL when no window is found but gdk_window_add_filter interprets gdkwin == NULL as 'all windows'.

Ubuntu  14.04
Gtk+    2.24.23-0ubuntu1.1
Chrome  40.0.2214.115
awesome v3.4.15
Comment 1 Matthias Clasen 2015-04-05 12:03:30 UTC
A stacktrace would be needed to understand what problem you are seeing.
Comment 2 Andy Getzendanner 2015-04-20 22:11:38 UTC
Program received signal SIGSEGV, Segmentation fault.
0x00007fedccc55426 in gtk_tray_icon_manager_filter (xevent=0x7fff526da1e0, event=<optimized out>, user_data=0x3df764b39080) at /build/buildd/gtk+2.0-2.24.23/gtk/gtktrayicon-x11.c:400
400	/build/buildd/gtk+2.0-2.24.23/gtk/gtktrayicon-x11.c: No such file or directory.
(gdb) bt full
  • #0 gtk_tray_icon_manager_filter
    at /build/buildd/gtk+2.0-2.24.23/gtk/gtktrayicon-x11.c line 400
  • #1 gdk_event_apply_filters
    at /build/buildd/gtk+2.0-2.24.23/gdk/x11/gdkevents-x11.c line 356
  • #2 gdk_event_translate
    at /build/buildd/gtk+2.0-2.24.23/gdk/x11/gdkevents-x11.c line 946
  • #3 _gdk_events_queue
    at /build/buildd/gtk+2.0-2.24.23/gdk/x11/gdkevents-x11.c line 2336
  • #4 gdk_event_dispatch
    at /build/buildd/gtk+2.0-2.24.23/gdk/x11/gdkevents-x11.c line 2397
  • #5 g_main_dispatch
    at /build/buildd/glib2.0-2.40.2/./glib/gmain.c line 3064
  • #6 g_main_context_dispatch
    at /build/buildd/glib2.0-2.40.2/./glib/gmain.c line 3663
  • #7 g_main_context_iterate
    at /build/buildd/glib2.0-2.40.2/./glib/gmain.c line 3734
  • #8 g_main_context_iteration
    at /build/buildd/glib2.0-2.40.2/./glib/gmain.c line 3795
  • #9 ??
  • #10 ??
  • #11 ??
  • #12 ??
  • #13 ??
  • #14 ??
  • #15 ??
  • #16 ??
  • #17 ??
  • #18 ??
  • #19 ??
  • #20 ??
  • #21 ??
  • #22 ??
  • #23 ??
  • #24 ??

Comment 3 Andy Getzendanner 2015-04-29 02:43:13 UTC
Ping; please let me know if I can help move this along e.g. by providing additional data or testing.
Comment 4 Matthias Clasen 2015-04-29 18:19:28 UTC
There have been fixes to filter handling in the 2.24 branch, such as https://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=5efefdb6550b3f00d5ca159c2ff74326bfd0e94b 

It would be nice if you could verify that the problem still happens with 2.24.27
Comment 5 Hugo Chargois 2015-07-18 17:22:58 UTC
I am experiencing the same bug, with Chrome, as stated, and also with Thunar. I am using AwesomeWM too. Here are some links to (most probably) the same bug that manifests in different programs:

Chromium: https://code.google.com/p/chromium/issues/detail?id=430910
Pidgin: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=708559
Thunar: https://bugzilla.xfce.org/show_bug.cgi?id=11035
Also reported for Thunar on AwesomeWM bugtracker: https://awesome.naquadah.org/bugs/index.php?do=details&task_id=1286

I still experience the bug with a freshly compiled 2.24.28
Comment 6 Andy Getzendanner 2015-08-11 02:03:38 UTC
Bump!  Can someone please review my patch and accept it or offer some feedback?

As described in the OP, the code in question is wrong, and the attached change fixes the crash.

I believe all requested information has been provided, but I can't change the status to anything other than NEEDINFO and RESOLVED.
Comment 7 Hugo Chargois 2015-08-11 12:07:56 UTC
BTW, I have tried Andy's patch and I can confirm that it fixes the problem for me too, I no longer have neither Chromium nor Thunar crashes.

I would love to see that bug fixed for good :)
Comment 8 infernix 2015-08-11 12:40:24 UTC
This crash has been happening to me randomly in Debian sid under libgtk2.0-0 2.24.28-1 for the last several weeks or more; I've posted a backtrace on the Debian Pidgin bugtracker in comment 5:

  • #0 gtk_tray_icon_manager_filter
    at /tmp/buildd/gtk+2.0-2.24.28/gtk/gtktrayicon-x11.c line 400

I will apply the patch and report back after several days of testing.
Comment 9 bugs 2015-08-12 16:31:03 UTC
I can confirm that this bug still exists in Debian sid with gtk 2.24.28-1 when I test Thunar and is indeed fixed with the patch attached here. Please apply! This one's been haunting me forever and is _extremely_ annoying.
Comment 10 bugs 2015-08-12 16:32:27 UTC
Created attachment 309167 [details]
Thunar gdb bt full with gtk 2.24.28-1
Comment 11 infernix 2015-08-17 13:16:05 UTC
Saved a dozen times since I applied the patch and no crashes, so I can confirm this solves the issue in Debian sid.
Comment 12 Roman Sokolkov 2015-08-17 15:37:35 UTC
Works for me too. Rebuilt gtk2 with Andy's patch and unable to reproduce the issue.

Arch:
awesome 3.5.6-2
gtk2 2.24.28-1
google-chrome 42.0.2311.90-1

BTW, Andy, thanks a lot. This issue annoyed me for a half of the year.
Comment 13 Emmanuele Bassi (:ebassi) 2015-08-17 15:52:17 UTC
The problem with accepting the patch as is is that it may be masking some deeper issues.

Why is the window associated with the selection owner not being found in the first place?
Comment 14 Andy Getzendanner 2015-08-20 00:26:31 UTC
I don't understand X or GTK well enough to diagnose or fix any deeper issues.

Unless someone who does can look at this soon, please consider accepting my patch in the mean time as it makes the situation much better for affected users and has no known or anticipated drawbacks.  This bug can be left open to track a deeper investigation if appropriate.

Thanks!
Comment 15 Ben Chociej 2015-09-03 04:17:50 UTC
An additional data point: I can reliably reproduce this crash in Gentoo with awesome-3.4.15, gtk+-2.24.28-r1, and google-chrome-45.0.2454.85_p1, and Andy's patch fixes the issue for me as well.

It may be worth noting that, at the moment the crash occurs, Chrome shows no tray icons in awesomewm's tray; i.e., I have dismissed any open desktop notifications, and the "alarm bell" icon has disappeared from the tray.
Comment 16 Jason Yan 2015-09-17 20:12:56 UTC
For any Ubuntu users, I have a branch with the patch applied and instructions so you can rebuild the package: https://github.com/tail/ubuntu-gtk/tree/gnome-bugzilla-747280.  Tested on Ubuntu 15.04 and this resolved the issue for me.
Comment 17 Andy Getzendanner 2015-09-17 21:11:24 UTC
Friendly ping!

I have provided a diagnosis and a patch and others on this thread have kindly provided quite a bit of testing.  Can some developer please apply the patch or some other fix ASAP?

There may indeed be some underlying problem in the code, but continuing to crash my browser is (evidently) not an effective way to bring competent developer attention to it.  Leaving this bug open to track the issue may not be any more effective, but at least it will be less irritating for those of us affected by the crash.

Thanks =)
Comment 18 Philipp Erhardt 2016-02-23 17:17:25 UTC
I also keep getting the Thunar crashes. Applying the provided patch fixes the issue for me, too.

Please look into it/apply!

Thanks
Comment 19 Emmanuele Bassi (:ebassi) 2016-03-04 14:58:33 UTC
Comment #13 still stands.

Apparently, we're getting into a situation where the owner of the _NET_SYSTEM_TRAY_<id> selection is set to a non zero value, and yet GDK cannot look up the GDK window associated to it in the current display connection.

This means that something is deeply broken, somewhere, and needs debugging. Adding a NULL check, while seemingly "fixing" the proximal cause of the crash, is not enough to fix the actual cause. If the tray does not have an event filter the whole memory accounting goes out of the window, and stuff like event handling will break.

This could also be the result of a bug inside the tray implementation, given that the _NET_SYSTEM_TRAY_<id> selection should be set up by whatever provides the tray where the tray icon is reparanted into.
Comment 20 Uli Schlachter 2016-05-12 08:38:27 UTC
Created attachment 327681 [details] [review]
Another possible fix

Hi,

attached is another possible fix. It's just a diff instead of a comment since it's likely wrong (g_print?), but feel free to build upon this and fix the crash properly.

The crash is a use-after-free: The event filter is added for window NULL, because gdk_window_lookup_for_display() fails when constructing the icon, but it does not fail when destroying the icon and thus gdk_window_remove_filter() is called with a non-NULL GDKWindow*.

Since GTK shouldn't crash if the tray manager does something wrong, I think it's a good idea to apply this patch (thinks will still work if the event filter is added for window NULL, ie for all windows).
Since the tray manager does something wrong, I also added a g_print in this patch blaming the tray manager of doing so. Feel free to replace this with something more appropriate (or leave it out completely).


Now on "the tray manager does something wrong": How the heck is this supposed work?

The manager window is made known to GTK by the code in gtk/gtkplug{,-x11}.c. It registers _gtk_plug_windowing_filter_func on the tray icon's window when the tray icon is realized and this function calls gdk_window_foreign_new_for_display when it receives a ReparentNotify.
However, gtk_tray_icon_update_manager_window (the code querying the _NET_SYSTEM_TRAY_Sn selection owner and trying to add an event filter for it) is run when a new tray icon is constructed and when a ClientMessage is received announcing a new selection owner. Only the second case is interesting right now.

So the manager window is registered with GDK when the tray icon reparented, but the manager window already needs to be known when the tray icon is created? The icon cannot be reparented before it is created, can it?

I thought that this is due to awesome mishandling _XEMBED_INFO (it always behaves as if the XEMBED_MAPPED bit is set; this will need to be fixed), however I cannot see how this causes something to go wrong in GTK.

Is there something about the interaction between gtktrayicon and gtkplug that I am missing? I'd like to really understand what is going on.

I had someone capture an xtrace of what is going on: GTK creates the window and sets up the properties (e.g. _XEMBED_INFO without the XEMBED_MAPPED bit set), then it sets the XEMBED_MAPPED bit even though no real events regarding the window were received yet. and then the reparent notify comes in.

So the event stream looks like: Lots of PropertyNotify, a generated ClientMessage from awesome announcing the supported XEMBED_VERSION and then ReparentNotify (as explained above, the error has to be before this ReparentNotify). So... what is awesome doing wrong?
Comment 21 GNOME Infrastructure Team 2018-05-02 16:29:52 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/542.