Found while fuzzing m-c 20221003-ad67ead1905a (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Note: Test case uses FuzzingFunctions.garbageCollect().

==20307==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000084698 at pc 0x7f537f973811 bp 0x7ffd01f36dc0 sp 0x7ffd01f36db8
WRITE of size 8 at 0x604000084698 thread T0 (Isolated Web Co)
    #0 0x7f537f973810 in mozilla::DoublyLinkedList<nsIMutationObserver, mozilla::GetDoublyLinkedListElement<nsIMutationObserver> >::remove(nsIMutationObserver*) /builds/worker/workspace/obj-build/dist/include/mozilla/DoublyLinkedList.h:330:38
    #1 0x7f537fcec982 in RemoveMutationObserver /builds/worker/checkouts/gecko/dom/base/nsINode.h:1158:29
    #2 0x7f537fcec982 in nsMutationReceiver::Disconnect(bool) /builds/worker/checkouts/gecko/dom/base/nsDOMMutationObserver.cpp:124:22
    #3 0x7f537fd9d1d6 in nsINode::LastRelease() /builds/worker/checkouts/gecko/dom/base/nsINode.cpp:665:15
    #4 0x7f537fa5aecb in nsIContent::Release() /builds/worker/checkouts/gecko/dom/base/FragmentOrElement.cpp:148:1
    #5 0x7f537e6a6edb in ~nsCOMPtr_base /builds/worker/workspace/obj-build/dist/include/nsCOMPtr.h:328:7
    #6 0x7f537e6a6edb in Destruct /builds/worker/workspace/obj-build/dist/include/nsTArray.h:642:45
    #7 0x7f537e6a6edb in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::DestructRange(unsigned long, unsigned long) /builds/worker/workspace/obj-build/dist/include/nsTArray.h:2408:7
    #8 0x7f537e682d6f in ClearAndRetainStorage /builds/worker/workspace/obj-build/dist/include/nsTArray.h:1477:5
    #9 0x7f537e682d6f in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::~nsTArray_Impl() /builds/worker/workspace/obj-build/dist/include/nsTArray.h:1031:7
    #10 0x7f537e682cd2 in nsHtml5OplessBuilder::~nsHtml5OplessBuilder() /builds/worker/checkouts/gecko/parser/html/nsHtml5DocumentBuilder.cpp:35:52
    #11 0x7f537e7218dd in nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor() /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:131:49
    #12 0x7f537c5a5155 in SnowWhiteKiller::Visit(nsPurpleBuffer&, nsPurpleBufferEntry*) /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp:2470:9
    #13 0x7f537c582996 in void nsPurpleBuffer::VisitEntries<SnowWhiteKiller>(SnowWhiteKiller&) /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp:939:23
    #14 0x7f537c583232 in nsCycleCollector::FreeSnowWhiteWithBudget(js::SliceBudget&) /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp:2638:14
    #15 0x7f537e23af7c in AsyncFreeSnowWhite::Run() /builds/worker/checkouts/gecko/js/xpconnect/src/XPCJSRuntime.cpp:155:9
    #16 0x7f537c77e179 in IdleRunnableWrapper::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:309:22
    #17 0x7f537c783db2 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:538:16
    #18 0x7f537c7445fd in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:851:26
    #19 0x7f537c741a85 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:725:15
    #20 0x7f537c741e90 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:461:36
    #21 0x7f537c78cce1 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:187:37
    #22 0x7f537c78cce1 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
    #23 0x7f537c765937 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1205:16
    #24 0x7f537c76fdb4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:465:10
    #25 0x7f537df120df in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
    #26 0x7f537dd8fbe1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
    #27 0x7f537dd8fbe1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
    #28 0x7f537dd8fbe1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
    #29 0x7f5385221da7 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:150:27
    #30 0x7f538a3d0597 in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:880:20
    #31 0x7f537dd8fbe1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
    #32 0x7f537dd8fbe1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
    #33 0x7f537dd8fbe1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
    #34 0x7f538a3cf47c in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:739:34
    #35 0x55d0aa24e575 in content_process_main(mozilla::Bootstrap*, int, char**) /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #36 0x55d0aa24e9c7 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:359:18
    #37 0x7f53abac2c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #38 0x55d0aa18e9b9 in _start (/home/twsmith/workspace/browsers/m-c-20221003094838-fuzzing-asan-opt/firefox+0x7a9b9) (BuildId: 468903e27e768f771f137d350fbab75ef3a40402)

0x604000084698 is located 8 bytes inside of 48-byte region [0x604000084690,0x6040000846c0)
freed by thread T0 (Isolated Web Co) here:
    #0 0x55d0aa210c0f in __interceptor_free /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3

previously allocated by thread T0 (Isolated Web Co) here:
    #0 0x55d0aa210ebb in __interceptor_malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/obj-build/dist/include/mozilla/DoublyLinkedList.h:330:38 in mozilla::DoublyLinkedList<nsIMutationObserver, mozilla::GetDoublyLinkedListElement<nsIMutationObserver> >::remove(nsIMutationObserver*)
Shadow bytes around the buggy address:
  0x0c0880008880: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 06 fa
  0x0c0880008890: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 fa
  0x0c08800088a0: fa fa 00 00 00 00 00 04 fa fa fd fd fd fd fd fa
  0x0c08800088b0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
  0x0c08800088c0: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 fa
=>0x0c08800088d0: fa fa fd[fd]fd fd fd fd fa fa fd fd fd fd fd fa
  0x0c08800088e0: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fa
  0x0c08800088f0: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 00 01
  0x0c0880008900: fa fa 00 00 00 00 00 02 fa fa fd fd fd fd fd fa
  0x0c0880008910: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 00 01
  0x0c0880008920: fa fa 00 00 00 00 00 03 fa fa 00 00 00 00 04 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
==20307==ABORTING

A Pernosco session is available here: https://pernos.co/debug/XsRslButiRutaQcw62jIfA/index.html

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20221003212025-d1d1d489003c.
The bug appears to have been introduced in the following build range:

Start: ddd9b4ac6f847aa0e758a7559fb284eda9210d39 (20220921103207)
End: 857f67ef123a600c2d20b98b60f6baefe0dca400 (20220921145442)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ddd9b4ac6f847aa0e758a7559fb284eda9210d39&tochange=857f67ef123a600c2d20b98b60f6baefe0dca400

This smells like someone isn't removing mutation observer from the node and bug 1777925 just revealed the issue. But that is just a theory.

We seem to create MutationObserverWrapper* 0x6040000c8fd0 for nsFormFillController::StartControllingInput and that seems to be the problematic case here.

Jan, would you have time to take a look at this?

oh, the issue is different after all, sort of.
https://searchfox.org/mozilla-central/rev/560b7b1b174ed36912b969eee0c1920f3c59bc56/dom/base/nsStubMutationObserver.cpp#103-109 doesn't remove the wrapper from the list, but some other mutation observers do remove themselves from the list, and when they do removal, there can be deleted objects around.
We need a aNode->RemoveMutationObserver(this); call there

Or, remove mutation observer in https://searchfox.org/mozilla-central/rev/560b7b1b174ed36912b969eee0c1920f3c59bc56/toolkit/components/satchel/nsFormFillController.cpp#203-213

Removing aNode->RemoveMutationObserver(this); isn't a good idea because aNode is const. const_casting the object for lookup is one thing, doing it for calling a non-const method another...

Isn't it rather an issue in the SafeDoublyLinkedList, which we could solve by adding some "safe-deleting" logic to the destructor of the List Element:

if (mNext != nullptr || mPrev != nullptr) {
  if (mPrev) {
    mPrev->mNext = mNext;
  }
  if(mNext) {
    mNext->mPrev = mPrev;
  }
}

That way the element would remove itself from the list when it is destructed.

We could do also that, but need to ensure iterators are updated correctly.

I do consider the real bug being in nsFormFillController, since it doesn't remove itself being an observer, but I guess it would be good to make the API less error prone.

(and I don't in general care too much about const. It doesn't tend to provide much of anything, other than possibly some false feeling of safety )

Okay, so I'll just do both (update the list and nsFormFillController).

Set release status flags based on info from the regressing bug 1777925

Landed: https://hg.mozilla.org/integration/autoland/rev/928edafe970f97e160eb0306098a9731f39b5302

Backout for causing leak failures.: https://hg.mozilla.org/integration/autoland/rev/4d17ec8f84e1fb3f9ef92366b0fb7da9139e56bb

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=63d9cf4baca9c90da52ad355ca689e1872443bcc&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Cdebug%2Cmochitests%2Ctest-linux1804-64-qr%2Fdebug-mochitest-devtools-chrome%2Cdt4&tochange=928edafe970f97e160eb0306098a9731f39b5302&selectedTaskRun=EZd3NGXySW631Gvn7thyMg.0
log: https://treeherder.mozilla.org/logviewer?job_id=393016871&repo=autoland

Failure line:

TEST-UNEXPECTED-FAIL | leakcheck | default process: negative leaks caught!

Fixed: https://treeherder.mozilla.org/jobs?repo=try&revision=556c57cdba143c0f14cf05c3aba40a611084fd9f
This was only a refcount logging issue.

Fixed memory issue in NodeWillBeDestroyed() for MultiMutationObserver classes. r=smaug
https://hg.mozilla.org/integration/autoland/rev/69a5c8ef2a45c68debfc86ae0effb432fa798cc2
https://hg.mozilla.org/mozilla-central/rev/69a5c8ef2a45

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20221013224755-a1624dfaa169.