Crash [@ nsStyleContext::GetRuleNode()] with deleted this

RESOLVED FIXED in Firefox 17

Status

()

defect
--
critical
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: bc, Assigned: roc)

Tracking

(Blocks 1 bug, {crash, reproducible, sec-critical})

Trunk
mozilla18
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox16- affected, firefox17+ fixed, firefox18+ fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [adv-track-main17+][adv-track-esr17-], crash signature, )

Attachments

(1 attachment)

Reporter

Description

7 years ago
1. http://altfoto.com/2012/08/el-sucio-sucio-secreto-de-las-grandes-marcas-y-sus-sensores-defectuosos

2. Crash nsStyleContext::GetRuleNode() nsIFrame::PresContext() nsRootPresContext::RequestUpdatePluginGeometry(nsIFrame*) PresShell::DoReflow(nsIFrame*, bool) PresShell::ProcessReflowCommands(bool)

Nightly Linux 32bit and Windows 7 64bit

See bug 781279 comment 15

crash address in Windows when you don't get a Flash related crash is 0xffffffffddddddf9 while in Linux it is 0xdadadaf6

On Linux I see nsStyleContext::GetRuleNode (this=0xdadadada) at ../../dist/include/nsStyleContext.h:190

bp-e7968d4a-42bb-4df7-98ef-920df2120830

Not very reproducible in opt build but pretty much so in debug.
This looks like a crash in nsRootPresContext::RequestUpdatePluginGeometry
doing mUpdatePluginGeometryForFrame->PresContext() on a deleted frame.
If so, I think it's mitigated by frame-poisoning.

It could be the same underlying problem as top-crash bug 785808.
Whiteboard: [mitigated by frame-poisoning]
On 32-bit Nightly: bp-69bfff59-550b-46bc-82d6-0cae32120902.
On 64-bit Nightly: bp-0200cf0c-f76c-4e8f-b271-0e4d82120902.
Blocks: 785808
Crash Signature: [@ nsStyleContext::GetRuleNode()] [@ nsRootPresContext::RequestUpdatePluginGeometry] → [@ nsStyleContext::GetRuleNode()] [@ nsRootPresContext::RequestUpdatePluginGeometry] [@ mozalloc_abort(char const* const) | NS_DebugBreak_P | nsIFrame::GetOffsetToCrossDoc(nsIFrame const* int)] [@ mozalloc_abort(char const* const) | NS_DebugBreak_P | n…
Keywords: reproducible
I cannot reproduce this.
Nor I, on Linux x64 or Windows 7 x64, and not on WinXP in VirtualBox.
I don't think this is worth having.
Assignee: nobody → roc
Attachment #657970 - Flags: review?(matspal)
bclary, if you can reproduce this, it would be helpful if you can determine whether the above patch fixes it. Thanks!
I can't reproduce in a debug or asan build.  After a lot a stress testing --
opening the URL in 30 or so tabs, reload-all-tabs, closing/reloading tabs,
all while resizing the window, I finally got one crash in a Nightly Linux64:
bp-e2ef7c12-29f6-44da-b6d9-e9d952120904
Attachment #657970 - Flags: review?(matspal) → review+
Reporter

Comment 8

7 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> bclary, if you can reproduce this, it would be helpful if you can determine
> whether the above patch fixes it. Thanks!

I applied it to Linux Nightly and was no longer able to reproduce it there on any of the 182 urls where was originally detected. The unpatched Windows 7 builds on Nightly and Aurora and the unpatched builds on Linux Aurora were able to reproduce the original crash on a few of the urls. This crash has a very low reproducibility rate, but given the data I have I agree this patch fixes the issue at hand.
https://hg.mozilla.org/mozilla-central/rev/df1a1a1cfaf4
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(In reply to Mats Palmgren [:mats] from comment #1)
> If so, I think it's mitigated by frame-poisoning.

The addresses in comment 0 don't look very mitigated, those are deleted memory pointers. Frame-poisoned addresses are in a different range. changing to sec-critical unless I'm missing something.

If it is sec-critical we should fix on older branches, especially if live web-content is triggering this crash which might increase the odds of discovery.
Keywords: sec-othersec-critical
Whiteboard: [mitigated by frame-poisoning]
I think most of these crashes are regression from bug 775965, which is only in FF17 and up. So I'll go for Aurora but not other branches at this time.
Comment on attachment 657970 [details] [diff] [review]
rip out mUpdatePluginGeometryForFrame optimization

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

Removes fragile optimization, so should be safe.
Attachment #657970 - Flags: approval-mozilla-aurora?
Comment on attachment 657970 [details] [diff] [review]
rip out mUpdatePluginGeometryForFrame optimization

Approving for Aurora - does this mean that we should untrack/mark unaffected for 16?
Attachment #657970 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Lukas Blakk [:lsblakk] from comment #14)
> Approving for Aurora - does this mean that we should untrack/mark unaffected
> for 16?

Probably, but you might want to confirm my guess that this bug doesn't happen (or happens at much lower rate) on 16.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> Probably, but you might want to confirm my guess that this bug doesn't
> happen (or happens at much lower rate) on 16.
This crash signature is #13 top browser crasher in 16.0b2 (partially tracked by bug 754380) but I don't know if it's related to this bug.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13)
> Comment on attachment 657970 [details] [diff] [review]
> rip out mUpdatePluginGeometryForFrame optimization
> 
> Review of attachment 657970 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Removes fragile optimization, so should be safe.

Do we know what the regressing bug was here?
I see comment 12 now. Sounds like this wouldn't help bug 754380 in that case, unless we find other related regressing bugs.
(In reply to Scoobidiver from comment #16)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> > Probably, but you might want to confirm my guess that this bug doesn't
> > happen (or happens at much lower rate) on 16.
> This crash signature is #13 top browser crasher in 16.0b2 (partially tracked
> by bug 754380) but I don't know if it's related to this bug.

I'm not seeing the crash signatures here, or the one listed in bug 754380, at high volume in b3. I don't believe we need to track for that version any longer. Please renominate if you disagree.
QA Contact: jbecerra
Please prepare a patch for the mozilla-esr10 branch the next opportunity you get. Thanks!
Any update on an ESR patch for this? It has been weeks and this is a sec-critical.
Sorry, I lost track of the request in comment #21 for some reason.

I don't think this needs to be fixed on esr10 because the crashes were primarily caused by bug 775965 which never landed on esr10.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> Sorry, I lost track of the request in comment #21 for some reason.
> 
> I don't think this needs to be fixed on esr10 because the crashes were
> primarily caused by bug 775965 which never landed on esr10.

Thanks roc, marking as unaffected on ESR10 given that.
Whiteboard: [adv-track-main17+]
Whiteboard: [adv-track-main17+] → [adv-track-main17+][adv-track-esr17-]

Updated

7 years ago
Summary: Crash [@ nsStyleContext::GetRuleNode()} with deleted this → Crash [@ nsStyleContext::GetRuleNode()] with deleted this
Group: core-security
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.