Closed Bug 1640009 Opened 8 months ago Closed 8 months ago

Crash in [@ PLDHashTable::Iterator::Iterator | mozilla::gfx::VRManager::ShutdownVRManagerParents]

Categories

(Core :: WebVR, defect)

Unspecified
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 --- unaffected
firefox78 + fixed

People

(Reporter: mccr8, Assigned: jya)

References

(Regression)

Details

(4 keywords, Whiteboard: [post-critsmash-triage])

Crash Data

Attachments

(1 file)

This bug is for crash report bp-d0f57bfe-a996-4a3b-a944-9cb530200521.

Top 10 frames of crashing thread:

0 xul.dll PLDHashTable::Iterator::Iterator xpcom/ds/PLDHashTable.cpp:713
1 xul.dll mozilla::gfx::VRManager::ShutdownVRManagerParents gfx/vr/VRManager.cpp:1134
2 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/gfx/vr/ipc/VRManagerParent.cpp:131:7'>::Run xpcom/threads/nsThreadUtils.h:574
3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1211
4 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:302
5 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
7 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:444
8 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:399
9 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:139

The URLs look pretty generic and not specifically WebVR-related (Facebook, Twitch, YouTube, Reddit, etc.) It looks like this first showed up in the 20200519160549 build.

[Tracking Requested - why for this release]: #4 top crash for the May 20th Windows Nightly, and a regression.

Here's the regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=96c90df47bdab0cebeed56e31dbaf2d9ed5b97f6&tochange=8f68705097b4bf88cd61b43b14401cde98ac75b6

Not much there, but bug 1637890 involves changing something with shutdown, and this crash is happening during shutdown, so maybe that's to blame.

Regressed by: 1637890

Set release status flags based on info from the regressing bug 1637890

Jean-Yves, can you take a look?

Group: gfx-core-security
Flags: needinfo?(jyavenard)
Keywords: csectype-uaf

The VRManager code is racy.
First access to the CompositorThread is done via the global CompositorThread() this would be access on the main thread, and cleared on the main thread.

VRManager keeps a reference to the CompositorThreadHolder() but never use that to access it.

Then we have VRManager::MaybeGet() being used from the CompositorThread by VRManagerParent::ShutdownInternal

VRManager::MaybeGet() returns sVRManagerSingleton ; but this is cleared on the mainthread by the shutdown manager.

VRManager::Shutdown() gets called on the CompositorThread when the compositor thread gets shutdown, and it dispatches a task to clear the VRManager

At the same time, we have the VRManager::sVRManagerSingleton being called on the main thread that too will shutdown the VRManager.

The shutdown tasks since bug 1637890 are run in LIFO order which could have changed the timing of events.

What appears here however is that we have two tasks being dispatched on two different thread to shutdown the VRManager, if the main thread ones win, it may delete sVRManagerSingleton

To access sVRManagerSingleton the code does the following on the compositor thread:

void VRManagerParent::ShutdownInternal() {
VRManager* vm = VRManager::MaybeGet();
if (!vm) {
return;
}
vm->ShutdownVRManagerParents();
}

If the main thread task run first; vm can become a dangling pointer

Flags: needinfo?(jyavenard)
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED

Thanks for the fix, jya. I'm going to mark this as sec-moderate because it is happening late in shutdown.

Keywords: sec-moderate
Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Crash Signature: [@ PLDHashTable::Iterator::Iterator | mozilla::gfx::VRManager::ShutdownVRManagerParents] → [@ PLDHashTable::Iterator::Iterator | mozilla::gfx::VRManager::ShutdownVRManagerParents] [@ <name omitted> | mozilla::gfx::VRManager::ShutdownVRManagerParents ]
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.