Closed
Bug 344300
Opened 18 years ago
Closed 17 years ago
Crash [@ nsFrameItems::AddChild ] on 1.8.0.5 and 1.8.1 branch
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: martijn.martijn, Assigned: MatsPalmgren_bugz)
References
Details
(4 keywords, Whiteboard: [sg:nse null-deref])
Crash Data
Attachments
(3 files)
1.74 KB,
application/xhtml+xml
|
Details | |
111 bytes,
text/html
|
Details | |
1.51 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
dveditz
:
approval1.8.1.5+
dveditz
:
approval1.8.0.13+
|
Details | Diff | Splinter Review |
See upcoming testcase, which crashes current branches, it doesn't crash current trunk build. If desired, I can reduce the testcase. Talkback ID: TB20840965W nsFrameItems::AddChild [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 744] nsCSSFrameConstructor::ConstructFrameByDisplayType [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 6891] nsCSSFrameConstructor::CantRenderReplacedElement [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11028] CantRenderReplacedElementEvent::HandleEvent [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 4090] 0x778b0c24 0x00200064 0x0c718d08
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Comment 2•18 years ago
|
||
Ctrl++ to zoom the testcase seems to trigger the crash bug 343206 again on trunk.
Reporter | ||
Comment 3•18 years ago
|
||
A different testcase that also triggers this same crash in branch builds.
Comment 4•17 years ago
|
||
This looks like a safe null-deref on the branch and doesn't happen on the trunk. Any objection to removing the security-sensitive flag on this bug?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:nse null-deref]
Reporter | ||
Comment 6•17 years ago
|
||
(In reply to comment #4) > This looks like a safe null-deref on the branch and doesn't happen on the > trunk. Any objection to removing the security-sensitive flag on this bug? No, I don't have any objection to that (if you want to remove the security-sensitive flag, then I have no objection, in general).
Group: security
Assignee | ||
Comment 7•17 years ago
|
||
CantRenderReplacedElement() calls ConstructFrameByDisplayType() which only handles a limited set of cases since it's normally called at the end of ConstructFrameInternal() (after XUL frame creation an such). We have <object style="display: -moz-grid-line;"> which isn't handled so we fall into the UNREACHED at the end, since we haven't created a frame in this case we will call AddChild with a null frame. This is a 100% null-deref, so not a security problem. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=MOZILLA_1_8_BRANCH&root=/cvsroot&mark=6893,6905#6842
Assignee: nobody → mats.palmgren
OS: Windows XP → All
Hardware: PC → All
Assignee | ||
Comment 8•17 years ago
|
||
Attachment #263844 -
Flags: superreview?(bzbarsky)
Attachment #263844 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•17 years ago
|
Version: Trunk → 1.8 Branch
Reporter | ||
Comment 9•17 years ago
|
||
So maybe this should not be fixed on branch then? I mean, iirc, I hit crash a lot on branch. It might be that after this fix, I could hit something worse (not that I test that much on branch, unfortunately).
Assignee | ||
Comment 10•17 years ago
|
||
> It might be that after this fix, I could hit something worse
I don't think so. The patch results in the content in this case will have
no frame at all, which is a common and well tested state. I think the
risk of this patch enabling new and untested code paths is low.
OTOH, it's highly unlikely that any real web pages will trigger this crash
by accident...
Comment 12•17 years ago
|
||
Comment on attachment 263844 [details] [diff] [review] Patch rev. 1 Sure, I guess. I hate the branch. ;)
Attachment #263844 -
Flags: superreview?(bzbarsky)
Attachment #263844 -
Flags: superreview+
Attachment #263844 -
Flags: review?(bzbarsky)
Attachment #263844 -
Flags: review+
Assignee | ||
Updated•17 years ago
|
Attachment #263844 -
Flags: approval1.8.1.5?
Attachment #263844 -
Flags: approval1.8.0.13?
Comment 13•17 years ago
|
||
Comment on attachment 263844 [details] [diff] [review] Patch rev. 1 approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #263844 -
Flags: approval1.8.1.5?
Attachment #263844 -
Flags: approval1.8.1.5+
Attachment #263844 -
Flags: approval1.8.0.13?
Attachment #263844 -
Flags: approval1.8.0.13+
Assignee | ||
Comment 14•17 years ago
|
||
Checked in to MOZILLA_1_8_BRANCH: layout/base/nsCSSFrameConstructor.cpp: 1.1110.6.77 Checked in to MOZILLA_1_8_0_BRANCH: layout/base/nsCSSFrameConstructor.cpp: 1.1110.6.12.2.56
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite?
Keywords: fixed1.8.0.13,
fixed1.8.1.5
Resolution: --- → FIXED
Comment 15•17 years ago
|
||
verified fixed on 1.8.1.5 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.5pre) Gecko/20070705 BonEcho/2.0.0.5pre ID:2007070504 and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.5pre) Gecko/2007070503 BonEcho/2.0.0.5pre on Fedora F7 - no crash on testcases - adding verified keyword
Keywords: fixed1.8.1.5 → verified1.8.1.5
Comment 16•17 years ago
|
||
This is verified fixed on 1.8.0.13
Keywords: fixed1.8.0.13 → verified1.8.0.13
Comment 17•17 years ago
|
||
(In reply to comment #16) > This is verified fixed on 1.8.0.13 > Sorry, this was on Linux. I should have specified this.
Comment 18•15 years ago
|
||
crash test landed http://hg.mozilla.org/mozilla-central/rev/b480ed41fe22
Flags: in-testsuite? → in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ nsFrameItems::AddChild ]
You need to log in
before you can comment on or make changes to this bug.
Description
•