The default bug view has changed. See this FAQ.

Crash [@ nsSubDocumentFrame::AttributeChanged] with throwing error and removing type attribute on iframe

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
DOM
--
critical
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: mats)

Tracking

(4 keywords)

1.9.2 Branch
mozilla2.0b8
crash, testcase, verified1.9.1, verified1.9.2
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking1.9.2 needed, status1.9.2 .14-fixed, blocking1.9.1 needed, status1.9.1 .17-fixed)

Details

(Whiteboard: [sg:dos null pointer access] "fixed" on trunk by disabling content XUL, crash signature)

Attachments

(4 attachments)

(Reporter)

Description

7 years ago
Created attachment 468196 [details]
testcase

See testcase, which crashes current trunk build and Firefox3.6.8 after a while (normally within 1 minute or so).

The content of the iframe is this:
<?xml-stylesheet href="chrome://browser/skin/" type="text/css"?>
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" xmlns:html="http://www.w3.org/1999/xhtml">
<iframe type="a"></iframe>

<script><![CDATA[
setTimeout(function() {
parent.document.getElementById('content2').contentDocument.documentElement.appendChild(document.documentElement);
parent.document.getElementById('content2').contentWindow.location.reload();
}, 100);

setInterval(function() {
for (var i=0;i < 6; i++) {throw('t');}
},30);

var all = document.getElementsByTagName('*');
var iframe = all[1];

function doe(node) {
node.removeAttributeNode(node.attributes[0]);
}

setTimeout(function() {
doe(iframe);
},150);

]]></script>
</window>

http://crash-stats.mozilla.com/report/index/bp-39e25f91-184e-4504-a829-4ae472100822
0  	xul.dll  	nsSubDocumentFrame::AttributeChanged  	 layout/generic/nsFrameFrame.cpp:788
1 	xul.dll 	nsStyleContext::DoGetStyleDisplay 	layout/style/nsStyleStructList.h:95
2 	mozcrt19.dll 	arena_dalloc_small 	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4153
3 	mozcrt19.dll 	arena_dalloc 	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4284
4 	xul.dll 	nsDOMAttribute::Release 	content/base/src/nsDOMAttribute.cpp:151
5 	xul.dll 	nsTHashtable<nsBaseHashtableET<nsAttrHashKey,nsRefPtr<nsDOMAttribute> > >::s_ClearEntry 	obj-firefox/dist/include/nsTHashtable.h:397
6 	xul.dll 	nsNodeUtils::AttributeChanged 	content/base/src/nsNodeUtils.cpp:128
7 	xul.dll 	nsXULElement::UnsetAttr 	content/xul/content/src/nsXULElement.cpp:1455
8 	xul.dll 	nsDOMAttributeMap::RemoveNamedItem 	content/base/src/nsDOMAttributeMap.cpp:390
9 	xul.dll 	nsGenericElement::RemoveAttributeNode 	content/base/src/nsGenericElement.cpp:2478
10 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
11 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:561
12 		@0x11d215b2 	
13 	xul.dll 	nsXPConnect::GetWrapperForObject 	js/src/xpconnect/src/nsXPConnect.cpp:2481
14 	xul.dll 	js_GetPropertyHelper 	js/src/jsobj.cpp:4830
15 	xul.dll 	XPC_WN_JSOp_ThisObject 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1533
Assignee: nobody → matspal

Comment 1

7 years ago
Stack trace looks corrupt, so I'm calling this [sg:critical].

I'm having trouble reproducing on Mac OS X 10.5.x with Firefox trunk.
OS: Windows 7 → Windows XP
Whiteboard: [sg:critical]

Comment 2

7 years ago
Couldn't reproduce using Valgrind (on Mac OS X 10.6.x) either :(
Marcia will try to reproduce this in the QA lab following the meeting.
I crashed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 (.NET CLR 3.5.30729) - http://crash-stats.mozilla.com/report/index/bp-8aa81493-005c-4667-8961-bb3992100921

My first attempt at loading the testcase on the trunk, it did not crash. Will try again.
Not able to crash with testcase using  Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100921 Firefox/4.0b7pre.
Martijn: Can you try your STR again please? I am unable to get a trunk crash.
Did this get fixed along the way with another patch?
(Reporter)

Comment 8

7 years ago
I'm still crashing in current trunk build.
Couldn't reproduce on linux
(Reporter)

Comment 10

7 years ago
It doesn't crash on trunk, because xul is disabled by default. You'll have to enable xul for this site (but that's kinda difficult right now, because xul is disabled without the option of turning it back on in about:config or the ui).
This probably isn't critical on trunk, but nominating for 1.9.2.
blocking1.9.2: --- → ?

Comment 12

7 years ago
Does this also affect 1.9.1?
blocking1.9.1: --- → ?
blocking1.9.2: ? → needed
status1.9.1: --- → ?
status1.9.2: --- → wanted
On 3.6.11pre I crash at the same location referencing the same 0x0 address as the stacks in comment 0 and comment 4, but my stack didn't look messed up like the other two: bp-04b15f88-f811-414f-b660-0afc72101008
Shiretoko has the same crash: bp-f745d95f-1f53-42f6-b356-5f2032101008
blocking1.9.1: ? → needed
status1.9.1: ? → wanted
Mats, are you actually working on this?  Since we can reproduce this on branch, we should just fix this.
(Assignee)

Comment 16

7 years ago
I'm busy on bug 571995.  Optimistically, I could get to this in a couple
of days.  When is the next branch release(s) due?
(Assignee)

Comment 17

7 years ago
There were a couple of regressions from bug 571995 (bug 604843, bug 605340)
which now has patches for review, so I'll look at this bug after that.
Turning this into a branch bug since we've disabled content XUL on trunk (at least, for non-whitelisted sites).
Whiteboard: [sg:critical] → [sg:critical] "fixed" on trunk by disabling content XUL
Version: Trunk → 1.9.2 Branch
Mats, got an eta here?
Mats, ping?
(Assignee)

Comment 21

6 years ago
Sorry for the delay.

I've tested this in debug builds on WinXP, OSX and Linux64 and AFAICT
it's a safe null pointer access.  Jesse reported in comment 1 that
he saw evidence of a damaged stack so I guess there could have been
another bug involved earlier that is now gone.  I'm lowering this to
sg:dos until we have new evidence indicating otherwise.
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [sg:critical] "fixed" on trunk by disabling content XUL → [sg:dos null pointer access] "fixed" on trunk by disabling content XUL
(Assignee)

Comment 22

6 years ago
Created attachment 495726 [details] [diff] [review]
Fix for 1.9.2 and 1.9.1

BTW, there's a comment that says the code is more or less the same as
in nsFrameLoader::EnsureDocShell.  I've checked that method and it
already has a corresponding null check.
http://mxr.mozilla.org/mozilla1.9.2/source/layout/generic/nsFrameFrame.cpp#703

I've stress tested this patch on WinXP, OSX and Linux64 debug 1.9.2 builds
and a Linux64 1.9.1 debug build.

Making a testcase that actually halts seems tricky -- I'll try make one
in a separate patch.
Attachment #495726 - Flags: review?(roc)
(Assignee)

Comment 23

6 years ago
Created attachment 495727 [details] [diff] [review]
Fix for trunk
(Assignee)

Comment 24

6 years ago
Created attachment 495736 [details] [diff] [review]
crashtest.diff
(Assignee)

Updated

6 years ago
Attachment #495727 - Flags: review?(roc)
Attachment #495727 - Flags: review?(roc) → review+
Attachment #495726 - Flags: review?(roc) → review+
(Assignee)

Updated

6 years ago
Attachment #495727 - Flags: approval2.0?
(Assignee)

Updated

6 years ago
Attachment #495726 - Flags: approval1.9.2.14?
Attachment #495726 - Flags: approval1.9.1.17?
Attachment #495727 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 25

6 years ago
http://hg.mozilla.org/mozilla-central/rev/06e68e399226
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Comment on attachment 495726 [details] [diff] [review]
Fix for 1.9.2 and 1.9.1

Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
Attachment #495726 - Flags: approval1.9.2.14?
Attachment #495726 - Flags: approval1.9.2.14+
Attachment #495726 - Flags: approval1.9.1.17?
Attachment #495726 - Flags: approval1.9.1.17+
Group: core-security
(Assignee)

Comment 27

6 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/51edb84f5830
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/168304e2c396
status1.9.1: wanted → .17-fixed
status1.9.2: wanted → .14-fixed
Verified for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.14pre) Gecko/20110104 Namoroka/3.6.14pre ( .NET CLR 3.5.30729) and the testcase. With 1.9.2.13, the testcase will crash after a minute or three. It no longer does so with the 1.9.2.14pre build.

On 1.9.1, I cannot get a crash with the testcase in 1.9.1.16. Have we seen the crash there?
Keywords: verified1.9.2
Whiteboard: [sg:dos null pointer access] "fixed" on trunk by disabling content XUL → [sg:dos null pointer access] "fixed" on trunk by disabling content XUL [qa-examined-191] [qa-needs-STR]
I was wrong. I was able to get the crash, it just took longer. I've run it in the nightly 1.9.1 build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.17pre) Gecko/20110104 Shiretoko/3.5.17pre ( .NET CLR 3.5.30729)) for a while now and it hasn't crashed so I'm calling this verified for 1.9.1.
Keywords: verified1.9.1
Whiteboard: [sg:dos null pointer access] "fixed" on trunk by disabling content XUL [qa-examined-191] [qa-needs-STR] → [sg:dos null pointer access] "fixed" on trunk by disabling content XUL
Crash Signature: [@ nsSubDocumentFrame::AttributeChanged]
You need to log in before you can comment on or make changes to this bug.