Closed Bug 1555322 Opened 5 years ago Closed 5 years ago

Crash in [@ mozilla::dom::WebSocketImpl::Dispatch]

Categories

(Core :: Networking: WebSockets, defect, P2)

68 Branch
x86_64
Windows 7
defect

Tracking

()

RESOLVED WORKSFORME
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 - wontfix
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 + wontfix
firefox69 + wontfix
firefox70 + wontfix

People

(Reporter: philipp, Assigned: kershaw)

Details

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

Crash Data

Attachments

(2 files, 2 obsolete files)

This bug is for crash report bp-998b6b16-fea2-4949-8de6-1a41b0190529.

Top 10 frames of crashing thread:

0 xul.dll nsresult mozilla::dom::WebSocketImpl::Dispatch dom/websocket/WebSocket.cpp:2620
1 xul.dll nsresult mozilla::dom::WebSocketImpl::CloseConnection dom/websocket/WebSocket.cpp:435
2 xul.dll mozilla::dom::WebSocket::Close dom/websocket/WebSocket.cpp:2370
3 xul.dll static bool mozilla::dom::WebSocket_Binding::close dom/bindings/WebSocketBinding.cpp:503
4 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3165
5 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:535
6 xul.dll static bool InternalCall js/src/vm/Interpreter.cpp:590
7 xul.dll static bool Interpret js/src/vm/Interpreter.cpp:3082
8 xul.dll js::RunScript js/src/vm/Interpreter.cpp:423
9 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:563

content crashes with this signature are piking up in volume in the 68.0b cycle, they are accounting for 0.9% of all tab crashes at the moment in beta 4.

all the reports so far are coming from 64bit firefox builds from windows 7 with involvement of modules from www.proxifier.com hooking into the browser:

(100.0% in signature vs 00.60% overall) Module "PrxerDrv.dll" = true
(100.0% in signature vs 00.61% overall) Module "PrxerNsp.dll" = true

another striking correlation is that most of the crashes seem to take place in a virtual machine:

(89.19% in signature vs 00.98% overall) adapter_vendor_id = 0x15ad (VMware)

Michal, could you take a look at this?

Flags: needinfo?(michal.novotny)

There are few reports that crash on 0xe5e5e5e5 (e.g. https://crash-stats.mozilla.org/report/index/e6a73be4-8a63-4a83-a0c5-f77e90190531), so this might be UAF crash. I would guess that mImpl at https://hg.mozilla.org/releases/mozilla-release/annotate/2a7896ba9aa5d982abe01f859d771d411fda8101/dom/websocket/WebSocket.cpp#l2351 points to a freed memory. But I'm not familiar with life cycle of WebSocketImpl in the DOM part. Baku, can you please have a look?

Flags: needinfo?(michal.novotny) → needinfo?(amarchesini)
Group: dom-core-security, network-core-security
Keywords: csectype-uaf

Hsin-Yi, do you know, from your team, who can debug this crash?

Flags: needinfo?(amarchesini) → needinfo?(htsai)
Flags: needinfo?(htsai)

Oops, didn't mean to clear NI. I'm taking this to an email discussion.

Assignee: nobody → kershaw
Priority: -- → P2

I think the root cause of the crash is that WebSocketImpl is not deleted on the target thread. When this happens, there is a race between set WebSocket::mImpl to nullptr and access mImpl.
If WebSocketImpl is always deleted on the target thread, WebSocketImpl::Disconnect should be called in ~WebSocketImpl when mDisconnectingOrDisconnected is false.
So, this patch checks the ref count in WebSocketImpl::Release and make sure to delete WebSocketImpl on the right thread.

Keywords: checkin-needed

This bug affects Firefox outside Nightly/mozilla-central. That's the reason it needs a security rating to proceed. See https://wiki.mozilla.org/Security/Bug_Approval_Process for more details.

Flags: needinfo?(kershaw)
Keywords: checkin-needed

Comment on attachment 9070919 [details]
Bug 1555322 - Make sure WebSocketImpl is deleted on the right thread

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easy. We don't know how to let WebSocketImpl be deleted on the wrong thread.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: Not sure.
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: I think it's not easy to reproduce the crash.
    Given that the crash rate is not high, it's not very risky.
  • How likely is this patch to cause regressions; how much testing does it need?: The risk is low. This patch just makes sure WebSocketImpl is deleted on the right thread.
Flags: needinfo?(kershaw)
Attachment #9070919 - Flags: sec-approval?

sec-approval+ for trunk. Can I get a beta patch nominated as well?

Attachment #9070919 - Flags: sec-approval? → sec-approval+
Keywords: checkin-needed

(In reply to Al Billings [:abillings] from comment #8)

sec-approval+ for trunk. Can I get a beta patch nominated as well?

Thanks.
This patch should be able to apply to beta cleanly.

https://hg.mozilla.org/integration/autoland/rev/865186d0ab940e5f89e5d88cf0214b8df5a41085

(In reply to Kershaw Chang [:kershaw] from comment #9)

This patch should be able to apply to beta cleanly.

Please nominate like described at https://wiki.mozilla.org/Release_Management/Uplift_rules

Flags: needinfo?(kershaw)
Keywords: checkin-needed

(In reply to Julien Cristau [:jcristau] from comment #11)

Backed out for leaks on asan and debug:
https://hg.mozilla.org/integration/autoland/rev/0f2eb9968616fc2b5da5cc6a3a389460d63ac8dd

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=865186d0ab940e5f89e5d88cf0214b8df5a41085&selectedJob=252518350

So, the problem is that WebSocketImpl::Disconnect not always dispatch runnables successfully. If the dispatching fails, we should delete WebSocketImpl anyway.

Flags: needinfo?(kershaw)
Keywords: checkin-needed

https://hg.mozilla.org/mozilla-central/rev/3bd61ba63144

Please nominate this for Beta approval. It grafts cleanly as-is.

Group: dom-core-security, network-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(kershaw)
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9070919 [details]
Bug 1555322 - Make sure WebSocketImpl is deleted on the right thread

Beta/Release Uplift Approval Request

  • User impact if declined: Firefox could crash, but the crash rate is not really high.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): What this patch does is just dispatching the last release to the correct thread.
    The worst case is having some leaks if we fail to dispatch the release.
  • String changes made/needed: no
Flags: needinfo?(kershaw)
Attachment #9070919 - Flags: approval-mozilla-beta?

Comment on attachment 9070919 [details]
Bug 1555322 - Make sure WebSocketImpl is deleted on the right thread

Fixes a new WebSocket crash in 68 with sec implications. Approved for 68rc1.

Attachment #9070919 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]

We still have this crash with this patch, so this means that the problem is not about UAF of WebSocket::mImpl.
I take a look at the crash report again and I found something I can't understand.

The top 3 frames are:
0 xul.dll nsresult mozilla::dom::WebSocketImpl::Dispatch dom/websocket/WebSocket.cpp:2646
1 xul.dll nsresult mozilla::dom::WebSocketImpl::CloseConnection dom/websocket/WebSocket.cpp:435
2 xul.dll mozilla::dom::WebSocket::Close dom/websocket/WebSocket.cpp:2370

At frame 2, WebSocket::Close is called on worker thread, so I think this WebSocket object should be created on worker thread and WebSocketImpl::mIsMainThread should be false. Then, We have a crash at WebSocketImpl::Dispatch where we try to dispatch a runnable to main thread.
This should only happen when mIsMainThread is true.
So, it looks like somehow we get a main thread created WebSocket object on worker thread and call WebSocket::Close on it. I can't understand how this could happen or maybe I understand this code so wrong.

Currently, I have no idea what to do now. baku, do you have any idea?

Flags: needinfo?(amarchesini)

Moving the NI. Move the NI back to me if needed and I can take a look. Thanks!

Flags: needinfo?(amarchesini) → needinfo?(htsai)

Hey :baku, sorry that I move NI back to you as several of my team members are on leave this or next week. If you have time and can shed some light here working with Kershaw earlier, that'd be great. Otherwise I'll keep monitor this, and try best to find someone working with Kershaw in a week or two.

Flags: needinfo?(htsai) → needinfo?(amarchesini)

(In reply to Kershaw Chang [:kershaw] from comment #18)

We still have this crash with this patch, so this means that the problem is not about UAF of WebSocket::mImpl.
How you make that interpretation?
Why mImpl couldn't be a deleted object there. We just call through some methods and crash when trying to use something from the deleted object, no?

https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/dom/websocket/WebSocket.cpp#227,230 is suspicious.
Why can we call Disconnect() in dtor when https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/dom/websocket/WebSocket.cpp#574,604 happens. (though, that might not have anything to do with this bug)

Wondering, is is possible to call Disconnect() on a wrong thread?

Another thing, observer handling. Is it possible that while we're Disconnecting on worker, WebSocketImpl::Observe gets called on the main thread? (WebSocketImpl is just a weakptr in observer service.)
I think it is, and that is racy.
(Still doesn't explain the stack in this bug.)

Thanks for the tips. I'll take a closer look and try to make some patches.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

How you make that interpretation?
Why mImpl couldn't be a deleted object there. We just call through some methods and crash when trying to use something from the deleted object, no?

Because I thought this crash might have something to do with accessing a main thread created WebSocket from worker thread. Apparently, I was wrong. Given the recent crash report and the crash address being 0xe5e5e5e5, this should be UAF.

https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/dom/websocket/WebSocket.cpp#227,230 is suspicious.
Why can we call Disconnect() in dtor when https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/dom/websocket/WebSocket.cpp#574,604 happens. (though, that might not have anything to do with this bug)

I think this should be ok, since DisconnectInternalRunnable is a sync runnable.

Wondering, is is possible to call Disconnect() on a wrong thread?

I think calling Disconnect() on a wrong thread might be the root cause of this crash.
Based on the previous patch, WebSocketImpl should be released on the right thread when mDisconnectingOrDisconnected is false. If Disconnect() is called on a wrong thread and mDisconnectingOrDisconnected becomes true, WebSocketImpl will still be released on the wrong thread.

Another thing, observer handling. Is it possible that while we're Disconnecting on worker, WebSocketImpl::Observe gets called on the main thread? (WebSocketImpl is just a weakptr in observer service.)
I think it is, and that is racy.

I think this should be also ok, since AddObserver is only called when mIsMainThread is true and there is no worker involved.

(Still doesn't explain the stack in this bug.)

Since the memory of WebSocketImpl is released, accessing WebSocketImpl::mIsMainThread is undefined behavior. I think this might be the explanation to the stack.

Attachment #9082614 - Attachment is obsolete: true

Does it make sense to uplift this diagnostic patch to beta? This might help us to catch the root cause earlier.

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #28)

Please get sec-approval for this patch. See https://wiki.mozilla.org/Security/Bug_Approval_Process

I think there is no need to ask for sec-approval, since this is a diagnostic patch. I just add some assertions.
:abillings, what do you think?

Flags: needinfo?(kershaw) → needinfo?(abillings)

(In reply to Kershaw Chang [:kershaw] from comment #29)

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #28)

Please get sec-approval for this patch. See https://wiki.mozilla.org/Security/Bug_Approval_Process

I think there is no need to ask for sec-approval, since this is a diagnostic patch. I just add some assertions.
:abillings, what do you think?

On a second thought, I think this might need a sec-approval.

Flags: needinfo?(abillings)

Comment on attachment 9082679 [details]
Bug 1555322 - Revert previous change and add MOZ_DIAGNOSTIC_ASSERT

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easy. I think it's difficult to let WebSocketImpl be released on a wrong thread. That's why I'd like to land this diagnostic patch.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: Not sure.
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: The risk is low, since this is a diagnostic patch.
  • How likely is this patch to cause regressions; how much testing does it need?: It's unlikely to cause regressions.
Attachment #9082679 - Flags: sec-approval?

Comment on attachment 9082679 [details]
Bug 1555322 - Revert previous change and add MOZ_DIAGNOSTIC_ASSERT

sec-approval+ for mozilla-central. Since this is a diagnostic patch, I assume it doesn't need to be backported.

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

Since the first patch didn't fix the issues, shouldn't we change Firefox69 and Firefox68 status flags back to "affected"?

Flags: needinfo?(kershaw)

(In reply to Al Billings [:abillings] from comment #33)

Since the first patch didn't fix the issues, shouldn't we change Firefox69 and Firefox68 status flags back to "affected"?

Sure. Thanks for the reminder.

Flags: needinfo?(kershaw)

Backed out for failing an assertion on WebSocket.cpp:

https://hg.mozilla.org/integration/autoland/rev/04546db61ecb32ca005541ec7175e01bc0503eec

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=259994563&resultStatus=retry%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception&revision=80c02bc9f35d1bc130c6f4cbad56d5c4c7447100
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=259994563&repo=autoland

[task 2019-08-05T20:35:11.754Z] 20:35:11 INFO - TEST-START | dom/websocket/tests/test_webSocket_sharedWorker.html
[task 2019-08-05T20:35:11.791Z] 20:35:11 INFO - GECKO(1712) | ++DOMWINDOW == 31 (0x11b408c00) [pid = 1714] [serial = 57] [outer = 0x12292f3e0]
[task 2019-08-05T20:35:11.838Z] 20:35:11 INFO - GECKO(1712) | [Child 1714, Main Thread] WARNING: Workers don't support the 'mem.mem.' preference!: file /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp, line 544
[task 2019-08-05T20:35:11.838Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x1229b9800 == 8 [pid = 1714] [id = {1bfb7f38-338d-624e-9446-080a4266e885}] [url = about:srcdoc]
[task 2019-08-05T20:35:11.840Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x11b4a4000 == 7 [pid = 1714] [id = {57ea8790-5f49-2944-8bc3-ae75441a2383}] [url = https://example.com/tests/dom/websocket/tests/iframe_webSocket_sandbox.html]
[task 2019-08-05T20:35:11.840Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x11b4aa800 == 6 [pid = 1714] [id = {c4497b53-1183-6344-b158-38fe210cb9ee}] [url = about:srcdoc]
[task 2019-08-05T20:35:11.841Z] 20:35:11 INFO - GECKO(1712) | --DOMWINDOW == 30 (0x11b414c00) [pid = 1714] [serial = 37] [outer = 0x0] [url = about:blank]
[task 2019-08-05T20:35:11.841Z] 20:35:11 INFO - GECKO(1712) | --DOMWINDOW == 29 (0x10c2a1c00) [pid = 1714] [serial = 31] [outer = 0x0] [url = http://mochi.test:8888/tests/SimpleTest/iframe-between-tests.html]
[task 2019-08-05T20:35:11.841Z] 20:35:11 INFO - GECKO(1712) | --DOMWINDOW == 28 (0x11b40c800) [pid = 1714] [serial = 34] [outer = 0x0] [url = about:blank]
[task 2019-08-05T20:35:11.841Z] 20:35:11 INFO - GECKO(1712) | --DOMWINDOW == 27 (0x122cac800) [pid = 1714] [serial = 23] [outer = 0x0] [url = http://mochi.test:8888/tests/dom/websocket/tests/test_event_listener_leaks.html]
[task 2019-08-05T20:35:11.841Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x11b4a5000 == 5 [pid = 1714] [id = {f354661d-2d9f-a042-8940-96fa026901da}] [url = about:srcdoc]
[task 2019-08-05T20:35:11.846Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x10ac41800 == 4 [pid = 1714] [id = {c47597fb-d48c-3c4b-ba14-46bfa65ef9e4}] [url = https://example.com/tests/dom/websocket/tests/iframe_webSocket_sandbox.html?popup]
[task 2019-08-05T20:35:11.846Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x11b4b0000 == 3 [pid = 1714] [id = {2fd873d6-51f4-0549-aa9b-4f8a1159ab4a}] [url = about:blank]
[task 2019-08-05T20:35:11.873Z] 20:35:11 INFO - GECKO(1712) | --DOCSHELL 0x117fd7800 == 4 [pid = 1713] [id = {500e2c44-85da-7249-8381-023c0c2570b0}] [url = moz-extension://1c593e66-63ce-1b46-ae4b-2e2af4960805/_generated_background_page.html]
[task 2019-08-05T20:35:11.897Z] 20:35:11 INFO - GECKO(1712) | MEMORY STAT | vsize 7309MB | residentFast 126MB | heapAllocated 19MB
[task 2019-08-05T20:35:11.904Z] 20:35:11 INFO - GECKO(1712) | Assertion failure: NS_IsMainThread() == mIsMainThread, at /builds/worker/workspace/build/src/dom/websocket/WebSocket.cpp:228
[task 2019-08-05T20:35:32.400Z] 20:35:32 INFO - GECKO(1712) | #01: <name omitted> [dom/websocket/WebSocket.cpp:237]
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO - GECKO(1712) | #02: void detail::ProxyRelease<nsISupports>(char const*, nsIEventTarget*, already_AddRefed<nsISupports>, bool) [xpcom/threads/nsProxyRelease.h:0]
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO - GECKO(1712) | #03: <name omitted> [xpcom/threads/nsProxyRelease.cpp:15]
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO - GECKO(1712) | #04: mozilla::net::BaseWebSocketChannel::ListenerAndContextContainer::~ListenerAndContextContainer() [netwerk/protocol/websocket/BaseWebSocketChannel.cpp:357]
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO - GECKO(1712) | #05: mozilla::net::BaseWebSocketChannel::ListenerAndContextContainer::Release() [netwerk/protocol/websocket/BaseWebSocketChannel.h:80]
[task 2019-08-05T20:35:32.401Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO - GECKO(1712) | #06: mozilla::net::BaseWebSocketChannel::~BaseWebSocketChannel() [netwerk/protocol/websocket/BaseWebSocketChannel.h:24]
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO - GECKO(1712) | #07: <name omitted> [netwerk/protocol/websocket/WebSocketChannelChild.cpp:64]
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO - GECKO(1712) | #08: mozilla::net::NeckoChild::DeallocPWebSocketChild(mozilla::net::PWebSocketChild*) [netwerk/ipc/NeckoChild.cpp:191]
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO - GECKO(1712) | #09: mozilla::ipc::ActorLifecycleProxy::Release() [ipc/glue/ProtocolUtils.h:926]
[task 2019-08-05T20:35:32.402Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO - GECKO(1712) | #10: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) [s3:gecko-generated-sources:12bd08af6fb015839a51a7a3119282f2a1b73f5d072ea3a332b788e34f656deba3ab1a1ce1c83023d9cde2fa70ecf29677ebf6c644bd10068575b5d48d80061e/ipc/ipdl/PContentChild.cpp::7714]
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO - GECKO(1712) | #11: mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) [ipc/glue/MessageChannel.cpp:2185]
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO - GECKO(1712) | #12: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) [ipc/glue/MessageChannel.cpp:2108]
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO - GECKO(1712) | #13: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) [ipc/glue/MessageChannel.cpp:0]
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO - GECKO(1712) | #14: mozilla::ipc::MessageChannel::MessageTask::Run() [ipc/glue/MessageChannel.cpp:1988]
[task 2019-08-05T20:35:32.403Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO - GECKO(1712) | #15: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1213]
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO - GECKO(1712) | #16: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:486]
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO - GECKO(1712) | #17: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:86]
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO - GECKO(1712) | #18: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.404Z] 20:35:32 INFO - GECKO(1712) | #19: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:139]
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO - GECKO(1712) | #20: nsAppShell::Run() [widget/cocoa/nsAppShell.mm:705]
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO - GECKO(1712) | #21: XRE_RunAppShell() [toolkit/xre/nsEmbedFunctions.cpp:919]
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO - GECKO(1712) | #22: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:238]
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO - GECKO(1712) | #23: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:291]
[task 2019-08-05T20:35:32.405Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.406Z] 20:35:32 INFO - GECKO(1712) | #24: XRE_InitChildProcess(int, char**, XREChildData const*) [toolkit/xre/nsEmbedFunctions.cpp:758]
[task 2019-08-05T20:35:32.406Z] 20:35:32 INFO -
[task 2019-08-05T20:35:32.406Z] 20:35:32 INFO - GECKO(1712) | #25: main [ipc/app/MozillaRuntimeMain.cpp:23]
[task 2019-08-05T20:35:32.406Z] 20:35:32 INFO -

Flags: needinfo?(kershaw)

Hi Kershaw, have you been able to take any recent looks at this bug?

Flags: needinfo?(kershaw)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #40)

Hi Kershaw, have you been able to take any recent looks at this bug?

I've just checked if there is a crash report caused by the MOZ_DIAGNOSTIC_ASSERT I added, but I can't find one.
Maybe we have to wait for more days.

Flags: needinfo?(kershaw)

Too late for 69 at this point with RC coming next week.

Kershaw, is there more debugging info now to fix this for 70?

Flags: needinfo?(amarchesini) → needinfo?(kershaw)

(In reply to Nhi Nguyen (:nhi) from comment #43)

Kershaw, is there more debugging info now to fix this for 70?

I can't find this crash on 70. Let's wait a bit longer. I'll keep my eye on this.

Flags: needinfo?(kershaw)
Flags: needinfo?(kershaw)

(In reply to Liz Henry (:lizzard) from comment #45)

Here's a couple of reports,
https://crash-stats.mozilla.org/report/index/119c5a27-fdb1-4d2d-a82e-cb7a00190911 (beta 5)
https://crash-stats.mozilla.org/report/index/5e0d71cc-4898-46bb-bbc4-0639f0190909 (beta 4)

Hope that helps!

Thanks! I'll take a look.

We still have the same crash, but I can't find the crash report for MOZ_DIAGNOSTIC_ASSERT added in https://phabricator.services.mozilla.com/D40455. I have no idea how to figure out why WebSocketImpl is released on a wrong thread.
Maybe we should try to make mImpl a ref pointer to avoid the uaf.

This is a bit confusing. I think we should be opening followup bugs rather than trying to keep landing patches on the same bug.

Flags: needinfo?(kershaw)

Getting late for a fix in 70. We can still try to land something for 71/72.

I don't even see any of these crashes in Firefox 70: the two September crashes linked in comment 45 are the only ones ever, out of nearly 300 crashes in the last 2 months. I also don't see any crashes on ESR68. Is it fixed/WFM?

(In reply to Liz Henry (:lizzard) from comment #49)

This is a bit confusing. I think we should be opening followup bugs rather than trying to keep landing patches on the same bug.

Let's close this as WFM since this crash seems disappeared.
If this crash happens again, we can try to fix in another bug.

Flags: needinfo?(kershaw)
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → WORKSFORME
Attachment #9097689 - Attachment is obsolete: true
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: