1459443 - Crash in static void mozilla::dom::`anonymous namespace'::StructuredCloneCallbacksFreeTransfer

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-d9234c55-44c1-45cc-acfb-9fe360180505.
=============================================================

Seen while reviewing nightly crash data - crashes started using 20180504220115: https://bit.ly/2woBZwf. 84 crashes/27 installations so far, and additional crashes are happening with the 2nd signature listed.

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a2d1d4158bb4718d8bb31e1284e133aa99273600&tochange=5207b1392b11db534550a5eb801302e6dbb58f95

Bug 1456986 is in that range. ni on :bkelly

Top 10 frames of crashing thread:

0  @0x110 
1 xul.dll static void mozilla::dom::`anonymous namespace'::StructuredCloneCallbacksFreeTransfer dom/base/StructuredCloneHolder.cpp:122
2 xul.dll static void DiscardTransferables<js::SystemAllocPolicy> js/src/vm/StructuredClone.cpp:704
3 xul.dll JSAutoStructuredCloneBuffer::clear js/src/vm/StructuredClone.cpp:2832
4 xul.dll mozilla::dom::StructuredCloneHolder::~StructuredCloneHolder dom/base/StructuredCloneHolder.cpp:278
5 xul.dll mozilla::dom::`anonymous namespace'::SendMessageEventRunnable::`scalar deleting destructor' 
6 xul.dll mozilla::dom::WorkerRunnable::Release dom/workers/WorkerRunnable.cpp:212
7 xul.dll nsCOMPtr<nsISupportsCString>::~nsCOMPtr<nsISupportsCString> 
8 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1115
9 xul.dll NS_ProcessPendingEvents xpcom/threads/nsThreadUtils.cpp:461

=============================================================

Comment 1

[Marcia Knous [:marcia]]

Adding Mac signature.

Comment 2

[Calixte Denizet (:calixte)]

In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1458836.
:baku, could you investigate please ?

[1] https://hg.mozilla.org/mozilla-central/rev?node=e73e0231a9e3

Comment 3

[Andrea Marchesini [:baku]]

It seems that holder is null here: https://hg.mozilla.org/mozilla-central/annotate/b1628ac71fcc15797baec6083650bfcde650f190/dom/base/StructuredCloneHolder.cpp#l122

This code runs during the DTOR of a WorkerRunnable. We can clean up data when ::Cancel() is executed, but I wonder why a similar setup in MessageEventRunnable works fine and here we have this issue.

Comment 4

[Marcia Knous [:marcia]]

Adding another signature spotted in crash stats: https://bit.ly/2KJfZPC.

Comment 5

[Marcia Knous [:marcia]]

I made this bug security sensitive since several of the crash reports look like potential UAF -> https://crash-stats.mozilla.com/report/index/a0c8b388-6535-43f4-9e8d-640510180505

Comment 6

[Marcia Knous [:marcia]]

Adding two Linux signatures spotted in crash stats.

Comment 7

[Marcia Knous [:marcia]]

Adding 62 as affected. It appears as if most of these signatures (or a high %) are startup crashes.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

I think this is most likely due to bug 1456986.  That code was using a sandbox to unpack and re-pack structured clone data.  At shutdown its possible that the sandbox global is destroyed before the structured clone data.  I don't think I was using the right cloning values for this to be safe.

For now I backed out bug 1456986 on inbound and beta.  I will try to fix the cloning values and reland on trunk only.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Calling this fixed by backout. We can verify it once the backout reaches nightlies.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

The backout has merged to m-c.  Looks like the first build with the backout is 20180508231737.  Lets see if this persists in that build or later.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

No crashes on Nightlies with the backout included. Won't ship on Beta until 61.0b4 on Friday, but looks like it's going to work.

Comment 12

[Stuart Philp :sphilp]

I can reliably reproduce this on mac with today's nightly just by loading linkedin.com. Can you repro Ben?

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Please see bug 1461667.  The fix should be in the next nightly build.  It just missed this morning's build.