Closed Bug 1819410 Opened 2 years ago Closed 2 years ago

Crash in [@ mozilla::ipc::MessageChannel::~MessageChannel | mozilla::ipc::IToplevelProtocol::~IToplevelProtocol]

Categories

(Core :: Security: Process Sandboxing, defect, P3)

ARM64
Windows 10
defect

Tracking

()

RESOLVED FIXED
112 Branch
Tracking Status
firefox-esr102 112+ fixed
firefox110 --- unaffected
firefox111 --- fixed
firefox112 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

Crash report: https://crash-stats.mozilla.org/report/index/9f91dc58-0163-4f2c-b9f5-f75ff0230228

MOZ_CRASH Reason: MOZ_CRASH(MessageChannel destroyed without being closed (mChannelState == ChannelConnected).)

Top 10 frames of crashing thread:

0  xul.dll  mozilla::ipc::MessageChannel::~MessageChannel  ipc/glue/MessageChannel.cpp:510
1  xul.dll  mozilla::ipc::IToplevelProtocol::~IToplevelProtocol  ipc/glue/ProtocolUtils.h:393
1  xul.dll  mozilla::PRemoteSandboxBrokerChild::~PRemoteSandboxBrokerChild  ipc/ipdl/PRemoteSandboxBrokerParent.cpp:43
2  xul.dll  mozilla::RemoteSandboxBrokerParent::~RemoteSandboxBrokerParent  security/sandbox/win/src/remotesandboxbroker/RemoteSandboxBrokerParent.h:16
2  xul.dll  mozilla::RemoteSandboxBroker::~RemoteSandboxBroker  security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp:21
3  xul.dll  mozilla::RemoteSandboxBroker::~RemoteSandboxBroker  security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp:17
4  xul.dll  mozilla::AbstractSandboxBroker::Release  security/sandbox/win/src/sandboxbroker/sandboxBroker.h:30
4  xul.dll  mozilla::RefPtrTraits<mozilla::AbstractSandboxBroker>::Release  mfbt/RefPtr.h:50
4  xul.dll  RefPtr<mozilla::AbstractSandboxBroker>::ConstRemovingRefPtrTraits<mozilla::AbstractSandboxBroker>::Release  mfbt/RefPtr.h:381
4  xul.dll  RefPtr<mozilla::AbstractSandboxBroker>::~RefPtr  mfbt/RefPtr.h:81

I was not able to reduce the regression window further:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f45ac8766b61&tochange=e027953e2470621b104e21045bec0a568c2e93d7
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d561ece0b43b&tochange=f45ac8766b6167ecd4adc81490f8f9b927472200

But something subtly made us crash the GMP plugin process later than before.

Originally we would fail and hit this path:
https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp#97

We would not have successfully setup the IPDL channel and so there was nothing to teardown.

Now we fail to send the initial IPC messages at startup because the process crashes just after:
https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp#106
https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp#114

Since the higher levels assume that the failure to launch the process means nothing is setup / it doesn't have to teardown our sandbox broker IPDL objects, we should do it here before returning.

STR: On Windows ARM with a clean profile and DRM enabled, go to https://shaka-player-demo.appspot.com/demo/. The parent process will crash.

If the process being launched by the sandbox broker crashes after we
setup the IPDL channel, but before we successfully complete the
initialization process, we don't explicitly teardown
RemoteSandboxBrokerParent. As such, when we destroy the owning objects,
it attempts to destroy RemoteSandboxBrokerParent while the protocol is
still open, triggering a release assert for destroying
PRemoteSandboxBrokerParent while the protocol/channel are still open.

This patch makes us explicitly teardown the channel if there is a launch
failure inside the sandbox broker.

Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8fc7ac6810cc Ensure we shutdown the RemoteSandboxBrokerParent IPDL instance during launch failures. r=jld
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch

Comment on attachment 9320333 [details]
Bug 1819410 - Ensure we shutdown the RemoteSandboxBrokerParent IPDL instance during launch failures.

Beta/Release Uplift Approval Request

  • User impact if declined: Any streaming site depending on Widevine and Clearkey plugins are broken with the x86 plugins on Windows ARM. Uplifting the requested set will allow us to roll out a switch to the ARM plugins instead.

Visiting any DRM protected streaming site will crash the plugin process, and the user will be unable to play content.

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1814910, Bug 1814703, Bug 1819661, Bug 1814703, Bug 1820669
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk because we have verified this in 112 in both nightly and beta with no issues reported by QA. GMP plugins are already completely broken for Windows ARM users.
  • String changes made/needed:
  • Is Android affected?: Yes

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Any streaming site depending on Widevine and Clearkey plugins are broken with the x86 plugins on Windows ARM. Uplifting the requested set will allow us to roll out a switch to the ARM plugins instead.
  • User impact if declined: Visiting any DRM protected streaming site will crash the plugin process, and the user will be unable to play content.
  • Fix Landed on Version: 112
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk because we have verified this in 112 in both nightly and beta with no issues reported by QA. GMP plugins are already completely broken for Windows ARM users.

Depends on:
Bug 1814910, Bug 1814703, Bug 1819661, Bug 1814703, Bug 1820669, Bug 1811981

Attachment #9320333 - Flags: approval-mozilla-release?
Attachment #9320333 - Flags: approval-mozilla-esr102?

Comment on attachment 9320333 [details]
Bug 1819410 - Ensure we shutdown the RemoteSandboxBrokerParent IPDL instance during launch failures.

Approved for 102.10esr

Attachment #9320333 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+

Comment on attachment 9320333 [details]
Bug 1819410 - Ensure we shutdown the RemoteSandboxBrokerParent IPDL instance during launch failures.

Approved for 111.0.1

Attachment #9320333 - Flags: approval-mozilla-release? → approval-mozilla-release+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: