Closed Bug 1266578 Opened 7 years ago Closed 7 years ago

Missing OOM crash annotation in IPC::Channel::ChannelImpl::ProcessIncomingMessages

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

In this memcpy:
          buf = (char*)malloc(len);
          memcpy(buf, p, len);

I suspect that this crash happens if malloc fails and returns NULL. It seems like this case is for smaller allocations, so maybe the malloc() can be replaced with moz_xmalloc(). There's a version of this code in both ipc_channel_win.cc and ipc_channel_posix.cc. I read over the rest of the patch in bug 1235633 and did not see any other such issues (aside from the realloc one which has been fixed.)

https://crash-stats.mozilla.com/report/index/e22b99b5-2160-4322-82c7-114f72160420
This allocation is for no more than 32kb, so make it infallible rather than try to recover.
Attachment #8744085 - Flags: review?(wmccloskey)
Comment on attachment 8744085 [details] [diff] [review]
OOM crash if malloc fails in ProcessIncomingMessages().

Review of attachment 8744085 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.
Attachment #8744085 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/98b498a402eb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8744085 [details] [diff] [review]
OOM crash if malloc fails in ProcessIncomingMessages().

Approval Request Comment
[Feature/regressing bug #]: bug 1235633
[User impact if declined]: some crashes won't be annotated as OOM crashes
[Describe test coverage new/current, TreeHerder]: this code is tested, but not the crashing
[Risks and why]: very low, it just makes an allocation infallible
[String/UUID change made/needed]: none
Attachment #8744085 - Flags: approval-mozilla-aurora?
Comment on attachment 8744085 [details] [diff] [review]
OOM crash if malloc fails in ProcessIncomingMessages().

Correctly annotate said crashes as OOM crashes, Aurora47+
Attachment #8744085 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Summary: Null deref crash in IPC::Channel::ChannelImpl::ProcessIncomingMessages → Missing OOM crash annotation in IPC::Channel::ChannelImpl::ProcessIncomingMessages
You need to log in before you can comment on or make changes to this bug.