771873 - (CVE-2012-3961) Heap-use-after-free in RangeData::~RangeData

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Reproduces on trunk
20120707214206
http://hg.mozilla.org/mozilla-central/rev/9533b40ff28b

=================================================================
==25564== ERROR: AddressSanitizer heap-use-after-free on address 0x7f1cf1264780 at pc 0x7f1d2220e700 bp 0x7fffcdd81470 sp 0x7fffcdd81468
READ of size 8 at 0x7f1cf1264780 thread T0
    #0 0x7f1d2220e700 in ~nsRefPtr firefox/src/../../dist/include/nsAutoPtr.h:874
    #1 0x7f1d221a42b3 in ~nsRefPtr firefox/src/../../dist/include/nsAutoPtr.h:875
    #2 0x7f1d2288b4d3 in RangeData::~RangeData() firefox/src/../../dist/include/mozilla/Selection.h:24
    #3 0x7f1d227ea3b3 in RangeData::~RangeData() firefox/src/../../dist/include/mozilla/Selection.h:24
    #4 0x7f1d228838e3 in nsTArrayElementTraits<RangeData>::Destruct(RangeData*) firefox/src/../../dist/include/nsTArray.h:349
    #5 0x7f1d228823be in nsTArray<RangeData, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) firefox/src/../../dist/include/nsTArray.h:1210
    #6 0x7f1d22852ab7 in nsTArray<RangeData, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) firefox/src/../../dist/include/nsTArray.h:932
    #7 0x7f1d2283db0d in mozilla::Selection::AddItem(nsRange*, int*) firefox/src/layout/generic/nsSelection.cpp:3535
    #8 0x7f1d228674fe in mozilla::Selection::AddRange(nsIDOMRange*) firefox/src/layout/generic/nsSelection.cpp:4414
    #9 0x7f1d2b8c58db in mozInlineSpellChecker::AddRange(nsISelection*, nsIDOMRange*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1600
    #10 0x7f1d2b8c2d1d in mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil&, nsISelection*, mozInlineSpellStatus*, bool*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1399
    #11 0x7f1d2b8c70f1 in mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1488
    #12 0x7f1d2b8cf9dc in mozInlineSpellResume::Run() firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:462
    #13 0x7f1d2d045ccd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #14 0x7f1d2ccd501d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #15 0x7f1d2bd76226 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #16 0x7f1d2d2f984a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #17 0x7f1d2d2f9693 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #18 0x7f1d2d2f9578 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #19 0x7f1d2b2ae42e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #20 0x7f1d29ef9668 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:257
    #21 0x7f1d20736280 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3787
    #22 0x7f1d2073cc22 in XREMain::XRE_main(int, char**, nsXREAppData const*) firefox/src/toolkit/xre/nsAppRunner.cpp:3864
    #23 0x7f1d207400f2 in XRE_main firefox/src/toolkit/xre/nsAppRunner.cpp:3940
    #24 0x40c28f in do_main(int, char**) firefox/src/browser/app/nsBrowserApp.cpp:160
    #25 0x409cbd in main firefox/src/browser/app/nsBrowserApp.cpp:330
    #26 0x7f1d3b388c4d in ?? ??:0
0x7f1cf1264780 is located 0 bytes inside of 64-byte region [0x7f1cf1264780,0x7f1cf12647c0)
freed by thread T0 here:
    #0 0x4a4392 in free ??:0
    #1 0x7f1d382165c3 in moz_free firefox/src/memory/mozalloc/mozalloc.cpp:49
    #2 0x7f1d23d7e6c6 in ~nsRange firefox/src/content/base/src/nsRange.cpp:215
    #3 0x7f1d23d82997 in nsRange::Release() firefox/src/content/base/src/nsRange.cpp:258
    #4 0x7f1d2220e6a0 in ~nsRefPtr firefox/src/../../../dist/include/nsAutoPtr.h:874
    #5 0x7f1d221a42b3 in ~nsRefPtr firefox/src/../../../dist/include/nsAutoPtr.h:875
    #6 0x7f1d2288b4d3 in RangeData::~RangeData() firefox/src/../../dist/include/mozilla/Selection.h:24
    #7 0x7f1d227ea3b3 in RangeData::~RangeData() firefox/src/../../dist/include/mozilla/Selection.h:24
    #8 0x7f1d228838e3 in nsTArrayElementTraits<RangeData>::Destruct(RangeData*) firefox/src/../../dist/include/nsTArray.h:349
    #9 0x7f1d228823be in nsTArray<RangeData, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) firefox/src/../../dist/include/nsTArray.h:1210
    #10 0x7f1d22852ab7 in nsTArray<RangeData, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) firefox/src/../../dist/include/nsTArray.h:932
    #11 0x7f1d2283db0d in mozilla::Selection::AddItem(nsRange*, int*) firefox/src/layout/generic/nsSelection.cpp:3535
    #12 0x7f1d228674fe in mozilla::Selection::AddRange(nsIDOMRange*) firefox/src/layout/generic/nsSelection.cpp:4414
    #13 0x7f1d2b8c58db in mozInlineSpellChecker::AddRange(nsISelection*, nsIDOMRange*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1600
    #14 0x7f1d2b8c2d1d in mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil&, nsISelection*, mozInlineSpellStatus*, bool*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1399
    #15 0x7f1d2b8c70f1 in mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1488
    #16 0x7f1d2b8cf9dc in mozInlineSpellResume::Run() firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:462
    #17 0x7f1d2d045ccd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #18 0x7f1d2ccd501d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #19 0x7f1d2bd76226 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #20 0x7f1d2d2f984a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #21 0x7f1d2d2f9693 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #22 0x7f1d2d2f9578 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #23 0x7f1d2b2ae42e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #24 0x7f1d29ef9668 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:257
    #25 0x7f1d20736280 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3787
    #26 0x7f1d2073cc22 in XREMain::XRE_main(int, char**, nsXREAppData const*) firefox/src/toolkit/xre/nsAppRunner.cpp:3864
    #27 0x7f1d207400f2 in XRE_main firefox/src/toolkit/xre/nsAppRunner.cpp:3940
    #28 0x40c28f in do_main(int, char**) firefox/src/browser/app/nsBrowserApp.cpp:160
    #29 0x409cbd in main firefox/src/browser/app/nsBrowserApp.cpp:330
previously allocated by thread T0 here:
    #0 0x4a4452 in __interceptor_malloc ??:0
    #1 0x7f1d38216717 in moz_xmalloc firefox/src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7f1d2b8dc0e3 in mozInlineSpellWordUtil::MakeRange(mozInlineSpellWordUtil::NodeOffset, mozInlineSpellWordUtil::NodeOffset, nsRange**) firefox/src/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp:322
    #3 0x7f1d2b8d9fc6 in mozInlineSpellWordUtil::MakeRangeForWord(mozInlineSpellWordUtil::RealWord const&, nsRange**) firefox/src/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp:226
    #4 0x7f1d2b8dee62 in mozInlineSpellWordUtil::GetNextWord(nsAString_internal&, nsRange**, bool*) firefox/src/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp:297
    #5 0x7f1d2b8c1906 in mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil&, nsISelection*, mozInlineSpellStatus*, bool*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1317
    #6 0x7f1d2b8c70f1 in mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus*) firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:1488
    #7 0x7f1d2b8cf9dc in mozInlineSpellResume::Run() firefox/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:462
    #8 0x7f1d2d045ccd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #9 0x7f1d2ccd501d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #10 0x7f1d2bd76226 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #11 0x7f1d2d2f984a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #12 0x7f1d2d2f9693 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #13 0x7f1d2d2f9578 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #14 0x7f1d2b2ae42e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #15 0x7f1d29ef9668 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:257
    #16 0x7f1d20736280 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3787
    #17 0x7f1d2073cc22 in XREMain::XRE_main(int, char**, nsXREAppData const*) firefox/src/toolkit/xre/nsAppRunner.cpp:3864
    #18 0x7f1d207400f2 in XRE_main firefox/src/toolkit/xre/nsAppRunner.cpp:3940
    #19 0x40c28f in do_main(int, char**) firefox/src/browser/app/nsBrowserApp.cpp:160
    #20 0x409cbd in main firefox/src/browser/app/nsBrowserApp.cpp:330
    #21 0x7f1d3b388c4d in ?? ??:0
==25564== ABORTING
Stats: 156M malloced (189M for red zones) by 464984 calls
Stats: 41M realloced by 20345 calls
Stats: 112M freed by 231687 calls
Stats: 0M really freed by 0 calls
Stats: 376M (96306 full pages) mmaped in 94 calls
  mmaps   by size class: 8:393192; 9:49146; 10:20475; 11:18423; 12:3072; 13:2048; 14:1536; 15:384; 16:576; 17:128; 18:176; 19:48; 20:16;
  mallocs by size class: 8:376983; 9:47309; 10:16409; 11:17335; 12:2458; 13:1841; 14:1416; 15:336; 16:562; 17:116; 18:165; 19:41; 20:13;
  frees   by size class: 8:163249; 9:36760; 10:12958; 11:14054; 12:1531; 13:948; 14:1228; 15:278; 16:474; 17:101; 18:58; 19:38; 20:10;
  rfrees  by size class:
Stats: malloc large: 335 small slow: 2084
Shadow byte and word:
  0x1fe39e24c8f0: fd
  0x1fe39e24c8f0: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1fe39e24c8d0: fd fd fd fd fd fd fd fd
  0x1fe39e24c8d8: fd fd fd fd fd fd fd fd
  0x1fe39e24c8e0: fa fa fa fa fa fa fa fa
  0x1fe39e24c8e8: fa fa fa fa fa fa fa fa
=>0x1fe39e24c8f0: fd fd fd fd fd fd fd fd
  0x1fe39e24c8f8: fd fd fd fd fd fd fd fd
  0x1fe39e24c900: fa fa fa fa fa fa fa fa
  0x1fe39e24c908: fa fa fa fa fa fa fa fa
  0x1fe39e24c910: fd fd fd fd fd fd fd fd

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 1, adds a check to avoid subtraction underflow

This doesn't address why startIndex and endIndex were reversed in the first place.  It does fix the crash.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 2, add extra assert to nsTArray::RemoveElementsAt

This would have caught the bug more cleanly.  There's already an assert to check that start + count <= Length(), but in this case that passed because the addition overflows -- count is actually 2^32 - 1.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I forgot to post my comment before posting patches:

(In reply to Abhishek Arya from comment #0)
>     #6 0x7f1d22852ab7 in nsTArray<RangeData,
> nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int)
> firefox/src/../../dist/include/nsTArray.h:932

The arguments are instructive when I reproduce locally:

#10 0xb491c287 in nsTArray<RangeData, nsTArrayDefaultAllocator>::RemoveElementsAt (this=0x98812ad4, 
    start=61, count=4294967295) at ../../dist/include/nsTArray.h:931

The call line is:

  mRanges.RemoveElementsAt(startIndex, endIndex - startIndex);

So looks like startIndex > endIndex here.  They're set by GetIndicesForInterval.  Specifically, sticking in some MOZ_ASSERTs and printf()s: FindInsertionPoint() sets endsBeforeIndex to 60, and beginsAfterIndex to 61.  mRanges.Length() is 65 (!).  aAllowAdjacent is false.

So the easy workaround is adding

  if (startIndex > endIndex) {
    PRInt32 tmp = startIndex;
    startIndex = endIndex;
    endIndex = tmp;
  }

and that fixes the crash/asserts.  Also, nsTArray should assert that count <= Length() to catch future cases like this more quickly.

That leaves the question of why GetIndicesForInterval ever returns a start index greater than the end index.  That can be fixed separately, though.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Try for the two patches so far: https://tbpl.mozilla.org/?tree=Try&rev=206ebf4d367c

Comment 5

[(no longer active)]

Why did you choose to wallpaper over the crash and not fix GetIndicesForInterval itself?  It seems to me that if GetIndicesForInterval has a bug, we should fix that, instead of leaving all of the other callers affected by this kind of behavior.

Also, please don't include the test case when you're going to land your patches here, as it might be potentially helpful to attackers turning this bug into an exploit.

Comment 6

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

(In reply to Ehsan Akhgari [:ehsan] from comment #5)
> Why did you choose to wallpaper over the crash and not fix
> GetIndicesForInterval itself?  It seems to me that if GetIndicesForInterval
> has a bug, we should fix that, instead of leaving all of the other callers
> affected by this kind of behavior.

Because it's a lot easier, and an immediate but incomplete fix seems valuable to me.  This I could do easily; actually figuring out what's wrong with GetIndicesForInterval is not as easy.  To fix other callers too, I could put the swapping in GetIndicesForInterval itself instead of in the caller.

> Also, please don't include the test case when you're going to land your
> patches here, as it might be potentially helpful to attackers turning this
> bug into an exploit.

Okay.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 1 -- Resume spell checking from a script runner

Okay, so this took a lot of pounding my head against the wall, but I eventually discovered this:

(gdb) p mRanges[60].mRange->mEndParent->mFirstChild
$55 = (nsSVGSVGElement *) 0x9e1f3b00
(gdb) p mRanges[60].mRange->mEndParent->mFirstChild->mNextSibling
$56 = (nsTextNode *) 0xa1579fb0
(gdb) p mRanges[60].mRange->mEndParent->mFirstChild->mNextSibling->mNextSibling
$57 = (nsTextNode *) 0xa15909c0
(gdb) p mRanges[60].mRange->mEndOffset
$58 = 2
(gdb) p mRanges[60].mRange->mStartParent
$59 = {mRawPtr = 0xa15909c0}

The end offset of this range is 2.  The start parent is the third child of the end parent.  So the range's start is before its end.  Thus the end of the new range is before the start of ranges 0-60, but its start is after the end of ranges 0-61.  So the indexes are quite legitimate; it's the range that's messed up.  The reason is simple: once again, range endpoints have not yet been updated.

Fortunately, the fix is simple.  For the future, I'm starting to think we should hardcode range endpoint updating into the core methods that mutate the DOM, before absolutely anything else happens.  This kind of thing is much too likely to happen given the way we do things now.

https://tbpl.mozilla.org/?tree=Try&rev=89b14e21f047

Comment 8

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 640557 [details] [diff] [review]
Patch part 1 -- Resume spell checking from a script runner

Okay, so judging by the try run, this seems to break spellcheck.  What do you suggest I do?

Comment 9

[Benjamin Smedberg]

Comment on attachment 640152 [details] [diff] [review]
Patch part 2, add extra assert to nsTArray::RemoveElementsAt

As I understand the most recent dev.platform discussions about underflow, this assert is defined behavior because the underflow of unsigned ints is defined behavior?

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Apparently, yes.  That's what 3.9.1.4 here seems to say:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf

Comment 11

[(no longer active)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> Comment on attachment 640152 [details] [diff] [review]
> Patch part 2, add extra assert to nsTArray::RemoveElementsAt
> 
> As I understand the most recent dev.platform discussions about underflow,
> this assert is defined behavior because the underflow of unsigned ints is
> defined behavior?

Hmm yeah this should make spell checking happen in sync, which at best should be a perf hit and at worst should break assumptions that the spellcheck code makes.

How about just doing what you did before in GetIndicesForInterval itself?  I don't have a lot of ideas here, and I agree that this is a pain to debug. :(

Comment 12

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 1, v2 -- Bail out if indices are reversed

This fixes the crash, but things will be all kinds of broken when mRanges has ranges in it with their ends before their starts.  So I left an NS_ASSERTION.  I guess a while after the fix has been pushed out to users, we can commit the crashtest too, right?

Comment 13

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=e64a1793a92f

Comment 14

[(no longer active)]

Comment on attachment 641030 [details] [diff] [review]
Patch part 1, v2 -- Bail out if indices are reversed

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

sigh....

Comment 15

[(no longer active)]

(In reply to :Aryeh Gregor from comment #12)
> Created attachment 641030 [details] [diff] [review]
> Patch part 1, v2 -- Bail out if indices are reversed
> 
> This fixes the crash, but things will be all kinds of broken when mRanges
> has ranges in it with their ends before their starts.  So I left an
> NS_ASSERTION.  I guess a while after the fix has been pushed out to users,
> we can commit the crashtest too, right?

Yes.  We usually wait until the bug gets opened up, which happens when the fix has shipped to users.

Comment 16

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c8be30a28b
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f00954a0301

Comment 17

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

https://hg.mozilla.org/mozilla-central/rev/a0c8be30a28b
https://hg.mozilla.org/mozilla-central/rev/3f00954a0301

Comment 18

[Alex Keybl [:akeybl]]

Until we have a regression window, it's unclear if this sg:crit bug should be considered for release in a FF14 chemspill. We'll wait to make that evaluation.

Comment 19

[Alex Keybl [:akeybl]]

Just to be clear, this would not drive a chemspill but would be considered if we need to roll a 14.0.2.

Comment 20

[juan becerra [:juanb]]

Narrowed it down to 12 and 13b1, which showed a spell checker uplift from mozilla-central:

https://bugzilla.mozilla.org/show_bug.cgi?id=743819

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75c7378c87b6&tochange=cc5254f9825f

Builds prior to 4/25 don't crash.

Comment 21

[Daniel Veditz [:dveditz]]

That should make ESR-10 unaffected.

Comment 22

[Alex Keybl [:akeybl]]

Discussed with Dan - we'll track for FF14, but only consider as part of a 14.0.2 chemspill if one is required for other reasons.

Comment 23

[Alex Keybl [:akeybl]]

(In reply to :Aryeh Gregor from comment #17)
> https://hg.mozilla.org/mozilla-central/rev/a0c8be30a28b
> https://hg.mozilla.org/mozilla-central/rev/3f00954a0301

Would you mind nominating for FF15 approval if deemed low risk? Thanks!

Comment 24

[(no longer active)]

Comment on attachment 641030 [details] [diff] [review]
Patch part 1, v2 -- Bail out if indices are reversed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: sec-critical
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): the patch is sane and relatively low risk.  It's basically a wallpaper.
String or UUID changes made by this patch: none

Comment 25

[Alex Keybl [:akeybl]]

Comment on attachment 641030 [details] [diff] [review]
Patch part 1, v2 -- Bail out if indices are reversed

[Triage Comment]
If landed before Tuesday, this will make it into beta 3 (which will give us time to react to any new regressions). Approved for branches.

Comment 26

[(no longer active)]

This had landed before the Aurora uplift, so no need to land it there...

Comment 27

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-beta/rev/268e31db5949

Comment 29

[u279076]

Attachment: ASAN Log

Testcase still crashes for me using :decoder's AddressSanitizer build for mozilla-central at revision 89dcadd42ec4 (debug), built on 20120803. Based on the date of check-in this should not be happening, should it?

Comment 30

[u279076]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #29)
> Created attachment 652939 [details]
> ASAN Log
> 
> Testcase still crashes for me using :decoder's AddressSanitizer build for
> mozilla-central at revision 89dcadd42ec4 (debug), built on 20120803. Based
> on the date of check-in this should not be happening, should it?

AddressSanitizer build for mozilla-aurora at revision 08e55f351f6d (debug) built on 20120817 does not reproduce a crash and displays "OTs@ tAar,t pf] |X# ]N2tKObhm?LAiAy )9.CDoic* J,F [NE(I[F;yI~aRni9 [[em4jOXgh uIVfj,}=j m[ MJ Ebc-dd%jWNDU7Hc,SNYXJk- mkd 5ejJ )E+R*WoNc W fEA ZXQ:O.l9q*QqMQ*NLEs~)m ^g%$Hz:4 $r8p{=#m@Pi- X" in content.

Comment 31

[(no longer active)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #29)
> Created attachment 652939 [details]
> ASAN Log
> 
> Testcase still crashes for me using :decoder's AddressSanitizer build for
> mozilla-central at revision 89dcadd42ec4 (debug), built on 20120803. Based
> on the date of check-in this should not be happening, should it?

Can you please get stacks for this crash?

Comment 32

[u279076]

(In reply to Ehsan Akhgari [:ehsan] from comment #31)
> (In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #29)
> Can you please get stacks for this crash?

I've never tried to retrieve a stack before -- can you please provide me some documentation/guidance?

Comment 33

[(no longer active)]

ASAN includes a script which symbolicates the call stacks: <http://code.google.com/p/address-sanitizer/wiki/AddressSanitizer#Call_stack>

Comment 34

[u279076]

I can't seem to reproduce this crash anymore. However with a try-server ASan build from decoder with changeset 9f3cc040e41a I can't reproduce the testcase. I've been trying to get my own builds set up but I've been running into technical hurdles that decoder has been helping me through. For the sake of time, given that we are days from release, would someone who can reproduce this please help with verification?

Comment 35

[u279076]

I managed to get my own try-server ASAN build built from http://hg.mozilla.org/mozilla-central/rev/9533b40ff28b as mentioned in comment 0 and am able to reproduce this bug now. Will now test to verify fixed.

Comment 36

[u279076]

Verified fixed with Firefox 16.0a2 2012-08-24 ASAN build. Marking [qa-] since I can't verify against Firefox 15 Beta.