738383 - OOM Crash [@ NS_LogCOMPtrRelease_P] with possibly-controllable invalid read

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Christian Holler (:decoder)]

Attachment: Log with failed allocations

Tested on m-c revision c4eb6151a3bf: During OOM testing for bug 555303 I got a crash as shown in the attached log (very large, lots of allocation failures before the crash, but I believe only the very last allocation is relevant). 

The log shows an invalid read and the address being read from is entirely ASCII, so it's very likely that this is data (which makes it possibly-controllable, depending on where this data comes from).

Comment 1

[Mats Palmgren (inactive)]

Making Hashtable classes infallable by default should fix this.

That said, an OOM crash in code that only runs during startup is extremely unlikely
to occur in real use and not worth spending time on, IMO.  It would be better if you
could delay injecting OOMs until you get past the startup phase.

Comment 2

[Christian Holler (:decoder)]

> That said, an OOM crash in code that only runs during startup is extremely
> unlikely

Are you sure this is startup code?

Independent of that: It's not possible to wait past the startup phase without actually doing allocation counting again. The allocation that I picked here and simulated OOM for, was after the startup. If this crash is in startup code now, then the reason is most likely that the same function allocates during *and* after startup.

Comment 3

[Mats Palmgren (inactive)]

> Are you sure this is startup code?

Yes, if the stack frames don't lie to me.
The crash is in nsLayoutStatics::Initialize() which is only run once,
from the layout module's Init method, which is only run (once) during startup.

Comment 4

[Christian Holler (:decoder)]

(In reply to Mats Palmgren [:mats] from comment #3)

> The crash is in nsLayoutStatics::Initialize() which is only run once,
> from the layout module's Init method, which is only run (once) during
> startup.

Thanks :) I suspect that the program was simulating OOM for some common function here (that is also called later, but also involved in this startup code). That would explain why it found out a startup crash.

Comment 5

[Daniel Veditz [:dveditz]]

If this is only a startup crash can we unhide the bug?

Comment 6

[Mats Palmgren (inactive)]

Bug 734847 will fix this and I think this particular crash is a wontfix on branches
where bug 734847 doesn't land, so I think we can just close this bug since there's
no action to track.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Marking fixed because bug 734847 is fixed (per comment 6).