Closed
Bug 856106
Opened 12 years ago
Closed 11 years ago
crash in mozilla::FrameLayerBuilder::RemoveFrameFromLayerManager
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
People
(Reporter: MatsPalmgren_bugz, Assigned: mattwoodrow)
Details
(Keywords: crash, sec-high)
Crash Data
This bug was filed from the Socorro interface and is
report bp-4e5c0cbb-3bf5-4243-9283-e9daf2130329 .
=============================================================
Crash occurs on all platforms and in all versions from 18.0 to 22.0a1,
1795 reported incidents in the last 4 weeks.
The "Crash Address" is all over the place so it might be a security problem.
mozilla::FrameLayerBuilder::RemoveFrameFromLayerManager layout/base/FrameLayerBuilder.cpp:897
PresShell::NotifyDestroyingFrame layout/base/nsPresShell.cpp:2039
nsFrame::DestroyFrom layout/generic/nsFrame.cpp:652
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:302
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsContainerFrame::DestroyFrom layout/generic/nsContainerFrame.cpp:228
nsHTMLScrollFrame::DestroyFrom layout/generic/nsGfxScrollFrame.cpp:96
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:302
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:278
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:281
nsBlockFrame::DestroyFrom layout/generic/nsBlockFrame.cpp:302
nsContainerFrame::DestroyFrom layout/generic/nsContainerFrame.cpp:228
nsCanvasFrame::DestroyFrom layout/generic/nsCanvasFrame.cpp:56
nsContainerFrame::DestroyFrom layout/generic/nsContainerFrame.cpp:228
nsHTMLScrollFrame::DestroyFrom layout/generic/nsGfxScrollFrame.cpp:96
nsContainerFrame::DestroyFrom layout/generic/nsContainerFrame.cpp:228
nsFrameManager::Destroy layout/base/nsFrameManager.cpp:218
PresShell::Destroy layout/base/nsPresShell.cpp:1081
DocumentViewerImpl::DestroyPresShell layout/base/nsDocumentViewer.cpp:4375
DocumentViewerImpl::DestroyPresShell layout/base/nsDocumentViewer.cpp:4378
DocumentViewerImpl::Destroy layout/base/nsDocumentViewer.cpp:1664
Assignee: nobody → matt.woodrow
Comment 1•12 years ago
|
||
"EXCEPTION_ACCESS_VIOLATION_WRITE" is even more evidence of exploitability.
Keywords: sec-critical,
testcase-wanted
Comment 2•12 years ago
|
||
9 https://www.facebook.com/
7 http://www.facebook.com/
6 http://www.facebook.com/logout.php
5 https://www.facebook.com/logout.php
4 about:blank
4 about:newtab
3 https://www.facebook.com/?ref=tn_tnmn
2 https://www.facebook.com/index.php?l_s=r
2 https://apps.facebook.com/playhappyfarm/?fb_source=bookmark_apps&ref=bookmarks&count=0&fb_bmpos=2_0
Keywords: needURLs
Updated•12 years ago
|
status-firefox20:
--- → affected
status-firefox21:
--- → affected
status-firefox22:
--- → affected
status-firefox23:
--- → affected
tracking-firefox23:
--- → ?
Updated•12 years ago
|
status-b2g18:
--- → affected
status-firefox-esr17:
--- → affected
Comment 3•12 years ago
|
||
Anyone: steps to reproduce outside of URLs in comment 2? Not much to go on.
Updated•12 years ago
|
tracking-b2g18:
--- → +
tracking-firefox21:
--- → +
tracking-firefox22:
--- → +
tracking-firefox-esr17:
--- → ?
Comment 4•12 years ago
|
||
I'll track this because of security level, and having urls to at least try reproducing with but since it's been present as far back as 18.0 we'll un-track if this doesn't get forward movement in the next cycle.
Comment 5•12 years ago
|
||
(In reply to Matt Wobensmith from comment #3)
> Anyone: steps to reproduce outside of URLs in comment 2? Not much to go on.
I think that's what we're hoping you can help out with given URLs, but they're pretty typical.
mattwoodrow - any tips for paths to reproduce?
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 6•12 years ago
|
||
No, I've got no leads at all here.
I've tried to reproduce with the given URLs, and haven't been able to reproduce.
Flags: needinfo?(matt.woodrow)
Comment 7•12 years ago
|
||
We've got no leads so there's little reason to track this longstanding issue.
Updated•12 years ago
|
Comment 8•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #7)
> We've got no leads so there's little reason to track this longstanding issue.
Right. We probably should tracking- on b2g18 too, right?
Comment 9•12 years ago
|
||
Actually, I'm pulling the tracking-b2g18 flag to get this off our radar.
tracking-b2g18:
? → ---
Comment 10•12 years ago
|
||
At this point, it's a zombie bug with no reproducible steps that continues to haunt our queries, but without anything actionable. Closing for now.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INCOMPLETE
Reporter | ||
Comment 11•12 years ago
|
||
The current crash rate is about the same as when I filed this bug so the
bug is still there and we need to fix it.
I don't think we should close bugs and give up on them just because
there's no activity or plan yet for how to address them.
There's lots of things we can do here; we could add assertions, logging,
poisoning, crash annotations etc in code areas of interest. We could
also go back and review the code again in FrameLayerBuilder /
DisplayItemData that deals with frame pointers and see if we can make it
more robust. (I think the crash is caused by some ownership problem in
that code.)
We could also try harder to find steps to reproduce the crash; with an
average of 400+ crashes per week there must be more interesting URLs
than the ones listed so far (in comment 2). We could target those URLs
with fuzzing -- Bob Clary has a tool that does that, iirc.
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Comment 12•12 years ago
|
||
This was closed because it is an open sec-critical that shows up every time we look at sec-criticals (several meetings and independent things a week) that had no actionable work since no one knows what to do (we thought on reading all the comments). Critsmash triage decided to try to move it off since it just sits there. :-)
If there are things to do, since this is an open sec-crit, is Matt going to do these things soon? Given the high security rating, this should be important.
Comment 13•12 years ago
|
||
Marking this as tracking - for 24 since we seem to be doing nothing with this issue.
status-firefox24:
--- → affected
tracking-firefox24:
--- → -
Updated•12 years ago
|
status-firefox25:
--- → affected
Comment 14•12 years ago
|
||
I spelunked in crashstats for a bit.
So far I see only two crashes in 24.0a2 (aside: noticed both had testpilot installed - other pre-24 crashes do not though).
There are no consistent interesting URLs unfortunately.
Comment 15•12 years ago
|
||
(In reply to Mats Palmgren (on vacation) from comment #11)
...
>
> There's lots of things we can do here; we could add assertions, logging,
> poisoning, crash annotations etc in code areas of interest. We could
> also go back and review the code again in FrameLayerBuilder /
> DisplayItemData that deals with frame pointers and see if we can make it
> more robust. (I think the crash is caused by some ownership problem in
> that code.)
We looked at this bug again today and considered lowering it to sec-high due to lack of testcase/in-hand-exploitability but it worries us. Regardless we definitely think the ideas you suggest are worthwhile. Mats do you want to drive doing that? Perhaps on separate bugs (or are they filed already)?
Would bug 860254 help this case? (Still looking for a taker on that bug)
Keywords: sec-critical → sec-high
Comment 16•11 years ago
|
||
Needinfo'ing to get answers to questions in comment 15.
Flags: needinfo?(matspal)
Reporter | ||
Comment 17•11 years ago
|
||
> Mats do you want to drive doing that?
Sorry, I'll focus on implementing overflow:fragments for the near future (rest of 2013
at least), so I don't think I'll have large enough chunks of free time to drive this.
I'll be happy to help out with reviews or advice if anyone else wants to take it.
> Would bug 860254 help this case?
Sure. If it's a use-after-free then we'll see that in the crash addresses.
If it's not, then that's also useful information.
A smaller step than fixing bug 860254 might be to poison LayerManagerData.
Flags: needinfo?(matspal)
Comment 18•11 years ago
|
||
Hi Matt, any update? (Do you think the poison mitigation idea from Mats should be next?)
Flags: needinfo?(matt.woodrow)
Comment 19•11 years ago
|
||
Closing as incomplete because it isn't actionable at this point.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → INCOMPLETE
Comment 20•11 years ago
|
||
I don't see any crashes with RemoveFrameFromLayerManager in the top frame on the top 50 crashes, and Al said he didn't see any crashes at all in the last 30 days.
Updated•11 years ago
|
Flags: needinfo?(matt.woodrow)
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Keywords: testcase-wanted
Updated•9 years ago
|
Group: core-security-release
Updated•3 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•