Closed Bug 726483 Opened 12 years ago Closed 9 years ago

[Gtk 3.4/3.6] crash from spinning event loop during resize paint

Categories

(Core :: Widget: Gtk, defect)

Product:
Core
Component:
Widget: Gtk
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla45
Tracking Flags:
Tracking Status
firefox42 --- disabled
firefox43 + verified
firefox44 + verified
firefox45 + verified
firefox-esr38 --- disabled
b2g-v2.5 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

(5 keywords, Whiteboard: [b2g-adv-main2.5-])

Crash Data

Attachments

(5 files, 2 obsolete files)

testcase
(deleted), text/html
Details
remove unnecessary DispatchResized() parameters
(deleted), patch
roc
: review+
Details | Diff | Splinter Review
keep an extra reference to the window
(deleted), patch
roc
: review+
Details | Diff | Splinter Review
avoid DispatchResized() during size-allocate v1.1
(deleted), patch
lizzard
: approval-mozilla-aurora+
abillings
: sec-approval+
Details | Diff | Splinter Review
for 43: avoid DispatchResized() during size-allocate
(deleted), patch
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file testcase (deleted) — 
testcase and crash reported in bug 626963 comment 58:

1. load testcase
2. resize window
3. close window (with the window manager close button)

During this gdk_window_process_updates call from gtk_window_move_resize (i.e. when we paint), the GdkWindow must not be destroyed.

http://git.gnome.org/browse/gtk+/tree/gtk/gtkwindow.c?id=b448ae79634501582633b6a8ad71e75fbdf38526#n7026

With GTK+2, the GdkWindow* is nulled out and this is a safe crash.
With GTK+3, the GdkWindow* pointer will not be nulled and so will point at released memory.

Bug 685457 comment 18 identifies a similar gtk_window_move_resize issue and possible solution number 2 there could fix this.
Can you provide a stack trace (see https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report)?
Severity: normal → critical
Keywords: testcase
I can reproduce.
http://hg.mozilla.org/mozilla-central/rev/9253f058824a
Mozilla/5.0 (X11; Linux i686; rv:13.0a1) Gecko/20120212 Firefox/13.0a1 ID:20120212031149

bp-43cfcc1d-475a-4316-941f-0e30e2120213
Crash Signature: [@ IA__gdk_window_configure_finished ]
Version: Other Branch → Trunk
Unable to reproduce with Trunk & Gtk3 - Mozilla/5.0 (X11; Linux x86_64; rv:26.0) Gecko/20100101 Firefox/26.0
I'm receiving the same sort of crash, by resizing a frame that contains a flash element.
My report is here: http://crash-stats.mozilla.com/report/index/c1a7d508-12c3-4f63-a9e8-061962130826
A video demonstrating: http://www.youtube.com/watch?v=W-uVDPnWv5k&feature=youtu.be (info in description)

A 'PoC' canbe found here: www.nes-wiki.org/docs/poc.html (click play and let it play for a second).

If you resize it faster than perhaps a few pixels every second, Flash will crash. If you go veeerryyy slow, it wont crash.
Blocks: 1034064
No longer blocks: 1034064
can anybody still reproduce this?
I can reproduce the crash with STR of Comment#0 on ubuntu14.04 Nightly41.0a1

bp-087b486d-82ce-4bb9-86b7-2dfaf2150609
Keywords: reproducible
I should have been more specific (my fault)

 Can anybody with a gtk3 build reproduce this.
(In reply to Karl Tomlinson (ni?:karlt) from comment #0)
> During this gdk_window_process_updates call from gtk_window_move_resize
> (i.e. when we paint), the GdkWindow must not be destroyed.

Are you saying that nsWindow::OnExposeEvent is run after nsWindow::Destroy()? If that's the issue I assume we can detect that somehow in OnExposeEvent and cancel what we are about to do?
Flags: needinfo?(karlt)
IIRC this issue was about calling JS from OnExposeEvent(), which could allow
nsWindow::Destroy() to be called.

I wonder whether we still call JS from expose events, and if so, whether that is
still necessary.

However, the current crash seems to be from Destroy() during DispatchResized():

==26578==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0001c6f00 at pc 0x7f2b73972322 bp 0x7ffe92f1e470 sp 0x7ffe92f1e468
READ of size 8 at 0x60c0001c6f00 thread T0
    #0 0x7f2b73972321 in nsWindow::DispatchResized(int, int) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsWindow.cpp:518
    #1 0x7f2b7397f02c in nsWindow::OnSizeAllocate(_GdkRectangle*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsWindow.cpp:2416
    #2 0x7f2b7398774b in size_allocate_cb(_GtkWidget*, _GdkRectangle*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsWindow.cpp:5374
    #3 0x7f2b6b8d252e in g_closure_invoke (/usr/lib64/libgobject-2.0.so.0+0x1052e)
    #4 0x7f2b6b8e4153 in signal_emit_unlocked_R (/usr/lib64/libgobject-2.0.so.0+0x22153)
    #5 0x7f2b6b8ebd14 in g_signal_emit_valist (/usr/lib64/libgobject-2.0.so.0+0x29d14)
    #6 0x7f2b6b8ebfbc in g_signal_emit (/usr/lib64/libgobject-2.0.so.0+0x29fbc)
    #7 0x7f2b6b1a603c in gtk_widget_size_allocate /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwidget.c:4115
    #8 0x7f2b6b1afe4b in gtk_window_size_allocate /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwindow.c:4994
    #9 0x7f2b6b8d536d in g_cclosure_marshal_VOID__BOXEDv (/usr/lib64/libgobject-2.0.so.0+0x1336d)
    #10 0x7f2b6b8d2754 in _g_closure_invoke_va (/usr/lib64/libgobject-2.0.so.0+0x10754)
    #11 0x7f2b6b8eb353 in g_signal_emit_valist (/usr/lib64/libgobject-2.0.so.0+0x29353)
    #12 0x7f2b6b8ebfbc in g_signal_emit (/usr/lib64/libgobject-2.0.so.0+0x29fbc)
    #13 0x7f2b6b1a603c in gtk_widget_size_allocate /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwidget.c:4115
    #14 0x7f2b6b1b1126 in gtk_window_move_resize /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwindow.c:6244
    #15 0x7f2b6b1b1126 in gtk_window_check_resize /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwindow.c:5408
    #16 0x7f2b6b8d2754 in _g_closure_invoke_va (/usr/lib64/libgobject-2.0.so.0+0x10754)
    #17 0x7f2b6b8eb353 in g_signal_emit_valist (/usr/lib64/libgobject-2.0.so.0+0x29353)
    #18 0x7f2b6b8ebfbc in g_signal_emit (/usr/lib64/libgobject-2.0.so.0+0x29fbc)
    #19 0x7f2b6b00d467 in gtk_container_idle_sizer /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkcontainer.c:1357
    #20 0x7f2b6a512568 in gdk_threads_dispatch /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gdk/gdk.c:534
    #21 0x7f2b6b5d2613 in g_main_context_dispatch (/usr/lib64/libglib-2.0.so.0+0x4a613)
    #22 0x7f2b6b5d2937 in g_main_context_iterate (/usr/lib64/libglib-2.0.so.0+0x4a937)
    #23 0x7f2b6b5d29de in g_main_context_iteration (/usr/lib64/libglib-2.0.so.0+0x4a9de)
    #24 0x7f2b7399f27e in nsAppShell::ProcessNextNativeEvent(bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsAppShell.cpp:172
    #25 0x7f2b7392a111 in nsBaseAppShell::DoProcessNextNativeEvent(bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:138
    #26 0x7f2b7392a91d in nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:271
    #27 0x7f2b7392ad0f in non-virtual thunk to nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/widget/Unified_cpp_widget1.cpp:303
    #28 0x7f2b6f25def3 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/threads/nsThread.cpp:846
    #29 0x7f2b6f2dcc5e in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/glue/nsThreadUtils.cpp:277
    #30 0x7f2b6fa4f6cd in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:95
    #31 0x7f2b6f9c3b21 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #32 0x7f2b6f9c39c8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #33 0x7f2b7392a1e6 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:156
    #34 0x7f2b74ea3745 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/components/startup/nsAppStartup.cpp:281
    #35 0x7f2b74f94176 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4292
    #36 0x7f2b74f9542c in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4389
    #37 0x7f2b74f95f72 in XRE_main /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4478
    #38 0x48b4c0 in do_main(int, char**, nsIFile*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/browser/app/nsBrowserApp.cpp:212
    #39 0x48ab41 in main /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/browser/app/nsBrowserApp.cpp:399
    #40 0x7f2b8275ab9e in __libc_start_main (/lib64/libc.so.6+0x21b9e)
    #41 0x48a82c in _start (/mnt/sda10/karl/nightlies/2015-09-09/firefox/firefox+0x48a82c)

0x60c0001c6f00 is located 0 bytes inside of 120-byte region [0x60c0001c6f00,0x60c0001c6f78)
freed by thread T0 here:
    #0 0x472c41 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f2b7403f7b9 in nsFrame::DestroyFrom(nsIFrame*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/generic/nsFrame.cpp:701
    #2 0x7f2b73ee3986 in nsFrameManager::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsFrameManager.cpp:147
    #3 0x7f2b73f27949 in PresShell::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsPresShell.cpp:1276
    #4 0x7f2b73ed6905 in nsDocumentViewer::DestroyPresShell() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsDocumentViewer.cpp:4435
    #5 0x7f2b73ecc39f in nsDocumentViewer::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsDocumentViewer.cpp:1667
    #6 0x7f2b748741dd in nsDocShell::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/docshell/base/nsDocShell.cpp:5613
    #7 0x7f2b7489c8af in non-virtual thunk to nsDocShell::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/docshell/base/Unified_cpp_docshell_base0.cpp:5658
    #8 0x7f2b7498ea56 in nsXULWindow::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpfe/appshell/nsXULWindow.cpp:482
    #9 0x7f2b74974edc in nsWebShellWindow::Destroy() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpfe/appshell/nsWebShellWindow.cpp:779
    #10 0x7f2b74989da6 in nsWebShellWindow::RequestWindowClose(nsIWidget*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpfe/appshell/nsWebShellWindow.cpp:316
    #11 0x7f2b74989f1f in non-virtual thunk to nsWebShellWindow::RequestWindowClose(nsIWidget*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/xpfe/appshell/Unified_cpp_xpfe_appshell0.cpp:318
    #12 0x7f2b73987447 in delete_event_cb(_GtkWidget*, _GdkEventAny*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsWindow.cpp:5384
    #13 0x7f2b6b08a2f4 in _gtk_marshal_BOOLEAN__BOXED /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28-abi_x86_64.amd64/gtk/gtkmarshalers.c:86
    #14 0x7f2b6b8d252e in g_closure_invoke (/usr/lib64/libgobject-2.0.so.0+0x1052e)
    #15 0x7f2b6b8e3ebd in signal_emit_unlocked_R (/usr/lib64/libgobject-2.0.so.0+0x21ebd)
    #16 0x7f2b6b8eb9d7 in g_signal_emit_valist (/usr/lib64/libgobject-2.0.so.0+0x299d7)
    #17 0x7f2b6b8ebfbc in g_signal_emit (/usr/lib64/libgobject-2.0.so.0+0x29fbc)
    #18 0x7f2b6b1a1e9b in gtk_widget_event_internal /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkwidget.c:5010
    #19 0x7f2b6b08900a in gtk_main_do_event /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gtk/gtkmain.c:1598
    #20 0x7f2b6a54f7e3 in gdk_event_dispatch /mnt/sda11/portage/x11-libs/gtk+-2.24.28-r1/work/gtk+-2.24.28/gdk/x11/gdkevents-x11.c:2425
    #21 0x7f2b6b5d2613 in g_main_context_dispatch (/usr/lib64/libglib-2.0.so.0+0x4a613)
    #22 0x7f2b6b5d2937 in g_main_context_iterate (/usr/lib64/libglib-2.0.so.0+0x4a937)
    #23 0x7f2b6b5d29de in g_main_context_iteration (/usr/lib64/libglib-2.0.so.0+0x4a9de)
    #24 0x7f2b7399f27e in nsAppShell::ProcessNextNativeEvent(bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsAppShell.cpp:172
    #25 0x7f2b7392a111 in nsBaseAppShell::DoProcessNextNativeEvent(bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:138
    #26 0x7f2b7392a91d in nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:271
    #27 0x7f2b7392ad0f in non-virtual thunk to nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/widget/Unified_cpp_widget1.cpp:303
    #28 0x7f2b6f25def3 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/threads/nsThread.cpp:846
    #29 0x7f2b6f2877d4 in NS_InvokeByIndex /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:174

previously allocated by thread T0 here:
    #0 0x472e41 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x48ce8d in moz_xmalloc /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/memory/mozalloc/mozalloc.cpp:83
    #2 0x7f2b738e272e in operator new(unsigned long) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/view/../dist/include/mozilla/mozalloc.h:186
    #3 0x7f2b738e272e in nsView::operator new(unsigned long) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/view/nsView.h:60
    #4 0x7f2b738e2592 in nsViewManager::CreateView(nsRect const&, nsView*, nsViewVisibility) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/view/nsViewManager.cpp:130
    #5 0x7f2b73ed0fa1 in nsDocumentViewer::MakeWindow(nsSize const&, nsView*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsDocumentViewer.cpp:2398
    #6 0x7f2b73ecd73f in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsDocumentViewer.cpp:828
    #7 0x7f2b73ecd287 in nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsDocumentViewer.cpp:619
    #8 0x7f2b748a496a in nsDocShell::SetupNewViewer(nsIContentViewer*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/docshell/base/nsDocShell.cpp:9088
    #9 0x7f2b748a3a02 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/docshell/base/nsDocShell.cpp:6989
    #10 0x7f2b74868be8 in nsDocShell::CreateContentViewer(nsACString_internal const&, nsIRequest*, nsIStreamListener**) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/docshell/base/nsDocShell.cpp:8896
    #11 0x7f2b748670f8 in nsDSURIContentListener::DoContent(nsACString_internal const&, bool, nsIRequest*, nsIStreamListener**, bool*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/docshell/base/nsDSURIContentListener.cpp:129
    #12 0x7f2b7067dbc7 in nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/uriloader/base/nsURILoader.cpp:721
    #13 0x7f2b7067be59 in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/uriloader/base/nsURILoader.cpp:398
    #14 0x7f2b7067b33c in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/uriloader/base/nsURILoader.cpp:259
    #15 0x7f2b703a891b in nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/modules/libjar/nsJARChannel.cpp:1292
    #16 0x7f2b6f3f387f in nsInputStreamPump::OnStateStart() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/netwerk/base/nsInputStreamPump.cpp:527
    #17 0x7f2b6f3f308a in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/netwerk/base/nsInputStreamPump.cpp:429
    #18 0x7f2b6f222372 in nsInputStreamReadyEvent::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/io/nsStreamUtils.cpp:91
    #19 0x7f2b6f25e11c in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/threads/nsThread.cpp:874
    #20 0x7f2b6f2dcc5e in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/glue/nsThreadUtils.cpp:277
    #21 0x7f2b6fa4f6cd in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:95
    #22 0x7f2b6f9c3b21 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #23 0x7f2b6f9c39c8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #24 0x7f2b7392a1e6 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:156
    #25 0x7f2b74ea3745 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/components/startup/nsAppStartup.cpp:281
    #26 0x7f2b74f94176 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4292
    #27 0x7f2b74f9542c in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4389
    #28 0x7f2b74f95f72 in XRE_main /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsAppRunner.cpp:4478
    #29 0x48b4c0 in do_main(int, char**, nsIFile*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/browser/app/nsBrowserApp.cpp:212
    #30 0x48ab41 in main /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/browser/app/nsBrowserApp.cpp:399

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/gtk/nsWindow.cpp:518 nsWindow::DispatchResized(int, int)
Shadow bytes around the buggy address:
  0x0c1880030d90: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1880030da0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c1880030db0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1880030dc0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c1880030dd0: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
=>0x0c1880030de0:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c1880030df0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1880030e00: 00 00 00 00 00 00 07 fa fa fa fa fa fa fa fa fa
  0x0c1880030e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1880030e20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1880030e30: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
Group: core-security
Flags: needinfo?(karlt)
Group: core-security → layout-core-security
Comment 9 was with e10s off.
I have verified that this is related to recursive calls to gdk_window_process_updates() inside gtk_window_move_resize(), wherein the mShell widget inside nsWindow is destroyed because of a delete event. This only occurs in GTK2, whereas GTK3 does not use this recursive event processing behavior at all, so avoids the issue. Thus, for now, I'm removing the GTK3 blocker status.

A workaround for this behavior in our code on GTK2 seems non-trivial, and the best fix may ironically be to just use the GTK3 backend instead.
No longer blocks: gtk3, ship-gtk3
Depends on: 1206564
Bug 1206564 covers comment 9.

The UaF of comment 0 is only an issue prior to this change for GTK+ 3.8.

https://git.gnome.org/browse/gtk+/commit/gtk/gtkwindow.c?id=645b5f398d2d50c361600f4f8a089488a66eed99

That still leaves GTK+ version 3.4 and 3.6, which includes Ubuntu precise EOL 2017-04.
Blocks: gtk3, ship-gtk3
(In reply to Karl Tomlinson (ni?:karlt) from comment #12)
> Bug 1206564 covers comment 9.
> 
> The UaF of comment 0 is only an issue prior to this change for GTK+ 3.8.
> 
> https://git.gnome.org/browse/gtk+/commit/gtk/gtkwindow.
> c?id=645b5f398d2d50c361600f4f8a089488a66eed99
> 
> That still leaves GTK+ version 3.4 and 3.6, which includes Ubuntu precise
> EOL 2017-04.

That functionality was removed over 2 years ago from GTK3. Consider carefully, do you really want to *block* GTK3 shipping to the user because of this? I wouldn't.
Does Ubuntu a component update? For instance even RHEL7 updates its Gtk3 toolkit stack.
Summary: crash from spinning event loop during resize paint → [Gtk 3.4/3.6] crash from spinning event loop during resize paint
[Tracking Requested - why for this release]:
As discussed in bug 1207310, we probably want to disable GTK3 in 43 if this bug is not fixed.
status-firefox42: --- → disabled
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox45: --- → affected
tracking-firefox43: --- → ?
tracking-firefox44: --- → ?
Tracking for 43 and 44. We could still take a patch for this for 43.
tracking-firefox43: ? → +
tracking-firefox44: ? → +
tracking-firefox45: --- → +
1. Load testcase.
2. Resize window.  (Do not dismiss alert.)
3. Open second window.
4. In second window, load data:text/html,<script>alert("hello")</script>
5. Close first window.
6. Click "OK" on alert in second window.

Program received signal SIGSEGV, Segmentation fault.
0x00007f26a002ab54 in gdk_window_configure_finished (window=0x7f262b55c920)
    at gdkwindow.c:11029

(gdb) li
11024	 * Since: 2.6
11025	 **/
11026	void
11027	gdk_window_configure_finished (GdkWindow *window)
11028	{
11029	  GDK_WINDOW_IMPL_GET_CLASS (window->impl)->configure_finished (window);
11030	}
11031	
11032	/**
11033	 * gdk_window_set_opacity:
(gdb) p window->impl
$2 = (GdkWindowImpl *) 0xe5e5e5e5e5e5e5e5
(gdb) bt
#0  0x00007f26a002ab54 in gdk_window_configure_finished (
    window=0x7f262b55c920) at gdkwindow.c:11029
#1  0x00007f26a053172f in gtk_window_move_resize (window=0x7f262c03dfc0)
    at gtkwindow.c:7271
#2  gtk_window_check_resize (container=0x7f262c03dfc0) at gtkwindow.c:6313
#3  0x00007f269cdc0755 in _g_closure_invoke_va ()
   from /usr/lib64/libgobject-2.0.so.0
#4  0x00007f269cdd9354 in g_signal_emit_valist ()
   from /usr/lib64/libgobject-2.0.so.0
#5  0x00007f269cdd9fbd in g_signal_emit () from /usr/lib64/libgobject-2.0.so.0
#6  0x00007f26a03688f3 in gtk_container_idle_sizer (data=<optimized out>)
    at gtkcontainer.c:1686
#7  0x00007f26a000efa8 in gdk_threads_dispatch (data=0x7f25fb56aba0)
    at gdk.c:788
#8  0x00007f269cac0614 in g_main_context_dispatch ()
   from /usr/lib64/libglib-2.0.so.0
#9  0x00007f269cac0938 in g_main_context_iterate ()
   from /usr/lib64/libglib-2.0.so.0
#10 0x00007f269cac09df in g_main_context_iteration ()
   from /usr/lib64/libglib-2.0.so.0
#11 0x00007f269355a8f3 in nsAppShell::ProcessNextNativeEvent (
    this=0x7f266cd35470, mayWait=false)
    at /mnt/ssd1/karl/moz/dev/widget/gtk/nsAppShell.cpp:176
#12 0x00007f2693512743 in nsBaseAppShell::DoProcessNextNativeEvent (
    this=0x7f266cd35470, mayWait=false)
    at /mnt/ssd1/karl/moz/dev/widget/nsBaseAppShell.cpp:138
#13 0x00007f2693512acd in nsBaseAppShell::OnProcessNextEvent (
    this=0x7f266cd35470, thr=0x7f26a0ca3600, mayWait=false)
    at /mnt/ssd1/karl/moz/dev/widget/nsBaseAppShell.cpp:271
#14 0x00007f269022ca6a in nsThread::ProcessNextEvent (this=0x7f26a0ca3600, 
    aMayWait=true, aResult=0x7ffc3a6deb3f)
    at /mnt/ssd1/karl/moz/dev/xpcom/threads/nsThread.cpp:933
#15 0x00007f26902943ab in NS_ProcessNextEvent (aThread=0x7f26a0ca3600, 
    aMayWait=true) at /mnt/ssd1/karl/moz/dev/xpcom/glue/nsThreadUtils.cpp:297
#16 0x00007f269022c501 in nsThread::Shutdown (this=0x7f2619561800)
    at /mnt/ssd1/karl/moz/dev/xpcom/threads/nsThread.cpp:774
Assignee: nobody → karlt

        Attachment #8685320 -
        Flags: review?(roc)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        avoid DispatchResized() during size-allocate (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  
gtk_window_show() [1], gtk_window_realize() [2], and  
gtk_window_move_resize [3][4] do not expect the event loop to run during
gtk_widget_size_allocate(), even in modern versions of GTK+.

[1] https://git.gnome.org/browse/gtk+/tree/gtk/gtkwindow.c?id=b0a6af3783a8dd50781ca921de81d3879aafbb00#n6113
[2] https://git.gnome.org/browse/gtk+/tree/gtk/gtkwindow.c?id=b0a6af3783a8dd50781ca921de81d3879aafbb00#n7224
[3] https://git.gnome.org/browse/gtk+/tree/gtk/gtkwindow.c?id=b0a6af3783a8dd50781ca921de81d3879aafbb00#n9675
[4] https://bugzilla.mozilla.org/show_bug.cgi?id=685457#c18

Comment 9 seems to indicate that nsWindow::Destroy() is possible from within
DispatchResized().  I didn't work out how that happened, but
WindowResized() triggers
mDocument->FlushPendingNotifications(Flush_ContentAndNotify) so I guess
anything could happen under some conditions.

#15 0x00007f269396607f in PresShell::ResizeReflowIgnoreOverride (
    this=0x7f265aab1400, aWidth=47880, aHeight=50460)
    at /mnt/ssd1/karl/moz/dev/layout/base/nsPresShell.cpp:1872
#16 0x00007f2693965d53 in PresShell::ResizeReflow (this=0x7f265aab1400, 
    aWidth=47880, aHeight=50460)
    at /mnt/ssd1/karl/moz/dev/layout/base/nsPresShell.cpp:1813
#17 0x00007f26934df61e in nsViewManager::DoSetWindowDimensions (
    this=0x7f2619610640, aWidth=47880, aHeight=50460)
    at /mnt/ssd1/karl/moz/dev/view/nsViewManager.cpp:192
#18 0x00007f26934df7b3 in nsViewManager::SetWindowDimensions (
    this=0x7f2619610640, aWidth=47880, aHeight=50460)
    at /mnt/ssd1/karl/moz/dev/view/nsViewManager.cpp:213
#19 0x00007f269392df33 in nsDocumentViewer::SetBounds (this=0x7f262bcbc840, 
    aBounds=...)
    at /mnt/ssd1/karl/moz/dev/layout/base/nsDocumentViewer.cpp:1959
#20 0x00007f2693f133a6 in nsDocShell::SetPositionAndSize (
    this=0x7f262b437000, aX=0, aY=0, aWidth=798, aHeight=841, aFRepaint=false)
    at /mnt/ssd1/karl/moz/dev/docshell/base/nsDocShell.cpp:5772
#21 0x00007f2693fca75a in nsWebShellWindow::WindowResized (
    this=0x7f262bcbbd90, aWidget=0x7f262b455400, aWidth=798, aHeight=841)
    at /mnt/ssd1/karl/moz/dev/xpfe/appshell/nsWebShellWindow.cpp:282

For GTK+ versions 3.8 to 3.12, this also fixes some possible use in
now-removed [5][6] GTK code, after free if DispatchResized() can run the event
loop.  The Gecko events are delayed, possibly until OnExposeEvent, at which
time gdk_window_process_updates() holds a death grip on the GdkWindow.

Without [6] in pre-3.14 versions, [4] is still possible for override-redirect
windows.

[5] https://git.gnome.org/browse/gtk+/commit/gtk/gtkwindow.c?id=35f618e71dccc02a2226b9f8115d29ec878caf62
and process_updates for override-redirect windows in
[6] https://git.gnome.org/browse/gtk+/commit/gtk/gtkwindow.c?id=46e7f7f4567ef56102703f725d993cd151cec070

This patch does not address the pre-GTK+-3.8 bug of comment 18.

        Attachment #8685321 -
        Flags: review?(roc)

    
    

    
  


  
This addresses the use in code now-removed (comment 12) from
gtk_window_move_resize(), after free in WillPaintWindow(), originally reported in comment 0 and comment 18, for GTK+ versions 3.4 and 3.6.

    
  

        Attachment #8685322 -
        Flags: review?(roc)

    
    

    
  
Comment on attachment 8685321 [details] [diff] [review]
avoid DispatchResized() during size-allocate

Review of attachment 8685321 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gtk/nsWindow.h
@@ +378,5 @@
>  
>      // Should we send resize events on all resizes?
>      bool                mListenForResizes;
> +    // Does WindowResized need to be called on listeners?
> +    bool                mNeedsResizedDispatch;

mNeedsDispatchResized

You need to initialize this in the constructor.

        Attachment #8685321 -
        Flags: review?(roc) → review+

    
    

    
  


  
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #22)
> mNeedsDispatchResized
> 
> You need to initialize this in the constructor.

Done, thanks.

        Attachment #8685321 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 8685719 [details] [diff] [review]
avoid DispatchResized() during size-allocate v1.1

[Security approval request comment]
Please consider this an approval request for all three patches.
Note that this report was public until comment 9.

> How easily could an exploit be constructed based on the patch?

Some understanding of Gecko's event model is required to construct an exploit.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The patches are not to the precise piece of code in interest, but it would not be too difficult to work out.  The challenge is probably more in constructing the exploit.

> Which older supported branches are affected by this flaw?

Branches using the GTK 3 port. 43 and more recent.

> If not all supported branches, which bug introduced the flaw?

Bug 627699 and bug 1186229.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

I'll write a two-line additional patch to make these changes build on 43, which does not have https://hg.mozilla.org/mozilla-central/rev/9400b5b4f6da#l1.17

> How likely is this patch to cause regressions; how much testing does it need?

There is a small risk from change in resize event timing after window resizes and creation.  Regressions would be limited to the GTK port.

The risk is small relative to the risks associated with switching from GTK2 to GTK3.

        Attachment #8685719 -
        Flags: sec-approval?

    
    

    
  


  
For folding into attachment 8685719 [details] [diff] [review] to make a 43-only patch that builds on 43, which does not have
https://hg.mozilla.org/mozilla-central/rev/9400b5b4f6da#l1.17
nor leaky Dispatch()
https://hg.mozilla.org/mozilla-central/rev/83024a3a9586

        Attachment #8685732 -
        Flags: review?(roc)

    
    

    
  
Comment on attachment 8685719 [details] [diff] [review]
avoid DispatchResized() during size-allocate v1.1

sec-approval+. We'll want this on Aurora, Beta, and maybe ESR38 though.

        Attachment #8685719 -
        Flags: sec-approval? → sec-approval+

    
    

    
  
I guess not ESR38. I'm not sure why all these comments are marked private though.

    
  

        Attachment #8685322 -
        Attachment is private: false

    
  

        Attachment #8685719 -
        Attachment is private: false

    
  

        Attachment #8685732 -
        Attachment is private: false

    
  

        Attachment #8685321 -
        Attachment is private: false

    
  

          
            Comment 22 is private:
          false

          
            Comment 23 is private:
          false

          
            Comment 25 is private:
          false

    
  
Status: NEW → RESOLVED
Closed: 9 years ago

          status-firefox45:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

    
    

    
  


  
This patch is for use instead of transplanting ac6f3f8116d7 to 43.

Approval Request Comment
[Feature/regressing bug #]: GTK3 (bug 1186229)
[User impact if declined]: sec-high.
[Describe test coverage new/current, TreeHerder]:
Comment 18 describes a manual test case.
[Risks and why]: 
There is a small risk from change in resize event timing after window resizes and creation.  Regressions would be limited to the GTK port.
The risk is small relative to the general risks associated with switching from GTK2 to GTK3.  Shipping the GTK3 port would be introducing a security flaw that was public for years.
[String/UUID change made/needed]: None.

        Attachment #8685732 -
        Attachment is obsolete: true

        Attachment #8687029 -
        Flags: approval-mozilla-beta?

    
    

    
  
Comment on attachment 8685719 [details] [diff] [review]
avoid DispatchResized() during size-allocate v1.1

Please consider these approval requests as for all 3 patches, 2 of which are common to aurora and beta.

        Attachment #8685719 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8685719 [details] [diff] [review]
avoid DispatchResized() during size-allocate v1.1

Please uplift to aurora.  (but not beta)

        Attachment #8685719 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
  
Comment on attachment 8687029 [details] [diff] [review]
for 43: avoid DispatchResized() during size-allocate

Please uplift to beta, part of ongoing gtk3 work aimed at 43 release.

        Attachment #8687029 -
        Flags: approval-mozilla-beta? → approval-mozilla-beta+
Group: layout-core-security → core-security-release
Flags: qe-verify+

    
    

    
  
I could not reproduce the initial crash following the steps from comment 0 on an old Nightly normal or asan 41.0a1 (2015-06-08) using Ubuntu 14.04 64-bit and I can't reproduce using latest Nightly 45.0a1 asan build as well so I can't properly say this is Verified.
Alice can you please check on Firefox 43 beta 4 and other versions (44, 45) since you actually reproduced the crash on 41.0a1?
Flags: needinfo?(alice0775)

    
  
Blocks: 1225520

    
    

    
  
I can reproduce the crash on the bad build(Nightly 2012-Feb-12) of comment#2 on ubuntu12.04LTS.
And also I can reproduce the crash on Beta43.0b4, Auror44.0a2 and Nightly45.0a1.

bp-7a00b537-adde-4d94-a7ad-5f4262151117  : Beta43.0b4
bp-cd49549e-7cc7-4436-b8da-1e4fa2151117  : Aurora44.0a2 with disabled e10s
bp-7806d909-9049-4f16-bfa0-c05aa2151117  : Nightly45.0a1 with disabled e10s


I will filed a new Bug 1225520.
Flags: needinfo?(alice0775)

    
    

    
  
Thanks Alice! Dropping qe-verify since I could not reproduce it.
Flags: qe-verify+

    
    

    
  
I expect it is possible to test this with a chrome test using STR in comment 18 with step 6 replaced with closing the second window.
Flags: in-testsuite?

          
            Comment 18 is private:
          false

          
            Comment 20 is private:
          false

          
            Comment 21 is private:
          false

          
            Comment 24 is private:
          false

          
            Comment 26 is private:
          false

          
            Comment 27 is private:
          false

    
    

    
  
Bogdan, would you be able to verify this with Nightly and Aurora, on Ubuntu 12.04, using STR in comment 18, please?

43 Beta 6 should not crash, but 43 Beta 7 will crash (safely) because it runs against GTK2.
Flags: qe-verify+
Flags: needinfo?(bogdan.maris)

    
    

    
  
(In reply to Karl Tomlinson (ni?:karlt) from comment #42)
> Bogdan, would you be able to verify this with Nightly and Aurora, on Ubuntu
> 12.04, using STR in comment 18, please?
> 
> 43 Beta 6 should not crash, but 43 Beta 7 will crash (safely) because it
> runs against GTK2.

Verified that I'm not receiving the output from comment 18 in terminal using asan builds latest Nightly 45.0a1 and latest Developer Edtions 44.0a2.
Status: RESOLVED → VERIFIED

          status-firefox44:
          fixed → verified

          status-firefox45:
          fixed → verified
Flags: qe-verify+
Flags: needinfo?(bogdan.maris)

    
    

    
  
Also verified with Firefox 43 beta 6.

          status-firefox43:
          fixed → verified
Whiteboard: [b2g-adv-main2.5-]
Group: core-security-release

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: