Closed
Bug 461239
Opened 16 years ago
Closed 16 years ago
[64-bit] Crash [@ RuleProcessorData::GetNthIndex] when adding <style> node to <head>
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b2
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
Details
(4 keywords, Whiteboard: [sg:critical?] virtual method call on random pointer value)
Crash Data
Attachments
(3 files, 1 obsolete file)
|
(deleted),
application/xhtml+xml
|
Details | |
|
(deleted),
text/plain
|
Details | |
|
(deleted),
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE
1. load the attached testcase
ACTUAL RESULT
Crash occurs in local Firefox debug build on Linux (64-bit).
Crash occurs in Firefox 2008-10-22-02 nightly build (64-bit).
Crash does NOT occur in Firefox 2008-10-22-02 nightly build (32-bit).
Flags: blocking1.9.1?
|
Assignee | |
Comment 1•16 years ago
|
||
|
|
Assignee | |
Comment 2•16 years ago
|
||
|
|
Assignee | |
Comment 3•16 years ago
|
||
Regression window: 2008-08-18-05 -- 2008-08-19-05
http://hg.mozilla.org/mozilla-central/shortlog/3c518880b8d4
bug 449447 ?
Keywords: regression
|
|
Assignee | |
Comment 4•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/file/6c4bb68efd2c/layout/style/nsCSSRuleProcessor.cpp#l1011
With a random value at *curChildPtr and then calling the virtual method
IsNodeOfType() on it this bug seems sg:critical to me.
Whiteboard: [sg:critical?] virtual method call on random pointer value
|
|
Assignee | |
Comment 5•16 years ago
|
||
The problem is that 'childCount' is unsigned.
I found one more in SelectorMatches().
Assignee: nobody → mats.palmgren
Attachment #344399 -
Flags: superreview?(bzbarsky)
Attachment #344399 -
Flags: review?(bzbarsky)
|
||
Comment 6•16 years ago
|
||
So how come this wasn't a problem before? I guess because a very large |cur| just caused us to get null and bail?
The code in ::lastNode is ok, because if index == 0 then GetChildAt() will just return null and the effect will be the same: null lastNode and ending the loop.
|
|
||
Comment 7•16 years ago
|
||
Comment on attachment 344399 [details] [diff] [review]
Patch rev. 1
>+++ b/layout/style/nsCSSRuleProcessor.cpp
> stopPtr = curChildPtr - 1;
>- curChildPtr += childCount - 1;
>+ curChildPtr += PRInt32(childCount) - 1;
I'd prefer:
curChildPtr = stopPtr + childCount;
since that more clearly indicates that we'll walk through childCount (possibly 0) entries.
|
|
Assignee | |
Comment 8•16 years ago
|
||
(In reply to comment #6)
> I guess because a very large |cur| just caused us to get null and bail?
I would guess so too.
|
|
Assignee | |
Comment 9•16 years ago
|
||
Attachment #344399 -
Attachment is obsolete: true
Attachment #344783 -
Flags: superreview?(bzbarsky)
Attachment #344783 -
Flags: review?(bzbarsky)
Attachment #344399 -
Flags: superreview?(bzbarsky)
Attachment #344399 -
Flags: review?(bzbarsky)
|
|
||
Comment 10•16 years ago
|
||
Comment on attachment 344783 [details] [diff] [review]
Patch rev. 2
Please add a test.
Attachment #344783 -
Flags: superreview?(bzbarsky)
Attachment #344783 -
Flags: superreview+
Attachment #344783 -
Flags: review?(bzbarsky)
Attachment #344783 -
Flags: review+
|
|
||
Comment 11•16 years ago
|
||
I pushed the fix. The testcase seems to depend on timeouts and even worse on details of the textbox binding. Jesse, do you think you can whip together a self-contained crashtest?
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
|
||
Comment 12•16 years ago
|
||
Reading comment 0 I assume it's 64bit only? I'm not able to reproduce it on 32bit.
Hardware: x86 → x86_64
Target Milestone: --- → mozilla1.9.1b2
Version: unspecified → Trunk
|
||
Updated•13 years ago
|
Crash Signature: [@ RuleProcessorData::GetNthIndex]
|
||
Updated•11 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•