Closed Bug 672438 Opened 11 years ago Closed 11 years ago

Crash [@ nsINode::CompareDocPosition(nsINode*) ] with too many attr children

Categories

(Core :: DOM: HTML Parser, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla8
Tracking Status
firefox7 --- affected
firefox8 --- affected

People

(Reporter: bc, Assigned: mats)

References

()

Details

(Keywords: crash, reproducible, Whiteboard: [inbound][sg:dos] null-pointer crash)

Crash Data

Attachments

(2 files)

Attached file aurora stack
1. http://dopey.de/flash/
2. Crash Aurora/7, Nightly/8 on Mac.

rapidly issues oodles of warnings then crash with Aurora. Nightly takes a bit longer. Haven't checked Beta/6.

aurora bp-8698ca1b-054a-41c5-8d01-4f6572110718

WARNING: NS_ENSURE_TRUE(childCount < ATTRCHILD_ARRAY_MAX_CHILD_COUNT) failed: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsAttrAndChildArray.cpp, line 170
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp, line 3473

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x054254eb in nsINode::CompareDocPosition (this=0x0, aOtherNode=0x26151260) at /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp:774
774	  if (node2->IsNodeOfType(nsINode::eATTRIBUTE)) {

fwiw, this is another of these sites with broken php scripts that eat memory and crash the browser.

Automation did see a different stack with aurora:
NS_DebugBreak_P | nsCOMPtr<nsIParser>::operator-> | nsScriptElement::MaybeProcessScript nsHTMLScriptElement::BindToTree nsINode::doInsertChildAt nsGenericElement::InsertChildAt nsINode::AppendChildTo  

The original Socorro signature was mozalloc_abort(char const* const) | mozalloc_handle_oom() | nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) | nsTArray<nsHtml5TreeOperation, nsTArrayDefaultAllocator>::MoveElementsFrom<nsHtml5TreeOperation, nsTArrayD...
Jonas, can you take a look?
Severity: normal → critical
PostPendingAppendNotification is called before AppendChildTo:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeOperation.cpp#213
which results in a new nsHtml5PendingNotification object:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5PendingNotification.h#50
with mChildCount = aParent->GetChildCount().

The bug occurs when the child count has reached the maximum value,
then AppendChildTo() fails and mChildCount has a value that isn't
a valid index so the mParent->GetChildAt(mChildCount) at line 60
returns null leading to the null-pointer crash.
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [sg:dos] null-pointer crash
Attached patch Like so?Splinter Review
Notifying only if AppendChildTo succeeds seems like the way to go.
Attachment #547002 - Flags: review?(hsivonen)
Comment on attachment 547002 [details] [diff] [review]
Like so?

Looks reasonable in order to fix the crash.

I wonder if we should try, as a more elaborate follow-up, to halt the parser on unload the document upon failures that leave the DOM in a state that doesn't correspond to what the parser saw.
Attachment #547002 - Flags: review?(hsivonen) → review+
Filed bug 672928 to follow-up Henri's review comment.

http://hg.mozilla.org/integration/mozilla-inbound/rev/8d56702130fb
Assignee: jonas → matspal
Component: DOM → HTML: Parser
QA Contact: general → parser
Whiteboard: [sg:dos] null-pointer crash → [inbound][sg:dos] null-pointer crash
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/8d56702130fb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.