Closed
Bug 360642
Opened 18 years ago
Closed 18 years ago
Crash [@ nsListBoxBodyFrame::GetNextItemBox] removing child from <listboxbody>
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: smaug)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(3 files)
280 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
1.48 KB,
patch
|
enndeakin
:
review+
roc
:
superreview+
dveditz
:
approval1.8.0.9+
dveditz
:
approval1.8.1.1+
|
Details | Diff | Splinter Review |
3.14 KB,
patch
|
Details | Diff | Splinter Review |
Loading the testcase makes Firefox crash [@ nsListBoxBodyFrame::GetNextItemBox].
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Updated•18 years ago
|
Group: security
Reporter | ||
Comment 2•18 years ago
|
||
Oops. This looked like a null deref in an opt build, but in a debug build, not so much:
EXC_BAD_ACCESS (0x0001)
KERN_INVALID_ADDRESS (0x0001) at 0xddddde01
Thread 0 Crashed:
0 nsIFrame::IsBoxFrame() const + 20 (nsIFrame.h:1546)
1 nsListBoxBodyFrame::GetNextItemBox(nsIFrame*, int, int*) + 696 (nsListBoxBodyFrame.cpp:1224)
...
Assignee | ||
Updated•18 years ago
|
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
Assignee | ||
Comment 3•18 years ago
|
||
So the problem seems to be that mTopFrame is cached.
Assignee | ||
Comment 4•18 years ago
|
||
Using nsWeakFrame is at least simple, operator overloading works in most
cases.
Other option might be to not to cache mTopFrame at all.
Assignee | ||
Comment 5•18 years ago
|
||
Is this too late for 1.8.1.1?
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
Comment 6•18 years ago
|
||
Comment on attachment 245649 [details] [diff] [review]
possible patch
This should be ok for a security update if needed. There's also mBottomFrame and mLinkupFrame as well. Maybe we should do to the same for those.
I think in the long run we want to find a way to remove the need to cache these frames.
Attachment #245649 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 7•18 years ago
|
||
(In reply to comment #6)
> (From update of attachment 245649 [details] [diff] [review] [edit])
> This should be ok for a security update if needed. There's also mBottomFrame
> and mLinkupFrame as well. Maybe we should do to the same for those.
>
mBottomFrame is usually updated whenever mTopFrame is, but in any case yes,
should look at whether it is possible to crash with deleted mBottomFrame or
mLinkupFrame.
Assignee | ||
Updated•18 years ago
|
Attachment #245649 -
Flags: superreview?(roc)
Assignee | ||
Comment 8•18 years ago
|
||
mLinkupFrame is used only for comparisons, so that should be safe, I think.
Assignee | ||
Comment 9•18 years ago
|
||
And mBottomFrame should be safe too. It is used only when mTopFrame and mBottomFrame are both updated.
Fragile, but hopefully safe.
Reporter | ||
Updated•18 years ago
|
Whiteboard: [sg:critical?]
Updated•18 years ago
|
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+
Comment on attachment 245649 [details] [diff] [review]
possible patch
We probably shouldn't be holding pointers to these frames here. Can't we reacquire them as needed? I'd like to see a real fix for this on trunk...
Attachment #245649 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Updated•18 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•18 years ago
|
Attachment #245649 -
Flags: approval1.8.1.1?
Attachment #245649 -
Flags: approval1.8.0.9?
Comment 11•18 years ago
|
||
Comment on attachment 245649 [details] [diff] [review]
possible patch
approved for 1.8/1.8.0 branches, a=dveditz
Please also include port/build bustage fix bug 361058
Attachment #245649 -
Flags: approval1.8.1.1?
Attachment #245649 -
Flags: approval1.8.1.1+
Attachment #245649 -
Flags: approval1.8.0.9?
Attachment #245649 -
Flags: approval1.8.0.9+
Assignee | ||
Comment 12•18 years ago
|
||
Assignee | ||
Updated•18 years ago
|
Keywords: fixed1.8.0.9,
fixed1.8.1.1
Comment 13•18 years ago
|
||
v.fixed on 1.8.0 and 1.8.1 branches with
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.9pre) Gecko/20061128 Firefox/1.5.0.9pre
and
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1pre) Gecko/20061128 BonEcho/2.0.0.1pre
No crashes with testcase (was crashing FF2.0).
Updated•18 years ago
|
Group: security
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Updated•14 years ago
|
Crash Signature: [@ nsListBoxBodyFrame::GetNextItemBox]
You need to log in
before you can comment on or make changes to this bug.
Description
•