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

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Andrew McCreight [:mccr8]]

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

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: OOM crash if malloc fails in ProcessIncomingMessages().

This allocation is for no more than 32kb, so make it infallible rather than try to recover.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8744085 [details] [diff] [review]
OOM crash if malloc fails in ProcessIncomingMessages().

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

Thanks.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/98b498a402eb

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98b498a402eb

Comment 5

[Andrew McCreight [:mccr8]]

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

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8744085 [details] [diff] [review]
OOM crash if malloc fails in ProcessIncomingMessages().

Correctly annotate said crashes as OOM crashes, Aurora47+

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8ef6c6539b96