Last Comment Bug 332971 - crash with iframe removing itself [@ nsAttrAndChildArray::Clear] [@ nsAttrAndChildArray::~nsAttrAndChildArray]
: crash with iframe removing itself [@ nsAttrAndChildArray::Clear] [@ nsAttrAnd...
Status: RESOLVED FIXED
[sg:critical] [patch]
: crash, fixed1.8.1, verified1.8.0.4
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.9alpha1
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-06 03:02 PDT by georgi - hopefully not receiving bugspam
Modified: 2010-09-17 13:36 PDT (History)
9 users (show)
bzbarsky: blocking1.7.14?
bzbarsky: blocking‑aviary1.0.9?
darin.moz: blocking1.8.0.4+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
crash (653 bytes, text/html)
2006-04-06 03:02 PDT, georgi - hopefully not receiving bugspam
no flags Details
Fix (1.45 KB, patch)
2006-04-07 16:24 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
jst: review+
Details | Diff | Review
With that change (2.74 KB, patch)
2006-04-07 16:41 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
mrbkap: review+
jonas: superreview+
bzbarsky: approval‑branch‑1.8.1+
dveditz: approval1.8.0.4+
Details | Diff | Review

Description georgi - hopefully not receiving bugspam 2006-04-06 03:02:15 PDT
an iframe removing itself from the parent via javascript: uri causes memory
corruption after 2-3 reloads that may be automated.

on firefox $eip is often 0x0, on seamonkey there are clear signs of memory
corruption.

both trunk and branch.
Comment 1 georgi - hopefully not receiving bugspam 2006-04-06 03:02:53 PDT
Created attachment 217404 [details]
crash
Comment 2 georgi - hopefully not receiving bugspam 2006-04-06 03:07:25 PDT
stack

#0  0x0000000000865fb8 in ?? ()
#1  0x00002aaab0e8e6ad in nsAttrAndChildArray::Clear (this=0x1330ad0)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsAttrAndChildArray.cpp:647
#2  0x00002aaab0e8e713 in ~nsAttrAndChildArray (this=0x1330ad0)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsAttrAndChildArray.cpp:133
#3  0x00002aaab0ef3ba0 in ~nsGenericElement (this=0x1330aa0)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsGenericElement.cpp:870
#4  0x00002aaab0f72827 in nsGenericHTMLElement::~nsGenericHTMLElement$base ()
    at ../../../../dist/include/dom/nsIDOMElement.h:24
#5  0x00002aaab0f81c10 in ~nsHTMLBodyElement (this=0x1330aa0)
    at /opt/joro/suite-cvs/mozilla/content/html/content/src/nsHTMLBodyElement.cpp:294
#6  0x00002aaab0eed76d in nsGenericElement::Release (this=0x1330aa0)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsGenericElement.cpp:3141
#7  0x00002aaab0f81c6f in nsHTMLBodyElement::Release (this=0x1330aa0)
    at /opt/joro/suite-cvs/mozilla/content/html/content/src/nsHTMLBodyElement.cpp:298
#8  0x00002aaab0e8e6c1 in nsAttrAndChildArray::Clear (this=0x1316f90)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsAttrAndChildArray.cpp:648
#9  0x00002aaab0e8e713 in ~nsAttrAndChildArray (this=0x1316f90)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsAttrAndChildArray.cpp:133
#10 0x00002aaab0ef3ba0 in ~nsGenericElement (this=0x1316f60)
    at /opt/joro/suite-cvs/mozilla/content/base/src/nsGenericElement.cpp:870
Comment 3 Jesse Ruderman 2006-04-06 03:52:11 PDT
Crashes with a scary stack trace on Mac too.  Top of the stack:

Thread 0 Crashed:
0   <<00000000>> 	0xc0000000 0 + -1073741824
1   org.mozilla.firefox      	0x005693a8 nsAttrAndChildArray::~nsAttrAndChildArray [unified]() + 36
Comment 4 georgi - hopefully not receiving bugspam 2006-04-06 04:09:39 PDT
sure, the seamonkey stack is scary on linux
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-04-07 09:59:23 PDT
sicking, any idea what could be up here?  We're running the JS from under BindToTree, sadly, but we should be handling this particular setup right, I'd think....

roc, we should try to figure out how to plug this case into your new setup.  :(
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-04-07 15:43:54 PDT
mrbkap has a patch
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-04-07 15:46:43 PDT
The problem was an extra release in the content sink caused by SinkContext::OpenContainer incrementing mStackPos much later than where it stuck things on the stack -- the parent.document.write in the iframe appends to the parent between those two points (inside the AppendChildTo call), which messes up the sink context's stack.
Comment 8 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 16:24:59 PDT
Created attachment 217628 [details] [diff] [review]
Fix

This fixes the problem, as dbaron said.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2006-04-07 16:29:24 PDT
Comment on attachment 217628 [details] [diff] [review]
Fix

r=jst
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-04-07 16:30:03 PDT
I have to say, it would be nice if...

  if (mStackPos <= 0) {
    NS_ERROR("container w/o parent");

    return NS_ERROR_FAILURE;
  }

... were at the beginning of the function, before we potentially create and leak a node.

It would also be nice if the ++mStackPos were really *right* next to the code where we fill in mStack[mStackPos].  (I don't care whether the latter moves up or the former moves down, but it might matter.)
Comment 11 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 16:41:33 PDT
Created attachment 217630 [details] [diff] [review]
With that change

I'm convinced.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2006-04-07 16:58:01 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

Yeah, looks better.
Comment 13 Jonas Sicking (:sicking) PTO Until July 5th 2006-04-07 16:58:04 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

>   nsresult rv;
>   if (mStackPos + 1 > mStackSize) {
>     rv = GrowStack();
>     if (NS_FAILED(rv)) {
>       return rv;
>     }
>   }
> 
>+  if (mStackPos <= 0) {
>+    NS_ERROR("container w/o parent");
>+
>+    return NS_ERROR_FAILURE;
>+  }

But switch these two checks around.
Comment 14 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 17:32:32 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

jst reviewed this.
Comment 15 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 17:32:48 PDT
Fix checked into trunk.
Comment 16 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 17:33:31 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

This fixes a use of free'd memory.
Comment 17 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-04-07 17:34:46 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

Taking some liberties here... ;)
Comment 18 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-07 17:58:58 PDT
Fix checked into the 1.8 branch.
Comment 19 georgi - hopefully not receiving bugspam 2006-04-10 23:50:37 PDT
seems fixed in today trunk.

but a very similar abuse from xbl:
Bug 332807
xul iframe doing parent.document.open() - open this
https://bugzilla.mozilla.org/attachment.cgi?id=217441

still works.
Comment 20 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-11 02:14:15 PDT
(In reply to comment #19)
> but a very similar abuse from xbl:

This was a bug in the HTML content sink code, and the fix was also in the HTML content sink code. Therefore, abuses from XBL would be unaffected by such a fix :-(.
Comment 21 Daniel Veditz [:dveditz] 2006-04-17 12:08:11 PDT
Comment on attachment 217630 [details] [diff] [review]
With that change

approved for 1.8.0 branch, a=dveditz for drivers
Comment 22 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-04-21 14:42:49 PDT
Checked into the 1.8.0 branch.
Comment 23 Tracy Walker [:tracy] 2006-05-04 08:31:15 PDT
verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060504 Firefox/1.5.0.4

Note You need to log in before you can comment on or make changes to this bug.