1452090 - Only enable handle verifier on 32-bit Nightly and debug builds

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Bob Owen (:bobowen)]

Chromium only currently enables this on x86 Canary, developer and debug builds.

This also adds hooks similar to chromium to monitor handle closing for non-verifier tracked handles, to catch where problems exist in other code.

We might want to turn this on for Beta at some point as our Nightly Win32 coverage is pretty poor.

I'm also adding an environment variable MOZ_ENABLE_HANDLE_VERIFIER, so it can be turned on for any build, which might prove useful.

Comment 1

[Bob Owen (:bobowen)]

The hooks cause assertions on 64-bit builds anyway:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=16863768c4ad2cff208f6275f09242cb6ca49cbf

Here's a push for 32-bit only:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9f42154781679076917e6e97fd02f3e0c62b38e

Comment 2

[Bob Owen (:bobowen)]

Attachment: Only enable handle verifier on 32-bit Nightly and debug builds

This also adds the ability to enable it using the environement variable
MOZ_ENABLE_HANDLE_VERIFIER.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0c0f90a710d
Only enable handle verifier on 32-bit Nightly and debug builds. r=jimm

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d0c0f90a710d

Comment 5

[Bob Owen (:bobowen)]

Comment on attachment 8965701 [details] [diff] [review]
Only enable handle verifier on 32-bit Nightly and debug builds

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
The ActiveVerifier has caused a couple of different crashes.
Chrome don't have it turned on in Release for this reason and I've just realised that my patch to turn it off missed ESR.

User impact if declined:
Crashes associated with the ActiveVerifier will still occur and there is possibly some performance impact as well.

Fix Landed on Version:
Fx61

Risk to taking this patch (and alternatives if risky): 
Fairly low, while the patch does add some hooking code it is only actually used on Nightly.
The other change is just to not use the ActiveVerifier, which is diagnostic code to try and catch Windows Handle management issues, but can introduce its own crashes as well.
The diagnostic code would still crash either way round, so turning it off should only improve stability.

String or UUID changes made by this patch:
None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8965701 [details] [diff] [review]
Only enable handle verifier on 32-bit Nightly and debug builds

Avoids crashes by not shipping code that was intended for pre-release audiences only. Approved for ESR 60.1.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/e85d91afc9a3