830138 - Heap-use-after-free in nsFrameSelection::cycleCollection::TraverseImpl

Status: RESOLVED DUPLICATE of bug 830192

(Core :: Layout, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Reproduces on trunk. Load the testcase, wait 1-2 sec and click reload.

>==15820== ERROR: AddressSanitizer: heap-use-after-free on address 0x7f3bc53d0488 at pc 0x7f3be9843f28 bp 0x7fff10c57c00 sp 0x7fff10c57bf8
>READ of size 8 at 0x7f3bc53d0488 thread T0
>    #0 0x7f3be9843f27 in GetDocument src/../../dist/include/nsIPresShell.h:273
>    #1 0x7f3be9843f27 in nsFrameSelection::cycleCollection::TraverseImpl(nsFrameSelection::cycleCollection*, void*, nsCycleCollectionTraversalCallback&) src/layout/generic/nsSelection.cpp:514
>0x7f3bc53d0488 is located 8 bytes inside of 448-byte region [0x7f3bc53d0480,0x7f3bc53d0640)
>freed by thread T0 here:
>    #0 0x40f992 in __interceptor_free
>    #1 0x7f3be963c012 in PresShell::Release() src/layout/base/nsPresShell.cpp:750
>    #2 0x7f3be95d3188 in nsDocumentViewer::Destroy() src/layout/base/nsDocumentViewer.cpp:1628
>    #3 0x7f3be95d6787 in nsDocumentViewer::Show() src/layout/base/nsDocumentViewer.cpp:1934
>    #4 0x7f3be9631e37 in nsPresContext::EnsureVisible() src/layout/base/nsPresContext.cpp:1831
>    #5 0x7f3be9656d04 in PresShell::UnsuppressAndInvalidate() src/layout/base/nsPresShell.cpp:3578
>    #6 0x7f3beb69851f in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, tag_nsresult) src/docshell/base/nsDocShell.cpp:6525
>    #7 0x7f3beb695808 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) src/docshell/base/nsDocShell.cpp:6353
>    #8 0x7f3beb695c9c in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) src/docshell/base/nsDocShell.cpp:6360
>    #9 0x7f3beb6f4b72 in nsDocLoader::doStopDocumentLoad(nsIRequest*, tag_nsresult) src/uriloader/base/nsDocLoader.cpp:885
>    #10 0x7f3beb6f2c6c in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:775
>    #11 0x7f3beb6f3fdb in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) src/uriloader/base/nsDocLoader.cpp:659
>    #12 0x7f3beb6f47a9 in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) src/uriloader/base/nsDocLoader.cpp:663
>    #13 0x7f3be9dabcc5 in nsDocument::DoUnblockOnload() src/content/base/src/nsDocument.cpp:7658
>    #14 0x7f3bea6854e6 in nsBindingManager::DoProcessAttachedQueue() src/content/xbl/src/nsBindingManager.cpp:986
>    #15 0x7f3bec848472 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:238
>    #16 0x7f3bec10fe8c in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
>    #17 0x7f3bec99f8d8 in RunInternal src/ipc/chromium/src/base/message_loop.cc:215
>    #18 0x7f3bec99f8d8 in RunHandler src/ipc/chromium/src/base/message_loop.cc:208
>    #19 0x7f3bec99f8d8 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
>    #20 0x7f3bebdf88ed in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
>    #21 0x7f3be8d9021f in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
>    #22 0x7f3be8d911e1 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4093
>    #23 0x41afd6 in do_main src/browser/app/nsBrowserApp.cpp:195
>    #24 0x41afd6 in main src/browser/app/nsBrowserApp.cpp:388
>    #25 0x7f3bf391a76c in
>previously allocated by thread T0 here:
>    #0 0x40fa72 in malloc
>    #1 0x7f3bf0bd9148 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:54
>    #2 0x7f3be95c9e7e in nsDocumentViewer::InitPresentationStuff(bool) src/layout/base/nsDocumentViewer.cpp:702
>    #3 0x7f3be95c97ce in nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, nsIntRect const&, bool, bool, bool) src/layout/base/nsDocumentViewer.cpp:949
>    #4 0x7f3be95c87b0 in nsDocumentViewer::Init(nsIWidget*, nsIntRect const&) src/layout/base/nsDocumentViewer.cpp:683
>    #5 0x7f3beb69362f in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) src/docshell/base/nsDocShell.cpp:6170
>    #6 0x7f3beb6a58f7 in nsDocShell::CreateContentViewer(char const*, nsIRequest*, nsIStreamListener**) src/docshell/base/nsDocShell.cpp:7900
>    #7 0x7f3beb6d9a5b in nsDSURIContentListener::DoContent(char const*, bool, nsIRequest*, nsIStreamListener**, bool*) src/docshell/base/nsDSURIContentListener.cpp:122
>    #8 0x7f3beb6ec093 in nsDocumentOpenInfo::TryContentListener(nsIURIContentListener*, nsIChannel*) src/uriloader/base/nsURILoader.cpp:658
>    #9 0x7f3beb6e9e3e in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) src/uriloader/base/nsURILoader.cpp:360
>    #10 0x7f3beb6e95b1 in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) src/uriloader/base/nsURILoader.cpp:252
>    #11 0x7f3be8dd9d1f in nsBaseChannel::OnStartRequest(nsIRequest*, nsISupports*) src/netwerk/base/src/nsBaseChannel.cpp:720
>Shadow bytes around the buggy address:
>  0x1fe778a7a040: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a050: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a060: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fe778a7a080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>=>0x1fe778a7a090: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a0a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a0b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a0c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x1fe778a7a0d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fe778a7a0e0: 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
>  Heap righ redzone:     fb
>  Freed Heap region:     fd
>  Stack left redzone:    f1
>  Stack mid redzone:     f2
>  Stack right redzone:   f3
>  Stack partial redzone: f4
>  Stack after return:    f5
>  Stack use after scope: f8
>  Global redzone:        f9
>  Global init order:     f6
>  Poisoned by user:      f7
>  ASan internal:         fe
>Stats: 287M malloced (312M for red zones) by 486072 calls
>Stats: 50M realloced by 28055 calls
>Stats: 243M freed by 333270 calls
>Stats: 108M really freed by 234767 calls
>Stats: 504M (504M-0M) mmaped; 126 maps, 0 unmaps
>  mmaps   by size class: 8:327660; 9:32764; 10:12285; 11:16376; 12:3072; 13:2048; 14:1280; 15:384; 16:1408; 17:1280; 18:48; 19:40; 20:24;
>  mallocs by size class: 8:404641; 9:38664; 10:10893; 11:20755; 12:3248; 13:2155; 14:1829; 15:505; 16:1771; 17:1474; 18:71; 19:41; 20:25;
>  frees   by size class: 8:272632; 9:27336; 10:6634; 11:18257; 12:1922; 13:1602; 14:1625; 15:364; 16:1328; 17:1453; 18:58; 19:38; 20:21;
>  rfrees  by size class: 8:204661; 9:11416; 10:2634; 11:12111; 12:814; 13:667; 14:700; 15:197; 16:1104; 17:432; 18:26; 19:4; 20:1;
>Stats: malloc large: 1611 small slow: 2893
>Stats: StackDepot: 0 ids; 0M mapped
>==15820== ABORTING
>
>
>

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Debug build gives suspicious assertions. Are we not deleting all the frames.
Feels very similar to Bug 816359.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Olli Pettay [:smaug] from comment #1)
> Are we not deleting all the frames.
Based on assertions, that is the case.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Mats can you find an assignee?

Comment 4

[Mats Palmgren (inactive)]

Trevor, you worked on this code recently, can you take this bug?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Did you really mean Trevor? This isn't cycle collection or a11y bug, but some layout bug (nsIFrame object not destroyed).

Comment 6

[Mats Palmgren (inactive)]

He did some ref-counting cleanup in bug 828138 so I figured he had the
Selection code fresh in his mind (I'm not saying this is a regression from
that bug of course).  I got the impression this bug is a ownership/lifetime
problem in the Selection/Range code, but I haven't looked deeply ...

(looks at testcase) ... ah, right, the test has -moz-transform and position:fixed
in it so I guess it's something similar to bug 816359 then...

roc, can you take this?

Comment 7

[Trevor Saunders (:tbsaunde)]

(In reply to Mats Palmgren [:mats] from comment #6)
> He did some ref-counting cleanup in bug 828138 so I figured he had the
> Selection code fresh in his mind (I'm not saying this is a regression from
> that bug of course).  I got the impression this bug is a ownership/lifetime
> problem in the Selection/Range code, but I haven't looked deeply ...

Saddly I can't really claim to understand the life cycle of this stuff especially nsFrameSelection and how two different things can own it.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Patch in bug 830192 fixes this bug.

Comment 9

[Jason Smith [:jsmith]]

Roc - Do we need to block on this? Do we need the patch to fix this bug on b2g18? Thoughts?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I believe this is a regression from bug 827577, which has not (and will not) land for b2g18.

Comment 12

[Mats Palmgren (inactive)]

Testcase was added in:
https://hg.mozilla.org/mozilla-central/rev/4856e2c22f35