Closed Bug 1585760 Opened 6 years ago Closed 6 years ago

AddressSanitizer: heap-use-after-free [@ mozilla::layers::PersistentBufferProviderShared::Create] with READ of size 8

Categories

(Core :: Graphics: Canvas2D, defect, P1)

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla72
Tracking Flags:
Tracking Status
firefox-esr68 71+ fixed
firefox70 --- wontfix
firefox71 + fixed
firefox72 + fixed

People

(Reporter: jkratzer, Assigned: nical)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [adv-main71+r][adv-esr68.3+r])

Attachments

(3 files)

Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr68+
tjr
: sec-approval+
Details | Review
Bug 1585760 - Store a flag in the TextureChild indicating use of the ImageBridge. r=sotaro
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr68+
tjr
: sec-approval+
Details | Review
Bug 1585760 - Check that GetTextureForwarder isn't null in more places. r=sotaro
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr68+
tjr
: sec-approval+
Details | Review

Found while fuzzing mozilla-central rev e545376c3391. I don't currently have a working testcase but will update if one becomes available.

==128192==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000048ef8 at pc 0x7f7000f35d53 bp 0x7ffd0622a390 sp 0x7ffd0622a388
READ of size 8 at 0x617000048ef8 thread T0 (file:// Content)
    #0 0x7f7000f35d52 in mozilla::layers::PersistentBufferProviderShared::Create(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, mozilla::layers::KnowsCompositor*) /src/gfx/layers/PersistentBufferProvider.cpp:103:49
    #1 0x7f70012eb83b in mozilla::layers::WebRenderLayerManager::CreatePersistentBufferProvider(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /src/gfx/layers/wr/WebRenderLayerManager.cpp:746:7
    #2 0x7f7004a6384b in mozilla::dom::CanvasRenderingContext2D::TrySharedTarget(RefPtr<mozilla::gfx::DrawTarget>&, RefPtr<mozilla::layers::PersistentBufferProvider>&) /src/dom/canvas/CanvasRenderingContext2D.cpp:1435:32
    #3 0x7f7004a6204b in mozilla::dom::CanvasRenderingContext2D::EnsureTarget(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, bool) /src/dom/canvas/CanvasRenderingContext2D.cpp:1300:8
    #4 0x7f7004a7f76c in mozilla::dom::CanvasRenderingContext2D::EnsureWritablePath() /src/dom/canvas/CanvasRenderingContext2D.cpp:3068:3
    #5 0x7f7004a81650 in mozilla::dom::CanvasRenderingContext2D::Rect(double, double, double, double) /src/dom/canvas/CanvasRenderingContext2D.cpp:3030:3
    #6 0x7f700355ac48 in mozilla::dom::CanvasRenderingContext2D_Binding::rect(JSContext*, JS::Handle<JSObject*>, mozilla::dom::CanvasRenderingContext2D*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:5529:24
    #7 0x7f7004966516 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3250:13
    #8 0x7f700b5cd51c in CallJSNative /src/js/src/vm/Interpreter.cpp:458:13
    #9 0x7f700b5cd51c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /src/js/src/vm/Interpreter.cpp:551
    #10 0x7f700b5b5ba0 in CallFromStack /src/js/src/vm/Interpreter.cpp:624:10
    #11 0x7f700b5b5ba0 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3113
    #12 0x7f700b59733f in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:424:10
    #13 0x7f700b5ce026 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /src/js/src/vm/Interpreter.cpp:592:13
    #14 0x7f700b5d0379 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /src/js/src/vm/Interpreter.cpp:637:8
    #15 0x7f700c18b96b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2728:10
    #16 0x7f700412aed0 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
    #17 0x7f7005104765 in HandleEvent<mozilla::dom::EventTarget *> /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
    #18 0x7f7005104765 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1033
    #19 0x7f70051061db in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /src/dom/events/EventListenerManager.cpp:1231:17
    #20 0x7f70050ecbea in HandleEvent /src/obj-firefox/dist/include/mozilla/EventListenerManager.h:353:5
    #21 0x7f70050ecbea in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:349
    #22 0x7f70050eb402 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:551:16
    #23 0x7f70050f0dee in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:1045:11
    #24 0x7f7007ac2414 in nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1170:7
    #25 0x7f700a719ba1 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:6568:20
    #26 0x7f700a718e1e in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:6346:7
    #27 0x7f700a71da5f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp
    #28 0x7f70008fbd6c in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1354:3
    #29 0x7f70008fa9ec in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:913:14
    #30 0x7f70008f65b9 in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:732:9
    #31 0x7f70008f9466 in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:620:5
    #32 0x7f70008fa5cc in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp
    #33 0x7f6ffe0a6730 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:568:22
    #34 0x7f7001fbbbf8 in DoUnblockOnload /src/dom/base/Document.cpp:10769:18
    #35 0x7f7001fbbbf8 in mozilla::dom::Document::UnblockOnload(bool) /src/dom/base/Document.cpp:10701
    #36 0x7f7005082fda in ~LoadBlockingAsyncEventDispatcher /src/dom/events/AsyncEventDispatcher.cpp:117:18
    #37 0x7f7005082fda in mozilla::LoadBlockingAsyncEventDispatcher::~LoadBlockingAsyncEventDispatcher() /src/dom/events/AsyncEventDispatcher.cpp:115
    #38 0x7f6ffde2357c in Release /src/xpcom/threads/nsThreadUtils.cpp:54:1
    #39 0x7f6ffde2357c in mozilla::CancelableRunnable::Release() /src/xpcom/threads/nsThreadUtils.cpp:76
    #40 0x7f6ffdde3c84 in assign_assuming_AddRef /src/obj-firefox/dist/include/nsCOMPtr.h:367:7
    #41 0x7f6ffdde3c84 in assign_assuming_AddRef /src/obj-firefox/dist/include/nsCOMPtr.h:390
    #42 0x7f6ffdde3c84 in operator= /src/obj-firefox/dist/include/nsCOMPtr.h:678
    #43 0x7f6ffdde3c84 in mozilla::SchedulerGroup::Runnable::Run() /src/xpcom/threads/SchedulerGroup.cpp:299
    #44 0x7f6ffde153b9 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1225:14
    #45 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #46 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #47 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #48 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #49 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #50 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #51 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #52 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #53 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #54 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #55 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #56 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #57 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #58 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #59 0x5607b17d821c in _start (/home/worker/builds/m-c-20190930162137-fuzzing-asan-opt/firefox+0x4921c)

0x617000048ef8 is located 248 bytes inside of 648-byte region [0x617000048e00,0x617000049088)
freed by thread T0 (file:// Content) here:
    #0 0x5607b18838b2 in __interceptor_free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f70016d2f7c in Release /src/obj-firefox/dist/include/mozilla/layers/CompositorBridgeChild.h:60:3
    #2 0x7f70016d2f7c in Release /src/gfx/layers/../../mfbt/RefPtr.h:48
    #3 0x7f70016d2f7c in Release /src/gfx/layers/../../mfbt/RefPtr.h:373
    #4 0x7f70016d2f7c in assign_assuming_AddRef /src/gfx/layers/../../mfbt/RefPtr.h:67
    #5 0x7f70016d2f7c in operator= /src/gfx/layers/../../mfbt/RefPtr.h:162
    #6 0x7f70016d2f7c in Revoke /src/obj-firefox/dist/include/nsThreadUtils.h:749
    #7 0x7f70016d2f7c in Revoke /src/obj-firefox/dist/include/nsThreadUtils.h:1190
    #8 0x7f70016d2f7c in ~RunnableMethodImpl /src/obj-firefox/dist/include/nsThreadUtils.h:1151
    #9 0x7f70016d2f7c in mozilla::detail::RunnableMethodImpl<RefPtr<mozilla::layers::CompositorBridgeChild>, void (mozilla::layers::CompositorBridgeChild::*)(), true, (mozilla::RunnableKind)0>::~RunnableMethodImpl() /src/obj-firefox/dist/include/nsThreadUtils.h:1151
    #10 0x7f6ffddfff4c in mozilla::Runnable::Release() /src/xpcom/threads/nsThreadUtils.cpp:54:1
    #11 0x7f6ffde15b9a in ~nsCOMPtr_base /src/obj-firefox/dist/include/nsCOMPtr.h:331:7
    #12 0x7f6ffde15b9a in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1272
    #13 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #14 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #15 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #16 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #17 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #18 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #19 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #20 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #21 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #22 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #23 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #24 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #25 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #26 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

previously allocated by thread T0 (file:// Content) here:
    #0 0x5607b1883c33 in __interceptor_malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x5607b18b92bd in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f70016973ab in operator new /src/obj-firefox/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f70016973ab in mozilla::layers::CompositorManagerChild::CreateContentCompositorBridge(unsigned int) /src/gfx/layers/ipc/CompositorManagerChild.cpp:105
    #4 0x7f7006949501 in mozilla::dom::ContentChild::RecvInitRendering(mozilla::ipc::Endpoint<mozilla::layers::PCompositorManagerChild>&&, mozilla::ipc::Endpoint<mozilla::layers::PImageBridgeChild>&&, mozilla::ipc::Endpoint<mozilla::gfx::PVRManagerChild>&&, mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&, nsTArray<unsigned int>&&) /src/dom/ipc/ContentChild.cpp:1544:8
    #5 0x7f6fff3cb3b5 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PContentChild.cpp:8247:56
    #6 0x7f6fff04b316 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2185:25
    #7 0x7f6fff045f6d in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2109:9
    #8 0x7f6fff048597 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1954:3
    #9 0x7f6fff049427 in mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1985:13
    #10 0x7f6ffde153b9 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1225:14
    #11 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #12 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #13 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #14 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #15 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #16 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #17 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #18 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #19 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #20 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #21 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #22 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #23 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #24 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-use-after-free /src/gfx/layers/PersistentBufferProvider.cpp:103:49 in mozilla::layers::PersistentBufferProviderShared::Create(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, mozilla::layers::KnowsCompositor*)
Shadow bytes around the buggy address:
  0x0c2e80001180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2e800011c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c2e800011d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]
  0x0c2e800011e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001210: fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2e80001220: 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
==128192==ABORTING
Group: core-security → gfx-core-security
Priority: -- → P3

Nical, any suggestions as to what might be causing this?

Flags: needinfo?(nical.bugzilla)

It looks like we are shutting down or for some othre reason the CompositorBridgeChild is already shut down through the process of getting it as a TextureForwarder we use the IPDL-generated Manager() method to go from WebRenderBridgeChild to CompositorBirdgeChild and that doesn't check reference counting ot other mechanism.

Checking IPCOpen() before calling Manager() in WebRenderBridgeChild::GetCompositorBridgeChild should fix this.

Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)

This is more complicated than anticipated. Between the time IPCOpen() starts returning false and when the parent protocol dies, there appears to be a number of uses of GetCompositorBridgeChild and similar methods with some mutex interactions leading to timeouts.

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:nical, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(nical.bugzilla)

This avoids calling GetTextureForwarder during shutdown which may return a null pointer.

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Hard. It's a shutdown bug that relies on destruction order that isn't predictable.
  • 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?: All
  • 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 patches will probably apply without conflicts.
  • How likely is this patch to cause regressions; how much testing does it need?: Safe enough for Nightly. I would rather let it bake for a bit before thinking about uplifting as it may cause null-pointer crashes here and there.
    No need for testing. We just need to let it bake for a while and look out for nullptr shutdown crashes.
Attachment #9100885 - Flags: sec-approval?
Attachment #9106177 - Flags: sec-approval?
Attachment #9106178 - Flags: sec-approval?
Attachment #9100885 - Flags: sec-approval? → sec-approval+
Attachment #9106177 - Flags: sec-approval? → sec-approval+

Comment on attachment 9106178 [details]
Bug 1585760 - Check that GetTextureForwarder isn't null in more places. r=sotaro

Okay to land

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

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Priority: P3 → P1

Does this need Beta and ESR68 approval requests?

Flags: needinfo?(nical.bugzilla)

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Beta/Release Uplift Approval Request

  • User impact if declined: Potential shutdown UAF
  • 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: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): The patch has been in nightly for two weeks without making waves. The shutdown logic is complicated so it gets a "medium" on the other hand time shutdown crashes don't tend to ruin the user experience as much as other crashes.
    No risk of causing security issues (nullptr crahes at worst).
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: potential shutdown UAF.
  • Fix Landed on Version: 72
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky):
  • String or UUID changes made by this patch:
Flags: needinfo?(nical.bugzilla)
Attachment #9100885 - Flags: approval-mozilla-esr68?
Attachment #9100885 - Flags: approval-mozilla-beta?
Attachment #9106177 - Flags: approval-mozilla-esr68?
Attachment #9106178 - Flags: approval-mozilla-esr68?

Bugzilla gave me an error about setting the flags but the uplift requests apply to the other two patches as well.

Pascal, I'm ok with this for ESR if you'll take it for the 71 RC since it's had some bake time without bad results.

Flags: needinfo?(pascalc)

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Approved for RC

Flags: needinfo?(pascalc)
Attachment #9100885 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9100885 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9106177 - Flags: approval-mozilla-esr68?
Attachment #9106177 - Flags: approval-mozilla-esr68+
Attachment #9106177 - Flags: approval-mozilla-beta+
Attachment #9106178 - Flags: approval-mozilla-esr68?
Attachment #9106178 - Flags: approval-mozilla-esr68+
Attachment #9106178 - Flags: approval-mozilla-beta+
Whiteboard: [adv-main71+r]
Whiteboard: [adv-main71+r] → [adv-main71+r][adv-esr68.3+r]
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: