Closed
Bug 1459670
Opened 7 years ago
Closed 6 years ago
use-after-poison in [@ AnyContentAncestorModified]
Categories
(Core :: Web Painting, defect)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | + | fixed |
firefox62 | + | fixed |
People
(Reporter: tsmith, Assigned: mattwoodrow)
References
(Blocks 2 open bugs)
Details
(4 keywords)
Crash Data
Attachments
(3 files, 1 obsolete file)
(deleted),
application/x-javascript
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
mikokm
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Found with m-c:
BuildID=20180507140941
SourceStamp=93443d36d4bd53dba004f7b73430879f96daa681
==4110==ERROR: AddressSanitizer: use-after-poison on address 0x625000332a56 at pc 0x7f7d46f3ede0 bp 0x7ffc1d8a6390 sp 0x7ffc1d8a6388
READ of size 2 at 0x625000332a56 thread T0 (file:// Content)
#0 0x7f7d46f3eddf in IsFrameModified src/layout/generic/nsIFrame.h:4135:35
#1 0x7f7d46f3eddf in AnyContentAncestorModified src/layout/painting/RetainedDisplayListBuilder.cpp:193
#2 0x7f7d46f3eddf in HasModifiedFrame src/layout/painting/RetainedDisplayListBuilder.cpp:332
#3 0x7f7d46f3eddf in MergeState::ProcessOldNode(Index<OldListUnits>, nsTArray<Index<MergedListUnits> >&&) src/layout/painting/RetainedDisplayListBuilder.cpp:362
#4 0x7f7d46e37838 in MergeState::Finalize() src/layout/painting/RetainedDisplayListBuilder.cpp:308:7
#5 0x7f7d46e35ea2 in RetainedDisplayListBuilder::MergeDisplayLists(nsDisplayList*, RetainedDisplayList*, RetainedDisplayList*, mozilla::Maybe<mozilla::ActiveScrolledRoot const*>&) src/layout/painting/RetainedDisplayListBuilder.cpp:490:26
#6 0x7f7d46e3dccc in RetainedDisplayListBuilder::AttemptPartialUpdate(unsigned int, mozilla::DisplayListChecker*) src/layout/painting/RetainedDisplayListBuilder.cpp:1097:7
#7 0x7f7d465fd93b in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) src/layout/base/nsLayoutUtils.cpp:3679:40
#8 0x7f7d464f0305 in mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) src/layout/base/PresShell.cpp:6351:5
#9 0x7f7d45e7ea9a in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) src/view/nsViewManager.cpp:480:19
#10 0x7f7d45e7d89c in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) src/view/nsViewManager.cpp:412:33
#11 0x7f7d45e82ef6 in nsViewManager::ProcessPendingUpdates() src/view/nsViewManager.cpp:1102:5
#12 0x7f7d464679d4 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2067:11
#13 0x7f7d46474f60 in TickDriver src/layout/base/nsRefreshDriver.cpp:337:13
#14 0x7f7d46474f60 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:307
#15 0x7f7d46474b26 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:329:5
#16 0x7f7d4647789e in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:770:5
#17 0x7f7d4647789e in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:683
#18 0x7f7d4647749e in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:584:9
#19 0x7f7d46d1f0bf in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16
#20 0x7f7d3faaedc4 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
#21 0x7f7d3f986cd3 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1988:28
#22 0x7f7d3f4f6b6e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2141:25
#23 0x7f7d3f4f3b36 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2071:17
#24 0x7f7d3f4f52ec in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1917:5
#25 0x7f7d3f4f5948 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1950:15
#26 0x7f7d3e604ec9 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14
#27 0x7f7d3e620900 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#28 0x7f7d3f4fe6c6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:125:5
#29 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
#30 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
#31 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
#32 0x7f7d45f0c57a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
#33 0x7f7d4a180f7b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#34 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
#35 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
#36 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
#37 0x7f7d4a180940 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34
#38 0x4f1875 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#39 0x4f1875 in main src/browser/app/nsBrowserApp.cpp:280
#40 0x7f7d5dd8a82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#41 0x420f48 in _start (firefox+0x420f48)
0x625000332a56 is located 4438 bytes inside of 8192-byte region [0x625000331900,0x625000333900)
allocated by thread T0 (file:// Content) here:
#0 0x4c1c93 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
#1 0x7f7d3e5b3ec3 in AllocateChunk src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:193:15
#2 0x7f7d3e5b3ec3 in InternalAllocate src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:228
#3 0x7f7d3e5b3ec3 in Allocate src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:75
#4 0x7f7d3e5b3ec3 in mozilla::ArenaAllocator<8192ul, 8ul>::Allocate(unsigned long) src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:80
#5 0x7f7d466d0ecf in AllocateByFrameID src/layout/base/nsPresArena.h:39:12
#6 0x7f7d466d0ecf in AllocateFrame src/obj-firefox/dist/include/nsIPresShell.h:208
#7 0x7f7d466d0ecf in operator new src/layout/generic/ViewportFrame.cpp:34
#8 0x7f7d466d0ecf in NS_NewViewportFrame(nsIPresShell*, mozilla::ComputedStyle*) src/layout/generic/ViewportFrame.cpp:31
#9 0x7f7d46560eca in nsCSSFrameConstructor::ConstructRootFrame() src/layout/base/nsCSSFrameConstructor.cpp:2705:5
#10 0x7f7d464bb554 in mozilla::PresShell::Initialize() src/layout/base/PresShell.cpp:1800:36
#11 0x7f7d416c1172 in nsContentSink::StartLayout(bool) src/dom/base/nsContentSink.cpp:1270:26
#12 0x7f7d405a2f36 in nsHtml5TreeOpExecutor::StartLayout(bool*) src/parser/html/nsHtml5TreeOpExecutor.cpp:674:18
#13 0x7f7d4059e48b in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) src/parser/html/nsHtml5TreeOperation.cpp:1199:17
#14 0x7f7d4059b416 in nsHtml5TreeOpExecutor::RunFlushLoop() src/parser/html/nsHtml5TreeOpExecutor.cpp:490:17
#15 0x7f7d405a74cb in nsHtml5ExecutorFlusher::Run() src/parser/html/nsHtml5StreamParser.cpp:121:18
#16 0x7f7d3e5e60d1 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32
#17 0x7f7d3e604ec9 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14
#18 0x7f7d3e620900 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#19 0x7f7d3f4fe6da in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#20 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
#21 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
#22 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
#23 0x7f7d45f0c57a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
#24 0x7f7d4a180f7b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#25 0x7f7d3f452f89 in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
#26 0x7f7d3f452f89 in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
#27 0x7f7d3f452f89 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
#28 0x7f7d4a180940 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34
#29 0x4f1875 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#30 0x4f1875 in main src/browser/app/nsBrowserApp.cpp:280
#31 0x7f7d5dd8a82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
Flags: in-testsuite?
Reporter | ||
Comment 1•7 years ago
|
||
Helps with repro
Reporter | ||
Comment 2•7 years ago
|
||
This also only seems to repro with GNOME_ACCESSIBILITY=1
Comment 3•7 years ago
|
||
Calling it sec-high out of caution. Can't tell from just this stack whether it's a simple safely frame-poisoned frame, or if the display list is pointing at something that isn't a frame at all. Should be easy to tell in a debugger though and then we can downgrade the severity if it's not that bad.
Keywords: sec-high
Comment 4•6 years ago
|
||
Matt, have you had a chance to look at this further?
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 5•6 years ago
|
||
I can't get this one to reproduce unfortunately.
Flags: needinfo?(matt.woodrow)
Reporter | ||
Comment 6•6 years ago
|
||
Give this a shot. I could reproduce it consistently with testcase but it did take up to a minute sometimes.
I was using m-c:
BuildID=20180529095249
SourceStamp=f01bb6245db1ea2a87e5360104a4110571265137
Attachment #8973746 -
Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 7•6 years ago
|
||
Perfect, thanks!
Got a good understanding of this now, will try get a patch up before bed.
Flags: needinfo?(matt.woodrow)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → matt.woodrow
Assignee | ||
Updated•6 years ago
|
tracking-firefox61:
--- → ?
tracking-firefox62:
--- → ?
Assignee | ||
Comment 8•6 years ago
|
||
Basically the same as bug 1452464, just handling the table frame pointers.
Attachment #8982148 -
Flags: review?(mikokm)
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #3)
> Calling it sec-high out of caution. Can't tell from just this stack whether
> it's a simple safely frame-poisoned frame, or if the display list is
> pointing at something that isn't a frame at all. Should be easy to tell in a
> debugger though and then we can downgrade the severity if it's not that bad.
It is indeed a guaranteed to be a frame pointer that's UAF, so should always be poisoned (or reused for another frame of the exact same type). I think the sec rating can be lowered based on that.
Flags: needinfo?(dveditz)
Comment 10•6 years ago
|
||
Comment on attachment 8982148 [details] [diff] [review]
table-frame-lifetimes
Review of attachment 8982148 [details] [diff] [review]:
-----------------------------------------------------------------
This pattern with multiple invalidation frames seems to be repeating in quite a few places. Maybe we could refactor this later to use SmallPointerArray instead.
Attachment #8982148 -
Flags: review?(mikokm) → review+
Updated•6 years ago
|
Group: layout-core-security
tracking-firefox61:
? → ---
tracking-firefox62:
? → ---
Flags: needinfo?(dveditz)
Comment 11•6 years ago
|
||
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a768ef5b02e
Add frame tracking for the nsDisplayTable* classes. r=miko
Updated•6 years ago
|
status-firefox60:
--- → unaffected
status-firefox62:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
tracking-firefox61:
--- → +
tracking-firefox62:
--- → +
Comment 12•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment 13•6 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(matt.woodrow)
Flags: in-testsuite?
Flags: in-testsuite-
Assignee | ||
Comment 14•6 years ago
|
||
Comment on attachment 8982148 [details] [diff] [review]
table-frame-lifetimes
Approval Request Comment
[Feature/Bug causing the regression]: Retained-dl
[User impact if declined]: Possible crashes/UAF from table mutations.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Tested with the fuzzer test, can't reproduce the issue.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Hooks up the extra nsIFrame references to the frame tracking infrastructure that we've used without issue for similar bugs in the past.
[String changes made/needed]: None.
Flags: needinfo?(matt.woodrow)
Attachment #8982148 -
Flags: approval-mozilla-beta?
Comment 15•6 years ago
|
||
Comment on attachment 8982148 [details] [diff] [review]
table-frame-lifetimes
RDL crash fix. Approved for 61.0b12.
Attachment #8982148 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 16•6 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•