crash with iframe removing itself [@ nsAttrAndChildArray::Clear] [@ nsAttrAndChildArray::~nsAttrAndChildArray]

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
DOM
P1
critical
RESOLVED FIXED
12 years ago
7 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Assigned: mrbkap)

Tracking

({crash, fixed1.8.1, verified1.8.0.4})

Trunk
mozilla1.9alpha1
crash, fixed1.8.1, verified1.8.0.4
Points:
---
Bug Flags:
blocking1.7.14 ?
blocking-aviary1.0.9 ?
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical] [patch], crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

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.
Created attachment 217404 [details]
crash
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
Component: General → DOM
Product: Firefox → Core
Target Milestone: --- → Future

Comment 3

12 years ago
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
Assignee: nobody → general
Severity: normal → critical
Keywords: crash
OS: Linux → All
QA Contact: general → ian
Hardware: PC → All
Summary: an iframe removing itself → crash with iframe removing itself [@ nsAttrAndChildArray::Clear] [@ nsAttrAndChildArray::~nsAttrAndChildArray]
Whiteboard: [sg:critical]
Target Milestone: Future → ---
sure, the seamonkey stack is scary on linux

Comment 5

12 years ago
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.  :(

Updated

12 years ago
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Flags: blocking1.8.0.3?
Flags: blocking1.7.14?
Flags: blocking-aviary1.0.9?

Updated

12 years ago
Flags: blocking1.8.0.3? → blocking1.8.0.3+
mrbkap has a patch
Assignee: general → mrbkap
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.
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [sg:critical] → [sg:critical] [patch]
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 8

12 years ago
Created attachment 217628 [details] [diff] [review]
Fix

This fixes the problem, as dbaron said.
Attachment #217628 - Flags: superreview?(bugmail)
Attachment #217628 - Flags: review?(jst)
Comment on attachment 217628 [details] [diff] [review]
Fix

r=jst
Attachment #217628 - Flags: review?(jst) → review+
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.)
(Assignee)

Comment 11

12 years ago
Created attachment 217630 [details] [diff] [review]
With that change

I'm convinced.
Attachment #217628 - Attachment is obsolete: true
Attachment #217630 - Flags: superreview?(bugmail)
Attachment #217630 - Flags: review?(jst)
Attachment #217628 - Flags: superreview?(bugmail)
Attachment #217630 - Flags: superreview?(bugmail) → superreview+
Comment on attachment 217630 [details] [diff] [review]
With that change

Yeah, looks better.
Attachment #217630 - Flags: review?(jst) → review+
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.
Attachment #217630 - Flags: review+ → review?(jst)
(Assignee)

Comment 14

12 years ago
Comment on attachment 217630 [details] [diff] [review]
With that change

jst reviewed this.
Attachment #217630 - Flags: review?(jst) → review+
(Assignee)

Comment 15

12 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 16

12 years ago
Comment on attachment 217630 [details] [diff] [review]
With that change

This fixes a use of free'd memory.
Attachment #217630 - Flags: approval1.8.0.3?

Comment 17

12 years ago
Comment on attachment 217630 [details] [diff] [review]
With that change

Taking some liberties here... ;)
Attachment #217630 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 18

12 years ago
Fix checked into the 1.8 branch.
Keywords: fixed1.8.1
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.
(Assignee)

Comment 20

11 years ago
(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 on attachment 217630 [details] [diff] [review]
With that change

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #217630 - Flags: approval1.8.0.3? → approval1.8.0.3+
(Assignee)

Comment 22

11 years ago
Checked into the 1.8.0 branch.
Keywords: fixed1.8.0.3
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
Keywords: fixed1.8.0.4 → verified1.8.0.4
Group: security

Updated

11 years ago
Flags: blocking1.9a1?
Flags: blocking1.8.1?

Updated

7 years ago
Crash Signature: [@ nsAttrAndChildArray::Clear] [@ nsAttrAndChildArray::~nsAttrAndChildArray]
You need to log in before you can comment on or make changes to this bug.