Last Comment Bug 803853 - (CVE-2013-0766) [FIX] Heap-use-after-free in ~nsHTMLEditRules
(CVE-2013-0766)
: [FIX] Heap-use-after-free in ~nsHTMLEditRules
Status: RESOLVED FIXED
[adv-main18+][adv-esr17+][adv-esr10+]
: crash, csectype-uaf, sec-critical, testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86_64 All
: -- normal (vote)
: mozilla20
Assigned To: Olli Pettay [:smaug]
:
Mentors:
: 820998 (view as bug list)
Depends on: 842892
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-20 09:22 PDT by Abhishek Arya
Modified: 2015-11-10 05:14 PST (History)
13 users (show)
abillings: sec‑bounty+
anthony.s.hughes: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
+
fixed
+
fixed
+
fixed
18+
fixed
18+
fixed


Attachments
Testcase (2.28 KB, text/html)
2012-10-20 09:22 PDT, Abhishek Arya
no flags Details
patch? (2.54 KB, patch)
2012-12-05 07:44 PST, Olli Pettay [:smaug]
ehsan: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr17+
abillings: sec‑approval+
Details | Diff | Splinter Review
esr10 (2.35 KB, patch)
2012-12-12 07:05 PST, Olli Pettay [:smaug]
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-10-20 09:22:24 PDT
Created attachment 673576 [details]
Testcase

Reproduces on trunk by starting multiple firefox instances (~ 4-5) with the testcase on command line.

=================================================================
==25662== ERROR: AddressSanitizer heap-use-after-free on address 0x7fed9ab46400 at pc 0x7fedd68877a0 bp 0x7fffd167aef0 sp 0x7fffd167aee8
READ of size 8 at 0x7fed9ab46400 thread T0
    #0 0x7fedd688779f in ~nsHTMLEditRules editor/libeditor/html/nsHTMLEditRules.cpp:212
    #1 0x7fedd688745c in ~nsHTMLEditRules editor/libeditor/html/nsHTMLEditRules.cpp:205
    #2 0x7fedd623fec4 in nsTextEditRules::Release() editor/libeditor/text/nsTextEditRules.cpp:93
    #3 0x7fedd6887bc7 in nsHTMLEditRules::Release() editor/libeditor/html/nsHTMLEditRules.cpp:220
    #4 0x7fedd69ab0bd in nsRunnableMethodReceiver<nsHTMLEditRules, true>::Revoke() ../../../dist/include/nsThreadUtils.h:304
    #5 0x7fedd69ab596 in ~nsRunnableMethodReceiver ../../../dist/include/nsThreadUtils.h:303
    #6 0x7fedd69ab376 in ~nsRunnableMethodReceiver ../../../dist/include/nsThreadUtils.h:303
    #7 0x7fedd69ab25d in nsRunnableMethodImpl<void (nsHTMLEditRules::*)(), true>::~nsRunnableMethodImpl() ../../../dist/include/nsThreadUtils.h:333
    #8 0x7fedd69aac06 in nsRunnableMethodImpl<void (nsHTMLEditRules::*)(), true>::~nsRunnableMethodImpl() ../../../dist/include/nsThreadUtils.h:333
    #9 0x7fedd69aad3c in nsRunnableMethodImpl<void (nsHTMLEditRules::*)(), true>::~nsRunnableMethodImpl() ../../../dist/include/nsThreadUtils.h:333
    #10 0x7feddcc16022 in nsRunnable::Release() objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:30
    #11 0x7fedcf72428b in ~nsCOMPtr_base xpcom/build/../glue/nsCOMPtr.h:408
    #12 0x7fedcf7c54e9 in nsCOMPtr<nsIRunnable>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #13 0x7fedcf7c0cc6 in nsCOMPtr<nsIRunnable>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #14 0x7fedd0a3b3b6 in nsTArrayElementTraits<nsCOMPtr<nsIRunnable> >::Destruct(nsCOMPtr<nsIRunnable>*) ../../dist/include/nsTArray.h:359
    #15 0x7fedd0a3b121 in nsTArray<nsCOMPtr<nsIRunnable>, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) ../../dist/include/nsTArray.h:1223
    #16 0x7fedd0a3ab88 in nsTArray<nsCOMPtr<nsIRunnable>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) ../../dist/include/nsTArray.h:943
    #17 0x7fedd2f8aa9b in nsContentUtils::RemoveScriptBlocker() content/base/src/nsContentUtils.cpp:5007
    #18 0x7fedd14abc6e in ~nsAutoScriptBlocker ../../../dist/include/nsContentUtils.h:2315
    #19 0x7fedd14973d6 in ~nsAutoScriptBlocker ../../../dist/include/nsContentUtils.h:2314
    #20 0x7fedd31ab405 in nsDocument::AdoptNode(nsIDOMNode*, nsIDOMNode**) content/base/src/nsDocument.cpp:6410
    #21 0x7fedd83b0bab in nsIDOMDocument_AdoptNode(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:1453
    #22 0x7fede44440a9 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:364
    #23 0x7fede43f3cc0 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2372
    #24 0x7fede4352cbe in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) js/src/jsinterp.cpp:324
    #25 0x7fede44518ad in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) js/src/jsinterp.cpp:510
    #26 0x7fede445353b in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/jsinterp.cpp:547
    #27 0x7fede3b67a5d in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:5702
    #28 0x7fedd527e52e in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) dom/base/nsJSEnvironment.cpp:1507
    #29 0x7fedd543a736 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:9701
    #30 0x7fedd53f0c34 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:9960
    #31 0x7fedd54385e8 in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10227
    #32 0x7feddcfc3b02 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:472
    #33 0x7feddcfc500a in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:555
    #34 0x7feddcf88710 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:612
    #35 0x7feddcc1957b in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #36 0x7feddb5a75d6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
    #37 0x7feddd2662c1 in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:215
    #38 0x7feddd2660f6 in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:208
    #39 0x7feddd265fdb in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:182
    #40 0x7feddaa4da7a in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
    #41 0x7fedd967cc54 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:290
    #42 0x7fedcf7604bd in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3792
    #43 0x7fedcf766335 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3858
    #44 0x7fedcf7691e4 in XRE_main toolkit/xre/nsAppRunner.cpp:3933
    #45 0x40bb13 in do_main(int, char**) browser/app/nsBrowserApp.cpp:174
    #46 0x409255 in main browser/app/nsBrowserApp.cpp:279
    #47 0x7fedee6bf76c in ?? ??:0
0x7fed9ab46400 is located 0 bytes inside of 824-byte region [0x7fed9ab46400,0x7fed9ab46738)
freed by thread T0 here:
    #0 0x4c3660 in __interceptor_free ??:?
    #1 0x7fedebd38406 in moz_free memory/mozalloc/mozalloc.cpp:51
    #2 0x7fedd678db5d in operator delete(void*) ../../../dist/include/mozilla/mozalloc.h:224
    #3 0x7fedd6275a84 in nsEditor::Release() editor/libeditor/base/nsEditor.cpp:211
    #4 0x7fedd6795bd7 in nsHTMLEditor::Release() editor/libeditor/html/nsHTMLEditor.cpp:203
    #5 0x7fedcf72428b in ~nsCOMPtr_base ../../../../dist/include/nsCOMPtr.h:408
    #6 0x7fedd1a97149 in nsCOMPtr<nsIEditor>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #7 0x7fedd1a7f6e6 in nsCOMPtr<nsIEditor>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #8 0x7fedd9cbc0e2 in ~nsEditorSpellCheck editor/composer/src/nsEditorSpellCheck.cpp:219
    #9 0x7fedd9cbbecc in ~nsEditorSpellCheck editor/composer/src/nsEditorSpellCheck.cpp:215
    #10 0x7fedd9cb9254 in nsEditorSpellCheck::Release() editor/composer/src/nsEditorSpellCheck.cpp:193
    #11 0x7fedcf72428b in ~nsCOMPtr_base ../../../../dist/include/nsCOMPtr.h:408
    #12 0x7fedd631c389 in nsCOMPtr<nsIEditorSpellCheck>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #13 0x7fedd6295f86 in nsCOMPtr<nsIEditorSpellCheck>::~nsCOMPtr() ../../../dist/include/nsCOMPtr.h:447
    #14 0x7feddb0484dd in ~mozInlineSpellChecker extensions/spellcheck/src/mozInlineSpellChecker.cpp:506
    #15 0x7feddb0481cc in ~mozInlineSpellChecker extensions/spellcheck/src/mozInlineSpellChecker.cpp:505
    #16 0x7feddb0456b4 in mozInlineSpellChecker::Release() extensions/spellcheck/src/mozInlineSpellChecker.cpp:479
    #17 0x7feddb0701cb in ~nsRefPtr ../../../dist/include/nsAutoPtr.h:874
    #18 0x7feddb06ffd6 in ~nsRefPtr ../../../dist/include/nsAutoPtr.h:872
    #19 0x7feddb06fece in mozInlineSpellStatus::~mozInlineSpellStatus() extensions/spellcheck/src/mozInlineSpellChecker.h:33
    #20 0x7feddb051d96 in mozInlineSpellStatus::~mozInlineSpellStatus() extensions/spellcheck/src/mozInlineSpellChecker.h:33
    #21 0x7feddb06ec3d in mozInlineSpellResume::~mozInlineSpellResume() extensions/spellcheck/src/mozInlineSpellChecker.cpp:451
    #22 0x7feddb06e926 in mozInlineSpellResume::~mozInlineSpellResume() extensions/spellcheck/src/mozInlineSpellChecker.cpp:451
    #23 0x7feddb06ea5c in mozInlineSpellResume::~mozInlineSpellResume() extensions/spellcheck/src/mozInlineSpellChecker.cpp:451
    #24 0x7feddcc16022 in nsRunnable::Release() objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:30
    #25 0x7fedcf72428b in ~nsCOMPtr_base ../../../../dist/include/nsCOMPtr.h:408
    #26 0x7fedcf7c54e9 in nsCOMPtr<nsIRunnable>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:447
    #27 0x7fedcf7c0cc6 in nsCOMPtr<nsIRunnable>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:447
    #28 0x7feddcf88810 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:618
    #29 0x7feddcc1957b in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
previously allocated by thread T0 here:
    #0 0x4c3720 in malloc ??:?
    #1 0x7fedebd3855a in moz_xmalloc memory/mozalloc/mozalloc.cpp:57
    #2 0x7fedd12ca9ee in operator new(unsigned long) ../../dist/include/mozilla/mozalloc.h:200
    #3 0x7feddcc543fb in mozilla::GenericFactory::CreateInstance(nsISupports*, nsID const&, void**) objdir-ff-asan-sym/xpcom/build/GenericFactory.cpp:16
    #4 0x7feddcf3289c in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) xpcom/components/nsComponentManager.cpp:1006
    #5 0x7feddcbde0b9 in CallCreateInstance(char const*, nsISupports*, nsID const&, void**) objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:135
    #6 0x7feddcbdf48b in nsCreateInstanceByContractID::operator()(nsID const&, void**) const objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:178
    #7 0x7feddcbd2ddc in nsCOMPtr_base::assign_from_helper(nsCOMPtr_helper const&, nsID const&) objdir-ff-asan-sym/xpcom/build/nsCOMPtr.cpp:110
    #8 0x7fedd4211b6e in nsCOMPtr<nsIEditor>::operator=(nsCOMPtr_helper const&) ../../../dist/include/nsCOMPtr.h:689
    #9 0x7fedd9c93154 in nsEditingSession::SetupEditorOnWindow(nsIDOMWindow*) editor/composer/src/nsEditingSession.cpp:423
    #10 0x7fedd9c8a600 in nsEditingSession::MakeWindowEditable(nsIDOMWindow*, char const*, bool, bool, bool) editor/composer/src/nsEditingSession.cpp:173
    #11 0x7fedd4a89a3b in nsHTMLDocument::EditingStateChanged() content/html/document/src/nsHTMLDocument.cpp:2658
    #12 0x7fedd4aad55c in nsHTMLDocument::MaybeEditingStateChanged() content/html/document/src/nsHTMLDocument.cpp:2304
    #13 0x7fedd4aadd0d in nsHTMLDocument::EndUpdate(unsigned int) content/html/document/src/nsHTMLDocument.cpp:2317
    #14 0x7fedd6600f0a in nsHtml5TreeOpExecutor::EndDocUpdate() parser/html/nsHtml5TreeOpExecutor.h:248
    #15 0x7fedd6600548 in nsHtml5TreeOpExecutor::DidBuildModel(bool) parser/html/nsHtml5TreeOpExecutor.cpp:131
    #16 0x7fedd65ed858 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) parser/html/nsHtml5TreeOperation.cpp:639
    #17 0x7fedd6604b05 in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:564
    #18 0x7fedd66411e9 in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:127
    #19 0x7feddcf88710 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:612
    #20 0x7feddcc1957b in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #21 0x7feddb5a75d6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
    #22 0x7feddd2662c1 in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:215
    #23 0x7feddd2660f6 in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:208
    #24 0x7feddd265fdb in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:182
    #25 0x7feddaa4da7a in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
    #26 0x7fedd967cc54 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:290
    #27 0x7fedcf7604bd in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3792
    #28 0x7fedcf766335 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3858
    #29 0x7fedcf7691e4 in XRE_main toolkit/xre/nsAppRunner.cpp:3933
Shadow byte and word:
  0x1ffdb3568c80: fd
  0x1ffdb3568c80: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ffdb3568c60: fa fa fa fa fa fa fa fa
  0x1ffdb3568c68: fa fa fa fa fa fa fa fa
  0x1ffdb3568c70: fa fa fa fa fa fa fa fa
  0x1ffdb3568c78: fa fa fa fa fa fa fa fa
=>0x1ffdb3568c80: fd fd fd fd fd fd fd fd
  0x1ffdb3568c88: fd fd fd fd fd fd fd fd
  0x1ffdb3568c90: fd fd fd fd fd fd fd fd
  0x1ffdb3568c98: fd fd fd fd fd fd fd fd
  0x1ffdb3568ca0: fd fd fd fd fd fd fd fd
Stats: 271M malloced (1203M for red zones) by 552799 calls
Stats: 45M realloced by 28360 calls
Stats: 233M freed by 308733 calls
Stats: 155M really freed by 234701 calls
Stats: 960M (245847 full pages) mmaped in 240 calls
  mmaps   by size class: 11:313191; 12:4096; 13:1536; 14:1280; 15:256; 16:832; 17:1216; 18:176; 19:40; 20:24;
  mallocs by size class: 11:539587; 12:5764; 13:2495; 14:1669; 15:381; 16:1295; 17:1349; 18:196; 19:41; 20:22;
  frees   by size class: 11:298141; 12:3893; 13:2206; 14:1451; 15:324; 16:1189; 17:1331; 18:141; 19:38; 20:19;
  rfrees  by size class: 11:227507; 12:2307; 13:1313; 14:1203; 15:223; 16:990; 17:1125; 18:28; 19:4; 20:1;
Stats: malloc large: 1608 small slow: 9746
==25662== ABORTING
Comment 1 :Ehsan Akhgari 2012-10-22 13:34:35 PDT
Aryeh, can you please take a look at this?  Thanks!
Comment 2 :Aryeh Gregor (away until August 15) 2012-10-25 05:29:42 PDT
Ehsan, is the right fix here to make nsHTMLEditRules::mHTMLEditor a WeakPtr?  By the stack traces, it seems similar to bug 795804 -- something whose execution was deferred held a reference to the nsHTMLEditRules, so the editor object was destroyed before the rules.  The alternative fix would be to track down what's holding a reference and fix it, but a) the stack traces don't immediately tell me what was holding a reference, b) it might not be easy for it not to hold a reference, c) that doesn't prevent similar bugs in the future.
Comment 3 :Ehsan Akhgari 2012-10-25 08:45:06 PDT
Could it be this code? <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/nsHTMLEditRules.cpp#8823>  That's the only place where this should happen, I think.  But I don't see how that could happen, since when the editor object dies, it should call DetachEditor() on its rules, which should take care of this.  Another part of the mystery is why the runnable method object is losing its reference to the rules object...

Can you try breaking in the debugger and see why DetachEditor doesn't do its job?  I really like to understand what's going on before wallpapering over it...
Comment 4 Daniel Veditz [:dveditz] 2012-10-25 13:47:20 PDT
I'm guessing this would be an old problem... ESR-10 old?
Comment 5 :Ehsan Akhgari 2012-10-25 14:43:27 PDT
(In reply to Daniel Veditz [:dveditz] from comment #4)
> I'm guessing this would be an old problem... ESR-10 old?

Probably.
Comment 6 :Aryeh Gregor (away until August 15) 2012-10-27 11:11:02 PDT
I can't reproduce this on a locally-compiled debug build with repeated refreshing.  I guess it's about time I get an ASAN build working.

I'm not sure I understand the first stack trace in comment #0 -- what's actually being executed here?  Tell me if I'm right: the method that queues tasks to be run gets both an object pointer and a pointer to a method of that object.  It keeps a strong reference to the object so it doesn't go away.  The stack here is where it releases the object, which destroys it, which tries to access the already-freed editor.  But you think when the editor died, it should have called DetachEditor(), which should already have un-queued anything (somehow?) by calling mTimer->Cancel().

So assuming I reproduce this one way or the other, what should I look for in the debugger, exactly?  I don't think my understanding of what's going on here is clear enough that I know what to look for.  I'd probably want to look at the stack where the use-after-free happens and try to figure out what script runner this is by inspecting the nsTArray's elements somehow?

What do you mean by "why the runnable method object is losing its reference to the rules object"?
Comment 7 :Ehsan Akhgari 2012-10-29 10:53:13 PDT
nsTextEditRules::DetachEditor sets mEditor to null, and nsHTMLEditRules::DetachEditor sets mHTMLEditor to null, so if DetachEditor is called, the nsHTMLEditRules object should no longer try to access the editor.  Also, NS_NewRunnableMethod creates an owning pointer which calls AddRef when the object is created and Release when it goes away, so the nsHTMLEditRules object cannot (should not?) die before the runnable is executed.

Now, the free() stack (the second one) in comment 0 shows that the editor object is dying.  That _should_ call DetachEditor on its rules member, which _should_ make the nsHTMLEditRules object not dereference the editor pointers any longer.  Also, since the runnable object also owns the rules object, that object should not die prematurely.

The use-after-free stack (the first one) in comment 0 shows that the rules object's destructor is on top, which means that we're either double-deleting that object, or its editor pointer is somehow not null and pointing to something that is not freed/etc.  So, setting breakpoints on the destructors for all of the editor and rules types involved can show you what is going on when these objects are being destroyed, and if we're double-deleting the rules object, etc.

Note that on a non-ASAN build, this may not crash/etc. but still the ordering of the events should be reproducible, so you can just set those breakpoints and investigate the order in which the objects are being destroyed, etc.

Please let me know if this helps.
Comment 8 :Aryeh Gregor (away until August 15) 2012-11-04 04:48:15 PST
When I set breakpoints in a regular debug build on the two relevant destructors and both DetachEditor variants, I get the following when loading the test page: ~nsHTMLEditor calls nsHTMLEditRules::DetachEditor, which calls nsTextEditRules::DetachEditor, which calls ~nsHTMLEditRules.  The top of the stack at this point is:

#0  nsHTMLEditRules::~nsHTMLEditRules (this=0x9f3f6800, __in_chrg=<optimized out>)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditRules.cpp:204
#1  0xb433fecf in nsHTMLEditRules::~nsHTMLEditRules (this=0x9f3f6800, __in_chrg=<optimized out>)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditRules.cpp:213
#2  0xb424e121 in nsTextEditRules::Release (this=0x9f3f6800)
    at /mnt/ssd/checkouts/central/editor/libeditor/text/nsTextEditRules.cpp:93
#3  0xb433ff73 in nsHTMLEditRules::Release (this=0x9f3f6800)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditRules.cpp:220
#4  0xb424cc10 in nsCOMPtr<nsIEditRules>::~nsCOMPtr (this=0x9def68ac, __in_chrg=<optimized out>)
    at ../../../dist/include/nsCOMPtr.h:492
#5  0xb4245541 in nsPlaintextEditor::~nsPlaintextEditor (this=0x9def6800, __in_chrg=<optimized out>)
    at /mnt/ssd/checkouts/central/editor/libeditor/text/nsPlaintextEditor.cpp:84
#6  0xb431324f in nsHTMLEditor::~nsHTMLEditor (this=0x9def6800, __in_chrg=<optimized out>)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditor.cpp:100
#7  0xb431328b in nsHTMLEditor::~nsHTMLEditor (this=0x9def6800, __in_chrg=<optimized out>)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditor.cpp:146
#8  0xb425583d in nsEditor::Release (this=0x9def6800)
    at /mnt/ssd/checkouts/central/editor/libeditor/base/nsEditor.cpp:211
#9  0xb4314039 in nsHTMLEditor::Release (this=0x9def6800)
    at /mnt/ssd/checkouts/central/editor/libeditor/html/nsHTMLEditor.cpp:203
#10 0xb3906270 in nsCOMPtr<nsIEditor>::~nsCOMPtr (this=0xbfffb170, __in_chrg=<optimized out>)
    at ../../dist/include/nsCOMPtr.h:492
#11 0xb492090a in nsEditingSession::TearDownEditorOnWindow (this=0x98987460, aWindow=0xa1736e60)
    at /mnt/ssd/checkouts/central/editor/composer/src/nsEditingSession.cpp:593
#12 0xb3ef2f4e in nsHTMLDocument::TurnEditingOff (this=0xaa4bb000)
    at /mnt/ssd/checkouts/central/content/html/document/src/nsHTMLDocument.cpp:2573
#13 0xb3ef31bd in nsHTMLDocument::EditingStateChanged (this=0xaa4bb000)
    at /mnt/ssd/checkouts/central/content/html/document/src/nsHTMLDocument.cpp:2622

There is no subsequent call to ~nsHTMLEditRules for that object.  So I don't think I'm seeing the same double-free as in comment #0, am I?

(I see the above sequence of calls twice per page load, but it's for two different editor objects and two different rules objects.  Also, it asks to open a pop-up window, but nothing changes depending on whether I allow it to.)
Comment 9 :Ehsan Akhgari 2012-11-05 11:51:57 PST
Hmm, so I tried to debug this myself, and I can't reproduce the bug in my ASAN build.  Abhishek, can you reproduce this now?
Comment 10 Abhishek Arya 2012-11-05 12:06:38 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #9)
> Hmm, so I tried to debug this myself, and I can't reproduce the bug in my
> ASAN build.  Abhishek, can you reproduce this now?

Reproduces easily on trunk on an unoptimized ASAN release build (--disable-optimize, --disable-debug) with like ~7-8 firefox instances started at once with testcase on command line.
Comment 11 Abhishek Arya 2012-11-05 12:12:18 PST
Looks like you need to use prefs.js from https://github.com/mozilla/ADBFuzz/blob/master/misc/prefs.js. This looks to disable the popup blocker which is required for the repro.
Comment 12 :Ehsan Akhgari 2012-11-05 12:25:16 PST
Hmm, can you pastebin your mozconfig, please?
Comment 13 Abhishek Arya 2012-11-05 12:28:43 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #12)
> Hmm, can you pastebin your mozconfig, please?

http://pastebin.com/CLBc3b5D

You can use latest clang from http://commondatastorage.googleapis.com/chromium-browser-clang/index.html?path=Linux_x64/
Comment 14 :Ehsan Akhgari 2012-11-05 12:33:29 PST
I'm now building with the mozconfig in comment 13.  Also, I'm not sure what the significance of opening several firefox instances is.  Those instances don't talk to each other, so this bug should happen regardless, right?
Comment 15 :Ehsan Akhgari 2012-11-05 12:50:53 PST
Hmm, I still can't reproduce.  Not sure how to make progress here.  Aryeh, did you try to reproduce this on an ASAN build?
Comment 16 Abhishek Arya 2012-11-05 12:58:35 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> I'm now building with the mozconfig in comment 13.  Also, I'm not sure what
> the significance of opening several firefox instances is.  Those instances
> don't talk to each other, so this bug should happen regardless, right?

Those instances don't talk to each other, so yes this bug should happen regardless. However there are always bugs like these which are dependent on the right timing for things to trigger. I see setTimeout calls in the reduced testcase which makes me suspect this requires some subtle timing between the clear() and editFuzz() functions that becomes easier to reproduce when things get slowed (due to starting of multiple parallel instances).

# Python script i use. Make sure to use prefs.js from above comment.
import os
import time

thread_num = 8

for i in range(0, thread_num):
  profile_dir = '/tmp/firefox_profile_%d' % i
  os.system('mkdir %s' % profile_dir)
  os.system('cp ~/prefs.js %s/' % profile_dir)
  cmd = '~/firefox/src/objdir-ff-asan-sym/dist/bin/firefox-bin -no-remote -profile "%s" "path_to_testcase/test.html" 2>&1 | tee /tmp/%d.log &' % (profile_dir, i)

  os.system(cmd)
  time.sleep(0.3)
Comment 17 :Aryeh Gregor (away until August 15) 2012-11-06 04:37:24 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #15)
> Hmm, I still can't reproduce.  Not sure how to make progress here.  Aryeh,
> did you try to reproduce this on an ASAN build?

No.
Comment 18 :Ehsan Akhgari 2012-11-06 11:07:53 PST
Can you do that, please?  Thanks!
Comment 19 :Aryeh Gregor (away until August 15) 2012-11-07 04:38:38 PST
My development machine is 32-bit, so actually building ASAN myself would require either reinstalling my OS or setting up a build environment on my laptop, either of which would probably take too much time to be worth the benefit.

Anyway, I tried using the provided script and pref file with one of the prebuilt ASAN builds.  It didn't seem to do anything -- all eight instances were still running just fine, and the tails of /tmp/*.log looked like normal output for a Firefox build.

FWIW, the cocoa_window_focus.html file that's opened with open() doesn't exist -- is it supposed to?  There is such a file in the source tree, dom/plugins/test/mochitest/cocoa_window_focus.html, which is used for mochitests on OS X, but it's not in the same directory as the test file (and anyway seems unlikely to do anything interesting here).
Comment 20 Abhishek Arya 2012-11-07 08:46:31 PST
(In reply to :Aryeh Gregor from comment #19)
> My development machine is 32-bit, so actually building ASAN myself would
> require either reinstalling my OS or setting up a build environment on my
> laptop, either of which would probably take too much time to be worth the
> benefit.
> 
> Anyway, I tried using the provided script and pref file with one of the
> prebuilt ASAN builds.  It didn't seem to do anything -- all eight instances
> were still running just fine, and the tails of /tmp/*.log looked like normal
> output for a Firefox build.
> 
> FWIW, the cocoa_window_focus.html file that's opened with open() doesn't
> exist -- is it supposed to?  There is such a file in the source tree,
> dom/plugins/test/mochitest/cocoa_window_focus.html, which is used for
> mochitests on OS X, but it's not in the same directory as the test file (and
> anyway seems unlikely to do anything interesting here).

1. that cocoa_window_focus.html is not required.
2. Did you use the prefs.js from https://github.com/mozilla/ADBFuzz/blob/master/misc/prefs.js and sustituted the correct location in the python script.
3. can you try increasing the number of instances. like 12, 15 ?
Comment 21 :Aryeh Gregor (away until August 15) 2012-11-08 05:04:14 PST
(In reply to Abhishek Arya from comment #20)
> 1. that cocoa_window_focus.html is not required.

Okay.

> 2. Did you use the prefs.js from
> https://github.com/mozilla/ADBFuzz/blob/master/misc/prefs.js and sustituted
> the correct location in the python script.

Yes -- the popups did appear, they weren't blocked.

> 3. can you try increasing the number of instances. like 12, 15 ?

Aha -- with twelve at once I can reproduce in some of the instances consistently.  And when I try on my (much faster) desktop with regular debug builds, I can reproduce when starting 24 or so at once.  It will be a pain to debug this, but it shouldn't be impossible.
Comment 22 Daniel Veditz [:dveditz] 2012-11-08 13:32:18 PST
cdiehl: is this a case TSAN can help narrow down?
Comment 23 Christoph Diehl [:posidron] 2012-11-08 16:48:01 PST
Note, It can determine if there is a possible a race prior but won't recognize the use-after-free bug. I also would be interested if we can prevent such bugs by looking closer into threading issues.

nsHTMLEditRules and nsHTMLEditor were not in the stacks of TSan with this testcase, neither in hybrid nor in pure-happens-before mode.

The output is quite huge in hybrid mode, it will take a while do digger through it.

In pure-happens-before mode the results were poor, only 10 possible races, none of stacks looked similar to stack of this bug.

If we can find out the exact cause of this issue, we have perhaps a better chance to know after what we need to look out for and can then apply that pattern to prevent such kind of bugs in the future.
Comment 24 :Aryeh Gregor (away until August 15) 2012-11-13 04:10:38 PST
Okay, so now I managed to reproduce in a debugger, I think.  If I run twelve instances at once in a debugger, I get some that segfault, which is good enough to work with.  I have to run now, but I'll investigate more later.  ~nsHTMLEditRules is being run twice, it seems -- once by ~nsHTMLEditor and once by nsRunnableMethodReceiver::Revoke.  But "this" is different both times, so I don't know why it's a double-free.  This is unlike the cases that don't crash, where there's no second ~nsHTMLEditRules call.  I'll add some more breakpoints and see what I can figure out.
Comment 25 :Aryeh Gregor (away until August 15) 2012-11-22 04:21:52 PST
I tried again, and now I can't reproduce anymore.  Even if I could, I wouldn't be sure I could make any progress from there.  I really don't think I have the time that this bug needs -- it's both hard to reproduce and puzzling to me when I do reproduce it.  So I'm unassigning from myself.
Comment 26 Olli Pettay [:smaug] 2012-11-26 03:57:16 PST
I can't reproduce this, but I'll inspect the code.
Comment 27 Abhishek Arya 2012-11-26 20:02:06 PST
(In reply to Olli Pettay [:smaug] from comment #26)
> I can't reproduce this, but I'll inspect the code.

Reproduces on trunk using steps in c#16 [so that popup blocker is disabled and you need to play with number of instances]. Here is the stack with new line numbers.

=================================================================
==31746== ERROR: AddressSanitizer: heap-use-after-free on address 0x7fe2f8ad9c40 at pc 0x7fe32786e9e3 bp 0x7fff560be610 sp 0x7fff560be608
READ of size 8 at 0x7fe2f8ad9c40 thread T0
    #0 0x7fe32786e9e2 in ~nsHTMLEditRules src/editor/libeditor/html/nsHTMLEditRules.cpp:212
    #1 0x7fe32786e69c in ~nsHTMLEditRules src/editor/libeditor/html/nsHTMLEditRules.cpp:205
    #2 0x7fe32725a458 in nsTextEditRules::Release() src/editor/libeditor/text/nsTextEditRules.cpp:93
    #3 0x7fe32786efc7 in nsHTMLEditRules::Release() src/editor/libeditor/html/nsHTMLEditRules.cpp:220
    #4 0x7fe3279989fd in nsRunnableMethodReceiver<nsHTMLEditRules, true>::Revoke() src/../../../dist/include/nsThreadUtils.h:305
    #5 0x7fe327998ec5 in ~nsRunnableMethodReceiver src/../../../dist/include/nsThreadUtils.h:304
    #6 0x7fe327998cb5 in ~nsRunnableMethodReceiver src/../../../dist/include/nsThreadUtils.h:304
    #7 0x7fe327998b9d in ~nsRunnableMethodImpl src/../../../dist/include/nsThreadUtils.h:334
    #8 0x7fe327998555 in ~nsRunnableMethodImpl src/../../../dist/include/nsThreadUtils.h:334
    #9 0x7fe32799867c in ~nsRunnableMethodImpl src/../../../dist/include/nsThreadUtils.h:334
    #10 0x7fe32e1e1d02 in nsRunnable::Release() src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:31
    #11 0x7fe31fd47c3b in ~nsCOMPtr_base src/xpcom/build/../glue/nsCOMPtr.h:410
    #12 0x7fe31fde45c8 in ~nsCOMPtr src/../../../dist/include/nsCOMPtr.h:449
    #13 0x7fe31fddff85 in ~nsCOMPtr src/../../../dist/include/nsCOMPtr.h:449
    #14 0x7fe32110bad5 in nsTArrayElementTraits<nsCOMPtr<nsIRunnable> >::Destruct(nsCOMPtr<nsIRunnable>*) src/../../dist/include/nsTArray.h:360
    #15 0x7fe32110b841 in nsTArray<nsCOMPtr<nsIRunnable>, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) src/../../dist/include/nsTArray.h:1224
    #16 0x7fe32110b2a8 in nsTArray<nsCOMPtr<nsIRunnable>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) src/../../dist/include/nsTArray.h:944
    #17 0x7fe3237054bf in nsContentUtils::RemoveScriptBlocker() src/content/base/src/nsContentUtils.cpp:4982
    #18 0x7fe321bb351d in ~nsAutoScriptBlocker src/../../../dist/include/nsContentUtils.h:2336
    #19 0x7fe321b9ed65 in ~nsAutoScriptBlocker src/../../../dist/include/nsContentUtils.h:2335
    #20 0x7fe32394297c in nsDocument::AdoptNode(nsIDOMNode*, nsIDOMNode**) src/content/base/src/nsDocument.cpp:5973
    #21 0x7fe329811982 in nsIDOMDocument_AdoptNode(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:1388
    #22 0x7fe335a33089 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:364
    #23 0x7fe3359e2875 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2334
    #24 0x7fe3359424ee in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) src/js/src/jsinterp.cpp:326
    #25 0x7fe335a4068d in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) src/js/src/jsinterp.cpp:515
    #26 0x7fe335a4234b in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) src/js/src/jsinterp.cpp:552
    #27 0x7fe335191bed in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) src/js/src/jsapi.cpp:5612
    #28 0x7fe32610bfcb in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) src/dom/base/nsJSEnvironment.cpp:1520
    #29 0x7fe3262cd071 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) src/dom/base/nsGlobalWindow.cpp:9686
    #30 0x7fe326281b3e in nsGlobalWindow::RunTimeout(nsTimeout*) src/dom/base/nsGlobalWindow.cpp:9945
    #31 0x7fe3262cb186 in nsGlobalWindow::TimerCallback(nsITimer*, void*) src/dom/base/nsGlobalWindow.cpp:10214
    #32 0x7fe32e5ab1a8 in nsTimerImpl::Fire() src/xpcom/threads/nsTimerImpl.cpp:482
    #33 0x7fe32e5ac631 in nsTimerEvent::Run() src/xpcom/threads/nsTimerImpl.cpp:565
    #34 0x7fe32e56f32e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
    #35 0x7fe32e1e4f7f in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:221
    #36 0x7fe32c8cda16 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
    #37 0x7fe32e857e2e in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:215
    #38 0x7fe32e857c75 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:208
    #39 0x7fe32e857b5b in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
    #40 0x7fe32bce1414 in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
    #41 0x7fe32a844232 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:290
    #42 0x7fe31fd7dbd4 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3823
    #43 0x7fe31fd838a9 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
    #44 0x7fe31fd86620 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4084
    #45 0x40c146 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
    #46 0x409980 in main src/browser/app/nsBrowserApp.cpp:279
    #47 0x7fe33fff976c in ?? ??:0
0x7fe2f8ad9c40 is located 0 bytes inside of 824-byte region [0x7fe2f8ad9c40,0x7fe2f8ad9f78)
freed by thread T0 here:
    #0 0x4c37d0 in __interceptor_free ??:?
    #1 0x7fe33db87405 in moz_free src/memory/mozalloc/mozalloc.cpp:48
    #2 0x7fe32776803d in operator delete(void*) src/../../../dist/include/mozilla/mozalloc.h:224
    #3 0x7fe327291e78 in nsEditor::Release() src/editor/libeditor/base/nsEditor.cpp:212
    #4 0x7fe32776e277 in nsHTMLEditor::Release() src/editor/libeditor/html/nsHTMLEditor.cpp:203
    #5 0x7fe31fd47c3b in ~nsCOMPtr_base src/objdir-ff-asan-sym/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsCOMPtr.h:410
    #6 0x7fe3221f01f8 in ~nsCOMPtr src/../../../dist/include/nsCOMPtr.h:449
    #7 0x7fe3221d6895 in ~nsCOMPtr src/../../../dist/include/nsCOMPtr.h:449
    #8 0x7fe32aec25b3 in ~nsEditorSpellCheck src/editor/composer/src/nsEditorSpellCheck.cpp:238
    #9 0x7fe32aec239c in ~nsEditorSpellCheck src/editor/composer/src/nsEditorSpellCheck.cpp:234
    #10 0x7fe32aebfd08 in nsEditorSpellCheck::Release() src/editor/composer/src/nsEditorSpellCheck.cpp:212
    #11 0x7fe31fd47c3b in ~nsCOMPtr_base src/objdir-ff-asan-sym/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsCOMPtr.h:410
    #12 0x7fe32733f4d8 in nsCOMPtr<nsIEditorSpellCheck>::~nsCOMPtr() src/../../../dist/include/nsCOMPtr.h:449
    #13 0x7fe3272b3485 in nsCOMPtr<nsIEditorSpellCheck>::~nsCOMPtr() src/../../../dist/include/nsCOMPtr.h:449
    #14 0x7fe32c33b19d in ~mozInlineSpellChecker src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:506
previously allocated by thread T0 here:
    #0 0x4c3890 in malloc ??:?
    #1 0x7fe33db87541 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7fe3219d161e in operator new(unsigned long) src/../../dist/include/mozilla/mozalloc.h:200
    #3 0x7fe32e2201db in mozilla::GenericFactory::CreateInstance(nsISupports*, nsID const&, void**) src/objdir-ff-asan-sym/xpcom/build/GenericFactory.cpp:16
    #4 0x7fe32e511da6 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1035
    #5 0x7fe32e1a8ea8 in CallCreateInstance(char const*, nsISupports*, nsID const&, void**) src/objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:135
    #6 0x7fe32e1aa26b in nsCreateInstanceByContractID::operator()(nsID const&, void**) const src/objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:178
    #7 0x7fe32e19dd0c in nsCOMPtr_base::assign_from_helper(nsCOMPtr_helper const&, nsID const&) src/objdir-ff-asan-sym/xpcom/build/nsCOMPtr.cpp:110
    #8 0x7fe324b556dc in nsCOMPtr<nsIEditor>::operator=(nsCOMPtr_helper const&) src/../../../dist/include/nsCOMPtr.h:691
Shadow byte and word:
  0x1ffc5f15b388: fd
  0x1ffc5f15b388: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ffc5f15b368: fa fa fa fa fa fa fa fa
  0x1ffc5f15b370: fa fa fa fa fa fa fa fa
  0x1ffc5f15b378: fa fa fa fa fa fa fa fa
  0x1ffc5f15b380: fa fa fa fa fa fa fa fa
=>0x1ffc5f15b388: fd fd fd fd fd fd fd fd
  0x1ffc5f15b390: fd fd fd fd fd fd fd fd
  0x1ffc5f15b398: fd fd fd fd fd fd fd fd
  0x1ffc5f15b3a0: fd fd fd fd fd fd fd fd
  0x1ffc5f15b3a8: fd fd fd fd fd fd fd fd
Stats: 273M malloced (265M for red zones) by 559778 calls
Stats: 48M realloced by 28825 calls
Stats: 232M freed by 305754 calls
Stats: 197M really freed by 243492 calls
Stats: 254M (65239 full pages) mmaped in 486 calls
  mmaps   by size class: 7:237510; 8:51175; 9:18414; 10:7154; 11:7140; 12:1536; 13:1088; 14:544; 15:240; 16:752; 17:460; 18:34; 19:35; 20:21;
  mallocs by size class: 7:392383; 8:98405; 9:28888; 10:10881; 11:18705; 12:2854; 13:2369; 14:1714; 15:446; 16:1605; 17:1393; 18:72; 19:41; 20:22;
  frees   by size class: 7:191174; 8:62353; 9:20864; 10:6861; 11:16276; 12:1839; 13:1787; 14:1490; 15:309; 16:1313; 17:1374; 18:57; 19:38; 20:19;
  rfrees  by size class: 7:160691; 8:48116; 9:11251; 10:4336; 11:12368; 12:1351; 13:1296; 14:1325; 15:220; 16:1127; 17:1315; 18:50; 19:37; 20:9;
Stats: malloc large: 3579 small slow: 6109
==31746== ABORTING
Comment 28 Olli Pettay [:smaug] 2012-12-05 07:44:44 PST
Created attachment 688780 [details] [diff] [review]
patch?

Abhishek, could you test this?

Without the change to ::Init (which gets called also for nsHTMLEditor)
MOZ_ASSERT(!mRules) doesn't pass, which means mRules points to a
new rules object yet the old one has still a raw pointer to the editor.
Comment 29 Olli Pettay [:smaug] 2012-12-05 07:47:42 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

Regardless whether the patch fixes this particular bug, I think we should
take it. It fixes an obvious bug and adds useful assertions.
mRules->DetachEditor(); could be perhaps moved to ::PreDestroy, but
I want to be super-safe and make sure we don't re-create mRules without
destroying the detaching the old object first, so I prefer ::Init.
Comment 30 :Ehsan Akhgari 2012-12-05 07:58:23 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

Review of attachment 688780 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.
Comment 31 Olli Pettay [:smaug] 2012-12-05 08:19:37 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

[Security approval request comment]
How easily can the security issue be deduced from the patch?
It is quite easy to see the issue, but it is hard to find a testcase
which crashes reliably.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The commit message will be about leak, since the patch makes us
release the old object earlier.

Which older supported branches are affected by this flaw?
As far as I see, all

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch applies to all the branches (17esr, 18, 19, 20)

How likely is this patch to cause regressions; how much testing does it need?
A bit difficult to say. Certainly not super safe.
Comment 32 Olli Pettay [:smaug] 2012-12-05 08:23:23 PST
Though, for the sake of sane bugzilla queries I should move the patch to another bug if it doesn't
fix the problem Abhishek is seeing.
Comment 33 :Ehsan Akhgari 2012-12-05 08:36:39 PST
This also affects esr10, I think.
Comment 34 Abhishek Arya 2012-12-05 20:27:59 PST
(In reply to Olli Pettay [:smaug] from comment #32)
> Though, for the sake of sane bugzilla queries I should move the patch to
> another bug if it doesn't
> fix the problem Abhishek is seeing.

Verified in my build. The fix works out nicely and no crash occurs. I did make sure to check that without your patch, it does crash.
Comment 35 Olli Pettay [:smaug] 2012-12-06 05:23:51 PST
Thanks Abhishek!
Comment 36 Olli Pettay [:smaug] 2012-12-06 17:11:40 PST
https://hg.mozilla.org/mozilla-central/rev/c6e99da8ea9d
Comment 37 Olli Pettay [:smaug] 2012-12-06 17:14:21 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

[Approval Request Comment]
Bug caused by (feature/regressing bug #): really old stuff
User impact if declined: s-s crashes
Testing completed (on m-c, etc.): Just landed to m-c
Risk to taking this patch (and alternatives if risky): 
A bit difficult to say. Certainly not super safe. But anyhow the best option.
String or UUID changes made by this patch: NA
Comment 38 Lukas Blakk [:lsblakk] use ?needinfo 2012-12-07 09:52:19 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

let's go ahead with Aurora uplift and give it a bit of time there first.
Comment 40 Alex Keybl [:akeybl] 2012-12-11 15:41:50 PST
Comment on attachment 688780 [details] [diff] [review]
patch?

Given the risk evaluation above, there is a non-zero chance of regression. If we do regress on Beta, we have a higher than normal risk of:

1) Shipping FF18 with the regression due to a lack of feedback over the holidays, and thus requiring a re-spin
2) Backing ourselves into a corner when we do regress since the fix landed on Beta and we need to resolve before ship (and over the holidays)

Since this got sec-approval and landed, we don't have a lot of choice though.
Comment 41 Alex Keybl [:akeybl] 2012-12-11 15:42:16 PST
We also need an ESR10 patch.
Comment 43 Olli Pettay [:smaug] 2012-12-12 07:05:31 PST
Created attachment 691336 [details] [diff] [review]
esr10
Comment 45 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 10:44:56 PST
Requesting this testcase be put in testsuite if possible.
Comment 47 Daniel Veditz [:dveditz] 2013-01-09 11:21:46 PST
*** Bug 820998 has been marked as a duplicate of this bug. ***
Comment 49 Adrian Crespo 2015-11-09 17:02:20 PST
Is 1195029 related to this bug?
Comment 50 Olli Pettay [:smaug] 2015-11-10 05:14:17 PST
Nothing hints that this is related to that (except crash happening in the same file).

Note You need to log in before you can comment on or make changes to this bug.