Closed Bug 1585712 Opened 11 months ago Closed 10 months ago

Crash [@ js::jit::ExecutableAllocator::reprotectPool] should use AutoEnterOOMUnsafeRegion instead of MOZ_CRASH

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed

People

(Reporter: decoder, Assigned: jandem)

Details

(Keywords: crash, testcase)

Attachments

(1 file)

We sometimes hit crashes in the ExecutableAllocator that seem to indicate that we are OOMing. The current implementation has a MOZ_CRASH() call in place, it should probably use AutoEnterOOMUnsafeRegion instead so the fuzzers will ignore the crash entirely.

The specific line that we are crashing at is here:

https://searchfox.org/mozilla-central/rev/f372e8a46ef7659ef61be9938ec2a3ea34d343c6/js/src/jit/ExecutableAllocator.cpp#266

NI for :jandem as discussed on IRC.

Flags: needinfo?(jdemooij)

While I agree that this is an OOM and not a crash I want to share what I learned when abusing mprotect calls on LifoAlloc buffers:

  • Linux reports an OOM properly.
  • Windows reports that arguments are incorrect.
  • Mac crashes within mprotect.

Thus, if we see frequent cases of these crashes in crash stat, we should probably look at a different way to handle the granularity of mprotect calls, to avoid having too much fragmentation.

Maybe a work-around or fallback would be to split our reserved space in N chunks of multiple MB to avoid the fragmentation.

mprotect is fallible and this makes it easier for fuzzers to deal with.

Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/937f71d963ae
Use AutoEnterOOMUnsafeRegion instead of MOZ_CRASH in ExecutableAllocator::reprotectPool. r=jonco
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → jdemooij
You need to log in before you can comment on or make changes to this bug.