Closed Bug 375399 Opened 13 years ago Closed 13 years ago

Crash [@ nsElementSH::PostCreate] when removing window when accessing xul:tree in xul:tabs onselect in svg:foreignObject

Categories

(Core :: XUL, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9alpha5

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical?] using deleted memory?)

Crash Data

Attachments

(6 files, 1 obsolete file)

See upcoming testcase, which crashes current trunk and branch builds.

Talkback ID: TB30587921Q
0x02a8e765
nsElementSH::PostCreate  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 6836]
XPCWrappedNative::GetNewOrUsed  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 541]
XPCConvert::NativeInterface2JSObject  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 1104]
nsXPConnect::WrapNative  [mozilla/js/src/xpconnect/src/nsxpconnect.cpp, line 996]
nsDOMClassInfo::WrapNative  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 1484]
nsNodeSH::PreCreate  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 6388]
XPCWrappedNative::GetNewOrUsed  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 398]
XPCConvert::NativeInterface2JSObject  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 1104]
XPCConvert::NativeData2JS  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 480]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2337]

Talkback ID for branch: TB30587979K
Attached file testcase
Assignee: jag → Olli.Pettay
Note, only non-debug-builds crash.
hmm, this wasn't the kind of crash I thought.
I'll look at this anyway, but if someone notices immediately what is
going wrong, feel free to take this.
Assignee: Olli.Pettay → jag
So mRuleNode is deleted before nsStyleContext?
Flags: blocking1.9?
Based on the Mac branch crash log above, I don't think this is exploitable on the branch, but based on this trunk log, it could be exploitable there.

I'm also crashing in a completely different place than Martijn. Not sure if that's just Mac vs Windows. (And the branch and trunk crashes happen in different places as well but that's partially expected.)
OS: Windows XP → All
Hardware: PC → All
Boris, can you take a look at this? Or who's the best person to look at it?
This log from a debug build suggests this might be exploitable on the 1.8 branch, so I retract my above statement.
Attachment #263678 - Attachment is obsolete: true
I won't be able to look at this on a mac for a few weeks, probably...  Not sure who's best to look, to be honest.  :(
This is not a mac-only crash. (but non-debug-only maybe)
I'm really not familiar with stylecontext's memory handling, who owns what and how long. What should make sure that mRuleNode isn't deleted before stylecontext? dbaron?

#0  0x00000000006d0cf0 in ?? ()
#1  0x00002aaab0c182d6 in nsStyleContext::Destroy () from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libgklayout.so
#2  0x00002aaab0e0a4e8 in nsElementSH::PostCreate () from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libgklayout.so
#3  0x00002aaaaf519a30 in XPCWrappedNative::GetNewOrUsed ()
   from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libxpconnect.so
My crash log from comment 8 (attachment 263681 [details]) is from a branch debug build, so I don't think this is non-debug-only.
> I'm really not familiar with stylecontext's memory handling, who owns what
> and how long.

Style contexts are refcounted, but the memory is allocated from the presshell arena.  Which means if a style context outlives its presshell, it's toast.

Rule nodes are garbage collected, basically.  Every so often we nuke all the ones that are not pointed to by any style contexts.  See the NotifyStyleContextDestroyed() call in nsStyleContext::~nsStyleContext.  Again, they shouldn't be outliving the presshell.

Which brings us back to this bug.  Patch coming up, I think.
At a guess, the SVG node makes us not create frames for the <tabs>, so the binding is attached when we get the node, and the constructor fires the event, which kills off the presshell, etc.
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

That fixes it. and the patch makes sense to me :)
Attachment #263698 - Flags: review+
Attachment #263699 - Flags: superreview?(jst) → superreview+
Assignee: jag → bzbarsky
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: blocking1.9?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha5
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

It's worth doing this on branches.  It's a very safe fix.
Attachment #263698 - Flags: approval1.8.1.5?
Attachment #263698 - Flags: approval1.8.0.13?
Flags: in-testsuite?
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?] using deleted memory?
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070505 Minefield/3.0a5pre
Status: RESOLVED → VERIFIED
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #263698 - Flags: approval1.8.1.5?
Attachment #263698 - Flags: approval1.8.1.5+
Attachment #263698 - Flags: approval1.8.0.13?
Attachment #263698 - Flags: approval1.8.0.13+
Fixed on both branches.
verified fixed 1.8.1.5 using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.5pre) Gecko/2007070403 BonEcho/2.0.0.5pre on Fedora F7 and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.5pre) Gecko/20070704 BonEcho/2.0.0.5pre ID:2007070403 - no crash with the testcase from this bug - adding verified keyword
I wasn't able to reproduce the crash on Thunderbird 15012 or 15013 using the testcase #1. 
no crash on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.13pre) Gecko/20070822 Firefox/1.5.0.13pre with the testcase provided in this bug. Adding verified keyword for 1.8.0.13
Group: security
crash test landed
http://hg.mozilla.org/mozilla-central/rev/3a53dc42292c
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsElementSH::PostCreate]
You need to log in before you can comment on or make changes to this bug.