1671467 - Crash in [@ mozilla::widget::WindowSurfaceWayland::CommitImageCacheToWaylandBuffer]

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Sean Feng [:sefeng211]]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/3f53e8a5-a624-4cff-909f-0b9af0201014

Reason: SIGSEGV /0x00000080

Top 10 frames of crashing thread:

0 libxul.so mozilla::widget::WindowSurfaceWayland::CommitImageCacheToWaylandBuffer widget/gtk/WindowSurfaceWayland.cpp:882
1 libxul.so mozilla::widget::WindowSurfaceWayland::CommitWaylandBuffer widget/gtk/WindowSurfaceWayland.cpp:918
2 libxul.so RunnableFunction<void  ipc/chromium/src/base/task.h:324
3 libxul.so {virtual override thunk} 
4 libxul.so nsTimerEvent::Run xpcom/threads/TimerThread.cpp:251
5 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1197
6 libxul.so mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:332
7 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:309
8 libxul.so nsThread::ThreadFunc xpcom/threads/nsThread.cpp:442
9 libnspr4.so _pt_root nsprpub/pr/src/pthreads/ptthread.c:201

We do get a fair amount of crashes from this signature. Crash reason is SIGSEGV /0x00000080

Comment 1

[Matt Fagnani]

Nightly 84.0a1 (2020-10-21) on Wayland with WebRender enabled segmentation faulted in mozilla::widget::WindowSurfaceWayland::CommitImageCacheToWaylandBuffer in the Compositor thread when closing Firefox twice in Plasma 5.19.5 in Fedora 33.
https://crash-stats.mozilla.org/report/index/dc2ddf22-6523-4e47-8cfe-df9ab0201021
https://crash-stats.mozilla.org/report/index/6845f2f5-9d26-458c-93ed-857210201021

The crashes might've been due to null pointer dereferences where mDelayedImageCommits was null at WindowSurfaceWayland.cpp:882
if (!mDelayedImageCommits.Length()) {

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Got this without fission too.

mDelayedImageCommits can't be null. It is an AutoTArray. So is the object where it lives null or dead?

https://searchfox.org/mozilla-central/rev/7b40f0b246ad0b54975b1525811f2ad599b95f33/widget/gtk/WindowSurfaceWayland.cpp#982-985 looks
scary. What keeps 'this' alive?

Comment 3

[Martin Stránský [:stransky] (ni? me)]

(In reply to Olli Pettay [:smaug] from comment #2)

Got this without fission too.

mDelayedImageCommits can't be null. It is an AutoTArray. So is the object where it lives null or dead?

https://searchfox.org/mozilla-central/rev/7b40f0b246ad0b54975b1525811f2ad599b95f33/widget/gtk/WindowSurfaceWayland.cpp#982-985 looks
scary. What keeps 'this' alive?

This does not need to be alive. The WaylandBufferDelayCommitHandler() looks for this in active objects before it's used, see DelayedCommitsCheckAndRemoveSurface(). Also a lock is used here to avoid potential threading issues.

The backtrace confuses me as WindowSurfaceWayland.cpp is supposed to be used with Basic/SW compositor only - it's widet SW rendering. Is that wr-sw scenario? Or do we draw some windows with basic compositor when WR is on? (popups maybe?).

Anyway, thanks for looking into it. Is there any mozilla test which can reveal such crash? I can run mochitest locally on Wayland backend and check if I get the crash.

Comment 4

[Andrew McCreight [:mccr8]]

(I hid this bug because I think smaug wanted it closed.)

Comment 5

[Daniel Veditz [:dveditz]]

I checked 7 of the 17 crashes with this signature from this past week. In 6 rax = 0xe5e5e5e5e5e5e5e5 which is a clear sign of a UAF. In the 7th rax = 0xe501000000000000 which is suspicious (UAF poison value that got partially stomped?).

Comment 6

[Martin Stránský [:stransky] (ni? me)]

I'll look at it.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

This should be fixed by Bug 1648698 and Bug 1685055.