Closed Bug 856106 Opened 12 years ago Closed 11 years ago

crash in mozilla::FrameLayerBuilder::RemoveFrameFromLayerManager

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

RESOLVED INCOMPLETE
Tracking Status
firefox20 --- wontfix
firefox21 - wontfix
firefox22 - wontfix
firefox23 - wontfix
firefox24 - wontfix
firefox25 --- wontfix
firefox-esr17 - wontfix
b2g18 --- wontfix

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
Keywords: needURLs
"EXCEPTION_ACCESS_VIOLATION_WRITE" is even more evidence of exploitability.
Anyone: steps to reproduce outside of URLs in comment 2? Not much to go on.
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.
(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)
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)
We've got no leads so there's little reason to track this longstanding issue.
(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?
Actually, I'm pulling the tracking-b2g18 flag to get this off our radar.
tracking-b2g18: ? → ---
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
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 → ---
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.
Marking this as tracking - for 24 since we seem to be doing nothing with this issue.
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.
(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-criticalsec-high
Needinfo'ing to get answers to questions in comment 15.
Flags: needinfo?(matspal)
> 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)
Hi Matt, any update? (Do you think the poison mitigation idea from Mats should be next?)
Flags: needinfo?(matt.woodrow)
Closing as incomplete because it isn't actionable at this point.
Status: REOPENED → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → INCOMPLETE
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.
Flags: needinfo?(matt.woodrow)
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.