Closed Bug 1540136 Opened 5 years ago Closed 5 years ago

AddressSanitizer: heap-use-after-free [@ mozilla::gmp::ChromiumCDMParent::Shutdown] with READ of size 8

Categories

(Core :: Audio/Video: GMP, defect, P2)

Product:
Core
Component:
Audio/Video: GMP
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox-esr60 67+ fixed
firefox66 --- wontfix
firefox67 + fixed
firefox68 + fixed

People

(Reporter: jkratzer, Assigned: bryce)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [adv-main67+][adv-esr60.7+])

Attachments

(9 files)

harness.html
630 bytes, text/html
Details
part_1.html
333 bytes, text/html
Details
part_2.html
649 bytes, text/html
Details
Log showing unexpected ordering of operations
29.36 KB, text/plain
Details
Bug 1540136 - P1: Log more info in ChromiumCDMProxy dtor and Shutdown. r?cpearce
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
Details | Review
Bug 1540136 - P2: CDMProxy stores main thread as nsISerialEventTarget, rather than nsIEventTarget. r?cpearce
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
Details | Review
Bug 1540136 - P3: ChromiumCDMProxy runs CDM init thennable code on the main thread. r?cpearce
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
Details | Review
Bug 1540136 - P4: Remove ChromiumCDMParent mMainThread as it is unused. r?cpearce
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
Details | Review
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r?cpearce
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
abillings
: sec-approval+
Details | Review

Testcase found while fuzzing mozilla-central rev 401af0fbedf3. I'm currently reducing the testcase and will update once complete.

==23992==ERROR: AddressSanitizer: heap-use-after-free on address 0x6030006be840 at pc 0x7f7ce440f4dc bp 0x7f7cc47b1d90 sp 0x7f7cc47b1d88
READ of size 8 at 0x6030006be840 thread T34 (GMPThread)
#0 0x7f7ce440f4db in mozilla::gmp::ChromiumCDMParent::Shutdown() /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMParent.cpp:1041:19
#1 0x7f7ce4415223 in mozilla::gmp::ChromiumCDMParent::RecvShutdown() /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMParent.cpp:803:3
#2 0x7f7cddab0e65 in mozilla::gmp::PChromiumCDMParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PChromiumCDMParent.cpp:1283:61
#3 0x7f7cdcb535a9 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2151:21
#4 0x7f7cdcb4f2ea in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2078:9
#5 0x7f7cdcb51527 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1937:3
#6 0x7f7cdcb522b7 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1968:13
#7 0x7f7cdb877fd1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
#8 0x7f7cdb8803dd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:482:10
#9 0x7f7cdcb5e382 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
#10 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#11 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#12 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#13 0x7f7cdb8701f3 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:454:11
#14 0x7f7d006905ad in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
#15 0x7f7d002d36da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
#16 0x7f7cff2b188e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

0x6030006be840 is located 0 bytes inside of 24-byte region [0x6030006be840,0x6030006be858)
freed by thread T0 (file:// Content) here:
#0 0x55c8293299e2 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
#1 0x7f7ce441b718 in operator() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:486:5
#2 0x7f7ce441b718 in reset /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:323
#3 0x7f7ce441b718 in ~UniquePtr /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:274
#4 0x7f7ce441b718 in mozilla::ChromiumCDMProxy::~ChromiumCDMProxy() /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:35
#5 0x7f7ce441bae8 in mozilla::ChromiumCDMProxy::~ChromiumCDMProxy() /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:35:39
#6 0x7f7ce444b1c2 in mozilla::ChromiumCDMProxy::Release() /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.h:21:3
#7 0x7f7ce445445a in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:45:40
#8 0x7f7ce445445a in Release /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:362
#9 0x7f7ce445445a in assign_assuming_AddRef /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:64
#10 0x7f7ce445445a in operator= /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:155
#11 0x7f7ce445445a in Revoke /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:747
#12 0x7f7ce445445a in Revoke /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1188
#13 0x7f7ce445445a in ~RunnableMethodImpl /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1149
#14 0x7f7ce445445a in mozilla::detail::RunnableMethodImpl<mozilla::ChromiumCDMProxy*, void (mozilla::ChromiumCDMProxy::)(unsigned int), true, (mozilla::RunnableKind)0, unsigned int>::~RunnableMethodImpl() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1149
#15 0x7f7cdb85c5ab in mozilla::Runnable::Release() /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:50:1
#16 0x7f7cdb8386c1 in assign_assuming_AddRef /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:367:7
#17 0x7f7cdb8386c1 in assign_assuming_AddRef /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:390
#18 0x7f7cdb8386c1 in operator= /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:678
#19 0x7f7cdb8386c1 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:299
#20 0x7f7cdb877fd1 in nsThread::ProcessNextEvent(bool, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
#21 0x7f7cdb8803dd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:482:10
#22 0x7f7cdcb5c9af in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
#23 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#24 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#25 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#26 0x7f7ce5eba2e3 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
#27 0x7f7cea4b594e in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:933:20
#28 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#29 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#30 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#31 0x7f7cea4b4adc in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:771:34
#32 0x55c82935c834 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#33 0x55c82935c834 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:263
#34 0x7f7cff1b1b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

previously allocated by thread T34 (GMPThread) here:
#0 0x55c829329d63 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
#1 0x55c82935e5fd in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:68:15
#2 0x7f7ce446fef4 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
#3 0x7f7ce446fef4 in MakeUnique<mozilla::ChromiumCDMCallbackProxy, const RefPtr<mozilla::ChromiumCDMProxy> &, const nsCOMPtr<nsIEventTarget> &> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:617
#4 0x7f7ce446fef4 in mozilla::ChromiumCDMProxy::Init(unsigned int, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&)::$_11::operator()() const::'lambda'(RefPtr<mozilla::gmp::ChromiumCDMParent>)::operator()(RefPtr<mozilla::gmp::ChromiumCDMParent>) const /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:92
#5 0x7f7ce446edcd in InvokeMethod<(lambda at /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:89:13), void ((lambda at /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:89:13)::)(RefPtr<mozilla::gmp::ChromiumCDMParent>) const, RefPtr<mozilla::gmp::ChromiumCDMParent> > /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:502:12
#6 0x7f7ce446edcd in InvokeCallbackMethod<false, (lambda at /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:89:13), void ((lambda at /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMProxy.cpp:89:13)::)(RefPtr<mozilla::gmp::ChromiumCDMParent>) const, RefPtr<mozilla::gmp::ChromiumCDMParent>, RefPtr<mozilla::MozPromise<RefPtr<mozilla::gmp::ChromiumCDMParent>, mozilla::MediaResult, true>::Private> > /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:534
#7 0x7f7ce446edcd in mozilla::MozPromise<RefPtr<mozilla::gmp::ChromiumCDMParent>, mozilla::MediaResult, true>::ThenValue<mozilla::ChromiumCDMProxy::Init(unsigned int, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&)::$_11::operator()() const::'lambda'(RefPtr<mozilla::gmp::ChromiumCDMParent>), mozilla::ChromiumCDMProxy::Init(unsigned int, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&)::$_11::operator()() const::'lambda'(mozilla::MediaResult)>::DoResolveOrRejectInternal(mozilla::MozPromise<RefPtr<mozilla::gmp::ChromiumCDMParent>, mozilla::MediaResult, true>::ResolveOrRejectValue&) /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:716
#8 0x7f7ce44734ec in mozilla::MozPromise<RefPtr<mozilla::gmp::ChromiumCDMParent>, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:392:21
#9 0x7f7cdb845ae8 in mozilla::EventTargetWrapper::Runner::Run() /builds/worker/workspace/build/src/xpcom/threads/AbstractThread.cpp:113:25
#10 0x7f7cdb877fd1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
#11 0x7f7cdb8803dd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:482:10
#12 0x7f7cdcb5e382 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:303:20
#13 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#14 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#15 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#16 0x7f7cdb8701f3 in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:454:11
#17 0x7f7d006905ad in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
#18 0x7f7d002d36da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)

Thread T34 (GMPThread) created by T0 (file:// Content) here:
#0 0x55c82931267d in pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
#1 0x7f7d00682613 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
#2 0x7f7d0066c09e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
#3 0x7f7cdb873169 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:661:8
#4 0x7f7cdb87f095 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:416:12
#5 0x7f7cdb884144 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:135:57
#6 0x7f7ce4489309 in NS_NewNamedThread<10> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:71:10
#7 0x7f7ce4489309 in mozilla::gmp::GeckoMediaPluginService::GetThread(nsIThread**) /builds/worker/workspace/build/src/dom/media/gmp/GMPService.cpp:317
#8 0x7f7ce4486c3c in mozilla::gmp::GeckoMediaPluginService::Init() /builds/worker/workspace/build/src/dom/media/gmp/GMPService.cpp:217:10
#9 0x7f7ce44d8bdb in mozilla::gmp::GMPServiceCreateHelper::GetOrCreateOnMainThread() /builds/worker/workspace/build/src/dom/media/gmp/GMPService.cpp:108:18
#10 0x7f7ce4484bb6 in mozilla::gmp::GMPServiceCreateHelper::GetOrCreate() /builds/worker/workspace/build/src/dom/media/gmp/GMPService.cpp:76:17
#11 0x7f7cdb7e289b in mozilla::xpcom::CreateInstanceImpl(mozilla::xpcom::ModuleID, nsISupports*, nsID const&, void**) /builds/worker/workspace/build/src/obj-firefox/xpcom/components/StaticComponents.cpp:9209:60
#12 0x7f7cdb815554 in CreateInstance /builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp:220:46
#13 0x7f7cdb815554 in nsComponentManagerImpl::GetServiceLocked((anonymous namespace)::MutexLock&, (anonymous namespace)::EntryWrapper&, nsID const&, void**) /builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp:1401
#14 0x7f7cdb807a47 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) /builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp:1588:10
#15 0x7f7cdb820c75 in CallGetService /builds/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp:61:43
#16 0x7f7cdb820c75 in nsGetServiceByContractID::operator()(nsID const&, void**) const /builds/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp:243
#17 0x7f7cdb624fa4 in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) /builds/worker/workspace/build/src/xpcom/base/nsCOMPtr.cpp:82:7
#18 0x7f7ce44c93f0 in nsCOMPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:607:5
#19 0x7f7ce44c93f0 in mozilla::HaveGMPFor(nsTString<char> const&, nsTArray<nsTString<char> >&&) /builds/worker/workspace/build/src/dom/media/gmp/GMPUtils.cpp:179
#20 0x7f7ce437de63 in mozilla::dom::HavePluginForKeySystem(nsTString<char> const&) /builds/worker/workspace/build/src/dom/media/eme/MediaKeySystemAccess.cpp:94:21
#21 0x7f7ce435e0c8 in EnsureCDMInstalled /builds/worker/workspace/build/src/dom/media/eme/MediaKeySystemAccess.cpp:106:8
#22 0x7f7ce435e0c8 in mozilla::dom::MediaKeySystemAccess::GetKeySystemStatus(nsTSubstring<char16_t> const&, nsTSubstring<char>&) /builds/worker/workspace/build/src/dom/media/eme/MediaKeySystemAccess.cpp:120
#23 0x7f7ce436f04e in mozilla::dom::MediaKeySystemAccessManager::Request(mozilla::dom::DetailedPromise*, nsTSubstring<char16_t> const&, mozilla::dom::Sequence<mozilla::dom::MediaKeySystemConfiguration> const&, mozilla::dom::MediaKeySystemAccessManager::RequestType) /builds/worker/workspace/build/src/dom/media/eme/MediaKeySystemAccessManager.cpp:119:7
#24 0x7f7cdfd37fec in mozilla::dom::Navigator::RequestMediaKeySystemAccess(nsTSubstring<char16_t> const&, mozilla::dom::Sequence<mozilla::dom::MediaKeySystemConfiguration> const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/Navigator.cpp:1783:33
#25 0x7f7ce0b87026 in requestMediaKeySystemAccess /builds/worker/workspace/build/src/obj-firefox/dom/bindings/NavigatorBinding.cpp:1857:45
#26 0x7f7ce0b87026 in mozilla::dom::Navigator_Binding::requestMediaKeySystemAccess_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Navigator*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/NavigatorBinding.cpp:1873
#27 0x7f7ce30124c3 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3150:13
#28 0x7f7cea79f5a7 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
#29 0x7f7cea79f5a7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
#30 0x7f7cea7879ba in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
#31 0x7f7cea7879ba in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3075
#32 0x7f7cea7699e8 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
#33 0x7f7cea79ff16 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:562:13
#34 0x7f7cea7a1b62 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:605:8
#35 0x7f7ceb396e69 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2621:10
#36 0x7f7ce261a149 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
#37 0x7f7ce388c4b2 in HandleEvent<mozilla::dom::EventTarget > /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
#38 0x7f7ce388c4b2 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener, mozilla::dom::Event*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1038
#39 0x7f7ce388eae3 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1239:17
#40 0x7f7ce386eb90 in HandleEvent /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/EventListenerManager.h:355:5
#41 0x7f7ce386eb90 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:351
#42 0x7f7ce386cdb8 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:553:16
#43 0x7f7ce3873a03 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:1048:11
#44 0x7f7ce387b796 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp
#45 0x7f7cdff80b84 in nsINode::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/nsINode.cpp:1025:17
#46 0x7f7cdf88d18c in nsContentUtils::DispatchEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, mozilla::Trusted, bool*, mozilla::ChromeOnlyDispatch) /builds/worker/workspace/build/src/dom/base/nsContentUtils.cpp:4063:28
#47 0x7f7cdf88cefe in nsContentUtils::DispatchTrustedEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, bool*) /builds/worker/workspace/build/src/dom/base/nsContentUtils.cpp:4033:10
#48 0x7f7cdfbd31ca in mozilla::dom::Document::DispatchContentLoadedEvents() /builds/worker/workspace/build/src/dom/base/Document.cpp:4708:3
#49 0x7f7cdfcd8d9b in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1122:12
#50 0x7f7cdfcd8d9b in apply<mozilla::dom::Document, void (mozilla::dom::Document::)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1128
#51 0x7f7cdfcd8d9b in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1174
#52 0x7f7cdb838655 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:295:32
#53 0x7f7cdb877fd1 in nsThread::ProcessNextEvent(bool, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
#54 0x7f7cdb8803dd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:482:10
#55 0x7f7cdcb5c9af in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
#56 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#57 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#58 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#59 0x7f7ce5eba2e3 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
#60 0x7f7cea4b594e in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:933:20
#61 0x7f7cdca32fae in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
#62 0x7f7cdca32fae in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
#63 0x7f7cdca32fae in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
#64 0x7f7cea4b4adc in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:771:34
#65 0x55c82935c834 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#66 0x55c82935c834 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:263
#67 0x7f7cff1b1b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/dom/media/gmp/ChromiumCDMParent.cpp:1041:19 in mozilla::gmp::ChromiumCDMParent::Shutdown()
Shadow bytes around the buggy address:
0x0c06800cfcb0: 00 fa fa fa 00 00 00 00 fa fa 00 00 00 fa fa fa
0x0c06800cfcc0: 00 00 00 fa fa fa 00 00 00 00 fa fa 00 00 00 fa
0x0c06800cfcd0: fa fa 00 00 00 fa fa fa 00 00 00 00 fa fa 00 00
0x0c06800cfce0: 00 fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
0x0c06800cfcf0: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa
=>0x0c06800cfd00: fa fa fd fd fd fa fa fa[fd]fd fd fa fa fa fd fd
0x0c06800cfd10: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
0x0c06800cfd20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c06800cfd30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c06800cfd40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c06800cfd50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==23992==ABORTING

Group: core-security → media-core-security
Priority: -- → P2
Attached file harness.html —

In order to reproduce this issue, a build with --enable-fuzzing is required.
Further, copy all three files (harness.html, part_1.html, part_2.html) into the same directory and browse to harness.html.

Attached file part_1.html —
Attached file part_2.html —

In addition to the use-after-free shown above, the attached testcase also may trigger the following SEGV:

==2869==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7fd7dedd4fe6 bp 0x7ffd5ff6f480 sp 0x7ffd5ff6f340 T0)
==2869==The signal is caused by a WRITE memory access.
==2869==Hint: address points to the zero page.
#0 0x7fd7dedd4fe5 in mozilla::ipc::MessageChannel::Clear() /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:764:9
#1 0x7fd7dedd25d7 in mozilla::ipc::MessageChannel::~MessageChannel() /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:628:3
#2 0x7fd7dee05515 in mozilla::ipc::IToplevelProtocol::ToplevelState::~ToplevelState() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/ProtocolUtils.h:400:9
#3 0x7fd7dee05d68 in mozilla::ipc::IToplevelProtocol::ToplevelState::~ToplevelState() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ipc/ProtocolUtils.h:400:9
#4 0x7fd7dedfc550 in operator() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:486:5
#5 0x7fd7dedfc550 in reset /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:323
#6 0x7fd7dedfc550 in operator= /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:296
#7 0x7fd7dedfc550 in mozilla::ipc::IToplevelProtocol::~IToplevelProtocol() /builds/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp:567
#8 0x7fd7e5d42d88 in mozilla::gmp::GMPContentParent::~GMPContentParent() /builds/worker/workspace/build/src/dom/media/gmp/GMPContentParent.cpp:35:39
#9 0x7fd7dda30e8c in operator() /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.cpp:304:7
#10 0x7fd7dda30e8c in ForEachSlot<(lambda at /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.cpp:302:51)> /builds/worker/workspace/build/src/obj-firefox/dist/include/PLDHashTable.h:359
#11 0x7fd7dda30e8c in ForEachSlot<(lambda at /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.cpp:302:51)> /builds/worker/workspace/build/src/obj-firefox/dist/include/PLDHashTable.h:349
#12 0x7fd7dda30e8c in PLDHashTable::~PLDHashTable() /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.cpp:302
#13 0x7fd7e5da4c93 in ~nsTHashtable /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTHashtable.h:384:43
#14 0x7fd7e5da4c93 in ~GMPServiceChild /builds/worker/workspace/build/src/dom/media/gmp/GMPServiceChild.cpp:430
#15 0x7fd7e5da4c93 in mozilla::gmp::GMPServiceChild::~GMPServiceChild() /builds/worker/workspace/build/src/dom/media/gmp/GMPServiceChild.cpp:430
#16 0x7fd7e5da3086 in operator() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:486:5
#17 0x7fd7e5da3086 in reset /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:323
#18 0x7fd7e5da3086 in operator= /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/UniquePtr.h:296
#19 0x7fd7e5da3086 in mozilla::gmp::GeckoMediaPluginServiceChild::Observe(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/dom/media/gmp/GMPServiceChild.cpp:361
#20 0x7fd7e5da3174 in non-virtual thunk to mozilla::gmp::GeckoMediaPluginServiceChild::Observe(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/dom/media/gmp/GMPServiceChild.cpp
#21 0x7fd7dda55711 in nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/xpcom/ds/nsObserverList.cpp:66:19
#22 0x7fd7dda5b2cc in nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/xpcom/ds/nsObserverService.cpp:294:19
#23 0x7fd7ddc4e513 in mozilla::ShutdownXPCOM(nsIServiceManager*) /builds/worker/workspace/build/src/xpcom/build/XPCOMInit.cpp:653:24
#24 0x7fd7eb9dbc4d in XRE_TermEmbedding() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:222:3
#25 0x7fd7dee023d1 in mozilla::ipc::ScopedXREEmbed::Stop() /builds/worker/workspace/build/src/ipc/glue/ScopedXREEmbed.cpp:90:5
#26 0x7fd7eb9dca2e in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:761:16
#27 0x559dcac9d7bc in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#28 0x559dcac9d7bc in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:263
#29 0x7fd80038bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:764:9 in mozilla::ipc::MessageChannel::Clear()

The SEGV looks like bug 1538194. In both cases it seems we're not doing shutdown correctly. Will investigate.

Assignee: nobody → bvandyk

(In reply to Jason Kratzer [:jkratzer] from comment #1)

Created attachment 9055129 [details]
harness.html

In order to reproduce this issue, a build with --enable-fuzzing is required.
Further, copy all three files (harness.html, part_1.html, part_2.html) into the same directory and browse to harness.html.

Having some trouble reproing the fuzz + asan bustage, doing another build now to try and catch it.

To hit the SEGV I'm having to manually close my window after opening the test files, since the script is not being allowed to (I'm not sure if there's something I can set so the script can?).

(In reply to Jason Kratzer [:jkratzer] from comment #7)

Bryce, can you try using the following prefs?
https://github.com/MozillaSecurity/fuzzdata/blob/ca9f5329841b6a11eab8bb51071daf9257182c4e/settings/firefox/prefs-default-e10s.js

Will do once my new build is done.

NI: cpearce. Anything obvious leaping out at you?

Looks something like bug 1355506 to me.

My working theory is that the ChromiumCDMProxy is being destroyed before the ChromiumCDMParent has Shutdown run on it. This would cause the ChromiumCDMProxy to free the callback, leaving the ChromiumCDMParent with a dangling pointer.

The parent is given the raw pointer from the proxy, and then doesn't modify it aside from nulling it. It will null it when shutting down, which I think works on the assumption that it will always shutdown before the proxy dtors. Based on looking at the code, the above theory is the only way I can see to arrive at the bug.

I believe the stacks in comment 0 also bear out the theory.

The mystery then, is how is the ChromiumCDMProxy being destroyed before the ChromiumCDMParent? Bug 1355506 put steps in place to avoid this. However, here we're seeing the proxy be released because there are no remaining refs to it (2nd stack in comment 0).

Going to look into if MediaKeys is not doing what we expect with its ChromiumCDMProxy.

Flags: needinfo?(cpearce)

Looks like I needed to these prefs to true:

  • fuzzing.enabled
  • dom.allow_scripts_to_close_windows

Took me about 5 tries to repro the ASAN issue (reliably get the assert one), but think I've finally caught it. So I've got a locally working baseline I can start tinkering with.

Updated theory: the cause is a race between ChromiumCDMParent::Init() and the Then code in ChromiumCDMProxy which assigns ChromiumCDMProxy.mCDM, and ChromiumCDMProxy::Shutdown().

If we shutdown the proxy while the parent is in the process of initializing, the ChromiumCDMParent can set mCDMCallback, but the ChromiumCDMProxy may not have set mCDM (because it does so once the parent init is done in the Then). This means that the proxy shutdown doesn't kick off its special handling for if it has a CDM, and thus doesn't shutdown the CDM.

Because the init chain holds a self RefPtr, it's only after it's completed that the ChromiumCDMProxy dtors, at which stage the ChromiumCDMParent thinks it has a good reference, but the proxy has been freed.

With some extra logging I'm pretty sure this is what is happening. I will attach the log following this comment. Things to note in the log:

  • ChromiumCDMProxy::Shutdown happens before the ChromiumCDMParent::Init, this is why the proxy shutdown isn't shutting down the cdm.
  • ChromiumCDMProxy::OnCDMCreated is happening just before ChromiumCDMProxy::~ChromiumCDMProxy, I believe this is bearing out that the init chain is keeping the proxy alive, after which it dies.

Clearing the NI as I think I have a good handle on what's going on here. Figuring out an appropriate patch.

Flags: needinfo?(cpearce)

This gives us greater flexibility in using the main thread member to run
promises.

The site where we obtain the main thread returns a serial event target, so we're
not doing much more work here, we're just keeping the serial event target
interface, rather than converting to an event target interface.

Depends on D26204

This code is calling other code that expects to be on the main thread, and
having this on the main thread (now that the main thread is a serial event
target) makes it easier to reason about this and other main thread code. I.e.
this cannot be running during other main thread code.

Depends on D26205

  • Watch for if a proxy shuts down during init and if so, shutdown the CMD parent
    that is being initialized.
  • Make ChromiumCDMParent only store a pointer to a ChromiumCDMProxy when it has
    successfully initialized. This avoid the lopsided relationship where a if a
    ChromiumCDMParent fails to initialize it may keep a pointer to a proxy, but
    the proxy will never have reference to that CDM parent.

Depends on D26207

Comment on attachment 9055938 [details]
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r?cpearce

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The changes in the patch show that a race exists if we shutdown and init at the same time, but do not make clear the problematic impact of this (UAF). An attacker would need to do further analysis and/or fuzzy to discover the UAF and then craft an exploit from there. I think discovery would not be straight forward. I do not think my knowledge sufficient in the domain to judge the difficulty of exploiting the UAF should an attacker be aware of it.
  • 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?: I believe all supported branches are.
  • If not all supported branches, which bug introduced the flaw?: I believe all branches are.
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: Caveat: I believe these patches should apply to previous branches, but have not manually tried to apply them.
  • How likely is this patch to cause regressions; how much testing does it need?: Low to medium: Patch P5 contains the main behaviour changes, the amount of code changed is ~50 lines (relatively low), but the GMP code is fairly complicated due to being multithreaded. Based on the code changes being alterations to references between GMP components, I would expect the most likely regression to be shutdown hangs. I've done manual testing, and have not found any regressions. I do not think further manual testing is required, and think shutdown hangs will most likely be detected by automated tests if introduced.
Attachment #9055938 - Flags: sec-approval?
Attachment #9055933 - Flags: sec-approval?
Attachment #9055935 - Flags: sec-approval?
Attachment #9055936 - Flags: sec-approval?
Attachment #9055937 - Flags: sec-approval?

You only need to mark sec-approval on one patch. Just write your comments to apply to all of them (as you did).

I wasn't clear from the prompt if I should leave all the boxes for other patches checked or not. I figured I'd err on the side of marking all. To be clear, it would be appropriate to uncheck the sec approval boxes for the other patches and just put it up on P5? Should I do anything further to move this forward, or is the above enough?

Yeah, just clear the flags on the other patches. That won't affect the approval process. You've done everything you need to do for that. abillings goes through the list of bugs with pending sec-approvals once or twice a day, so I'm sure he'll get to it soon.

Comment on attachment 9055938 [details]
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r?cpearce

Sec-approval+ for mozilla-central. We should get patched nominated for Beta and ESR60 as well.

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

Comment on attachment 9055938 [details]
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r?cpearce

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Unclear, this appears to have existed for some time in GMP code.
  • User impact if declined: A race that results in a use after free will remain in the browser. If this were discovered and exploited it would be of significant user impact.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: No other bug IDs, just all patches in this chain.
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patches change a relatively small amount of code, but deal with a complicated part of the code base (GMP). They are in nightly and appear to be working without issue. Based on the code changed, I would say the most likely fallout from the patches would be shutdown hangs, but I haven't seen any yet.
  • String changes made/needed: None. The C++ code has some new log strings (P1, P5), but I do not believe these are the kind of strings this question is concerned with.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Sec-high.
  • User impact if declined: A race that results in a use after free will remain in the browser. If this were discovered and exploited it would be of significant user impact.
  • Fix Landed on Version: 68.
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patches change a relatively small amount of code, but deal with a complicated part of the code base (GMP). They are in nightly and appear to be working without issue. Based on the code changed, I would say the most likely fallout from the patches would be shutdown hangs, but I haven't seen any yet.
  • String or UUID changes made by this patch: None. The C++ code has some new log strings (P1, P5), but I do not believe these are the kind of strings this question is concerned with.
Attachment #9055938 - Flags: approval-mozilla-esr60?
Attachment #9055938 - Flags: approval-mozilla-beta?
Attachment #9055933 - Flags: approval-mozilla-beta?
Attachment #9055935 - Flags: approval-mozilla-beta?
Attachment #9055936 - Flags: approval-mozilla-beta?
Attachment #9055937 - Flags: approval-mozilla-beta?
Attachment #9055933 - Flags: approval-mozilla-esr60?
Attachment #9055935 - Flags: approval-mozilla-esr60?
Attachment #9055936 - Flags: approval-mozilla-esr60?
Attachment #9055937 - Flags: approval-mozilla-esr60?

As with the sec-approval flag, it is okay to just mark one patch for approval and say that it covers the other patches, too. :)

Had the thought when I saw the prompts, but since they were all pre-checked I figured I'd again err on the side caution :)

Comment on attachment 9055938 [details]
Bug 1540136 - P5: Handle if ChromiumCDMProxy is shutdown in the middle of init. r?cpearce

Fix for sec-high issue, verified in nightly.
OK for uplift for 67 beta 12, ESR 60.7.

Attachment #9055938 - Flags: approval-mozilla-esr60?
Attachment #9055938 - Flags: approval-mozilla-esr60+
Attachment #9055938 - Flags: approval-mozilla-beta?
Attachment #9055938 - Flags: approval-mozilla-beta+
Attachment #9055937 - Flags: approval-mozilla-esr60?
Attachment #9055937 - Flags: approval-mozilla-esr60+
Attachment #9055937 - Flags: approval-mozilla-beta?
Attachment #9055937 - Flags: approval-mozilla-beta+
Attachment #9055936 - Flags: approval-mozilla-esr60?
Attachment #9055936 - Flags: approval-mozilla-esr60+
Attachment #9055936 - Flags: approval-mozilla-beta?
Attachment #9055936 - Flags: approval-mozilla-beta+
Attachment #9055935 - Flags: approval-mozilla-esr60?
Attachment #9055935 - Flags: approval-mozilla-esr60+
Attachment #9055935 - Flags: approval-mozilla-beta?
Attachment #9055935 - Flags: approval-mozilla-beta+
Attachment #9055933 - Flags: approval-mozilla-esr60?
Attachment #9055933 - Flags: approval-mozilla-esr60+
Attachment #9055933 - Flags: approval-mozilla-beta?
Attachment #9055933 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [adv-main67+][adv-esr60.7+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: