The default bug view has changed. See this FAQ.

Crash [@ nsListBoxBodyFrame::GetNextItemBox] removing child from <listboxbody>

RESOLVED FIXED

Status

()

Core
XUL
--
critical
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: smaug)

Tracking

(Blocks: 2 bugs, 4 keywords)

Trunk
crash, testcase, verified1.8.0.9, verified1.8.1.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.1 +
blocking1.8.0.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
Loading the testcase makes Firefox crash [@ nsListBoxBodyFrame::GetNextItemBox].
(Reporter)

Comment 1

11 years ago
Created attachment 245540 [details]
testcase
(Reporter)

Updated

11 years ago
Group: security
(Reporter)

Comment 2

11 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)
...
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
So the problem seems to be that mTopFrame is cached.
Created attachment 245649 [details] [diff] [review]
possible patch

Using nsWeakFrame is at least simple, operator overloading works in most
cases.
Other option might be to not to cache mTopFrame at all.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #245649 - Flags: review?(enndeakin)
Is this too late for 1.8.1.1?
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?

Comment 6

11 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+
(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.

Attachment #245649 - Flags: superreview?(roc)
mLinkupFrame is used only for comparisons, so that should be safe, I think.
And mBottomFrame should be safe too. It is used only when mTopFrame and mBottomFrame are both updated.
Fragile, but hopefully safe.
(Reporter)

Updated

11 years ago
Whiteboard: [sg:critical?]
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+
Blocks: 360917
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Attachment #245649 - Flags: approval1.8.1.1?
Attachment #245649 - Flags: approval1.8.0.9?

Updated

11 years ago
Depends on: 361058
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+
Created attachment 246161 [details] [diff] [review]
for 1.8 with Bug 361058
Keywords: fixed1.8.0.9, fixed1.8.1.1

Comment 13

11 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).
Keywords: fixed1.8.0.9, fixed1.8.1.1 → verified1.8.0.9, verified1.8.1.1
Group: security
(Reporter)

Comment 14

9 years ago
Crashtest checked in.
Flags: in-testsuite+

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Crash Signature: [@ nsListBoxBodyFrame::GetNextItemBox]
You need to log in before you can comment on or make changes to this bug.