Closed Bug 1129369 Opened 5 years ago Closed 5 years ago

Turn on DEP_NO_ATL_THUNK, BOTTOM_UP_ASLR and MITIGATION_STRICT_HANDLE_CHECKS process-level mitigations for the GMP sandbox.

Categories

(Core :: Security: Process Sandboxing, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

Attachments

(3 files)

I'm going to turn these on in separate patches to aid with bisection.

This still leaves some 64-bit only mitigations, but I don't want to turn those on until I've had time to investigate the occasional 64-bit sandbox start-up issues we've had with the content process.
Tim - I hope you're OK for these reviews, just straight forward policy changes.
Attachment #8560631 - Flags: review?(tabraldes)
Attachment #8560631 - Flags: review?(tabraldes) → review+
Attachment #8560632 - Flags: review?(tabraldes) → review+
Attachment #8560633 - Flags: review?(tabraldes) → review+
Sorry for the delay!
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bd213a576671
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6cb61619e34f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f83176aaffe9

(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #5)
> Sorry for the delay!

No need for an apology, I only uploaded them on Friday. :)
Comment on attachment 8560631 [details] [diff] [review]
Part 1: Turn on DEP_NO_ATL_THUNK process-level mitigation for the GMP sandbox.

Approval Request Comment
[Feature/regressing bug #]:
This brings the x86 process-level mitigations (and the overall sandbox rules in general) in line with Chromium's renderer.

[User impact if declined]:
The general rationale for uplifting this is that we are uplifting EME to beta (bug 1137045), so it makes sense to have the latest sandbox policy as well.

[Describe test coverage new/current, TreeHerder]:
EME itself has test coverage and the only other thing these changes could affect is the OpenH264 GMP process, which should also have mochitest coverage.
The changes have been on m-c for a couple of weeks now.

[Risks and why]:
Low to medium - it is probably unlikely, but possible that these changes cause problems for OpenH264, although as I say above, I've not heard of any issues on Nightly.

[String/UUID change made/needed]:
None.
Attachment #8560631 - Flags: approval-mozilla-beta?
Comment on attachment 8560632 [details] [diff] [review]
Part 2: Turn on BOTTOM_UP_ASLR process-level mitigation for the GMP sandbox.

See comment 8.
Attachment #8560632 - Flags: approval-mozilla-beta?
Comment on attachment 8560633 [details] [diff] [review]
Part 3: Turn on MITIGATION_STRICT_HANDLE_CHECKS process-level mitigation for the GMP sandbox.

See comment 8.
Attachment #8560633 - Flags: approval-mozilla-beta?
Comment on attachment 8560631 [details] [diff] [review]
Part 1: Turn on DEP_NO_ATL_THUNK process-level mitigation for the GMP sandbox.

OK. Let's get this change into Beta 2. Beta+
Attachment #8560631 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8560632 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8560633 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.