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

VERIFIED FIXED in mozilla1.9alpha5

Status

()

Core
XUL
--
critical
VERIFIED FIXED
10 years ago
6 years ago

People

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

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9alpha5
crash, testcase, verified1.8.0.13, verified1.8.1.5
Points:
---
Bug Flags:
blocking1.8.1.5 +
wanted1.8.1.x +
blocking1.8.0.13 +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] using deleted memory?, crash signature)

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 259678 [details]
iframe needed for testcase

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
(Reporter)

Comment 1

10 years ago
Created attachment 259679 [details]
testcase

Updated

10 years ago
Assignee: jag → Olli.Pettay

Comment 2

10 years ago
Note, only non-debug-builds crash.

Comment 3

10 years ago
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

Comment 4

10 years ago
So mRuleNode is deleted before nsStyleContext?
(Reporter)

Updated

10 years ago
Flags: blocking1.9?
Created attachment 263678 [details]
Branch log from 2.0.0.4 RC1
Created attachment 263679 [details]
Mac Trunk log from 2007-04-28

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?
Created attachment 263681 [details]
Mac Branch Debug log from 1.8 branch (~2007050317)

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.  :(

Comment 10

10 years ago
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.
Created attachment 263698 [details] [diff] [review]
Does this fix the crash?

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 14

10 years ago
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+
Created attachment 263699 [details] [diff] [review]
Same as diff -w for ease of review
Attachment #263699 - Flags: superreview?(jst)

Updated

10 years ago
Attachment #263699 - Flags: superreview?(jst) → superreview+
Assignee: jag → bzbarsky
Fixed on trunk.
Status: NEW → RESOLVED
Last Resolved: 10 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?
(Reporter)

Comment 18

10 years ago
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.
Keywords: fixed1.8.0.13, fixed1.8.1.5
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
Keywords: fixed1.8.1.5 → verified1.8.1.5
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
Keywords: fixed1.8.0.13 → verified1.8.0.13
Group: security

Comment 24

8 years ago
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.