Last Comment Bug 737875 - OOM Crash [@ nsQueryInterfaceWithError::operator] trying to execute random memory
: OOM Crash [@ nsQueryInterfaceWithError::operator] trying to execute random me...
Status: RESOLVED FIXED
[sg:high][qa-]
: crash
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla14
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
: Andrew Overholt [:overholt]
Mentors:
: 733047 (view as bug list)
Depends on: 733047 737129
Blocks: 687256
  Show dependency treegraph
 
Reported: 2012-03-21 08:12 PDT by Christian Holler (:decoder)
Modified: 2015-10-16 11:38 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
12+
fixed


Attachments
Log with failed allocations (15.25 KB, text/plain)
2012-03-21 08:12 PDT, Christian Holler (:decoder)
no flags Details
Patch (2.81 KB, patch)
2012-03-22 11:31 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
hsivonen: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-03-21 08:12:29 PDT
Created attachment 607962 [details]
Log with failed allocations

+++ This bug was initially created as a clone of Bug #733047 +++

Tested on m-c revision 08809a43e082: During OOM testing for bug 737129 I got a crash shown in the attached log. The trace is very similar to that in bug 733047 but we're not trying to execute NULL here but random memory.

The log shows 6 failed allocations before the crash, and the crash location (address) varies (I had, "(bad)" and "Cannot access memory at", as well as random instructions).

Could it be that one of the failed allocations leaves a pointer uninitialized here?
Comment 1 Daniel Veditz [:dveditz] 2012-03-21 10:14:57 PDT
Not sure this is different than bug 733047, maybe it's the same and we need to raise the severity over there.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-22 11:31:57 PDT
Created attachment 608404 [details] [diff] [review]
Patch

What happens here is that we fail to append the nsIContent* to the nsCOMArray that holds it alive, and we end up with a dead pointer floating around.  The solution is to use an infallible array for mOwnedElements.
Comment 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-22 11:33:26 PDT
And I'm pretty sure this and bug 733047 are the same.
Comment 4 Henri Sivonen (:hsivonen) 2012-03-23 00:30:25 PDT
Comment on attachment 608404 [details] [diff] [review]
Patch

Thanks for figuring this out!
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-23 07:45:18 PDT
http://hg.mozilla.org/mozilla-central/rev/8042c37b8100
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-23 07:47:34 PDT
Comment on attachment 608404 [details] [diff] [review]
Patch

fwiw, I think this would be next to impossible to exploit, but the fix is safe enough that we could take it anywhere.

[Approval Request Comment]
Regression caused by (bug #): the HTML5 parser landing presumably
User impact if declined: in certain OOM scenarios we will crash in an uncontrolled way instead of in a controlled way
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): the risk is about as low as it gets
String changes made by this patch: N/A
Comment 7 Alex Keybl [:akeybl] 2012-03-26 14:15:22 PDT
Comment on attachment 608404 [details] [diff] [review]
Patch

[Triage Comment]
Approved for Aurora 13, Beta 12, and the ESR given the possible user impact and where we are in the schedule.
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 11:46:54 PDT
qa- until testcase-wanted has been satisfied.
Comment 10 Al Billings [:abillings] 2012-04-16 17:17:43 PDT
Are this and bug 733047 the same? This one is resolved fixed but bug 733047 is not even assigned and is still open.
Comment 11 Al Billings [:abillings] 2012-04-17 12:03:00 PDT
Christian thinks this is probably the same issue.
Comment 12 Daniel Veditz [:dveditz] 2012-04-18 23:55:17 PDT
*** Bug 733047 has been marked as a duplicate of this bug. ***
Comment 13 Al Billings [:abillings] 2012-05-04 14:42:09 PDT
Christian, is there any way to verify this fix or bug 737129, which is the same overall issue?
Comment 14 Christian Holler (:decoder) 2012-05-04 14:43:19 PDT
(In reply to Al Billings [:abillings] from comment #13)
> Christian, is there any way to verify this fix or bug 737129, which is the
> same overall issue?

Not really, bug if this is not fixed, then it will pop up again during OOM testing. I'm not doing that right now though since I'm too busy with other stuff :/

Note You need to log in before you can comment on or make changes to this bug.