1732362 - Allow JS Exception Handler setup to be disabled

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Per jandem on Matrix - On startup (on Windows) we need register an exception handler on windows for the (future) executable memory region. But the handler's code we generate at runtime which itself is causing the crash. We don't have a way to disable the exception handling generation right now.

There are JitOptions for disabling ion and baseline; maybe those would be sufficient to determine whether or not to set up the exception handling?

Comment 1

[Nicolas B. Pierron [:nbp]]

.

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Ping jandem - could you outline what needs to be done in the engine for this?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #2)

Ping jandem - could you outline what needs to be done in the engine for this?

(I'm looking into this. There's some complexity because the Wasm prefs and JIT prefs use a different mechanism..)

Is anything blocked on this short-term? We've been wanting to do some clean up in this area for a long time, but I don't want to block work on the socket process...

Comment 4

[Kershaw Chang [:kershaw]]

FYI: bug 1475641 is kind of blocked by this, but we could adjust the sandboxing rule to allow to initialize JS on the socket process.

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

It would be good to do this in the next few months, the longer we leave the Socket Process without ACG the more likely we wind up with some other code in it that breaks ACG that we don't know about, that we then need to investigate and fix.

Comment 6

[Jan de Mooij [:jandem]]

I've been looking into this. The main issue is the atomic-operation stubs we generate at runtime, these are used to load/store typed array elements for example. We have a fallback for no-jit platforms, but using that implementation at runtime without hurting perf is pretty complicated.

Comment 7

[Jan de Mooij [:jandem]]

Another issue is that it would run PAC scripts without any JIT support. This could affect scripts that iterate over a list of hosts/IPs or that use regular expressions for pattern matching. Maybe that's fine and we don't really care, but there is some risk.

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Jan de Mooij [:jandem] from comment #6)

I've been looking into this. The main issue is the atomic-operation stubs we generate at runtime, these are used to load/store typed array elements for example. We have a fallback for no-jit platforms, but using that implementation at runtime without hurting perf is pretty complicated.

I'm not sure we need to do the complicated thing? The ask here is to just give us an option to tell the JS Engine to initialize without any codegen (so we can enable the security mitigation that blocks ever making writable memory executable.) We don't need to do it all the time - the only places we expect to ask the Engine to initialize without codegen is the Socket Process and potentially the Utility Process if that winds up hosting JS but right now we don't have plans for that AFAIK.

(In reply to Jan de Mooij [:jandem] from comment #7)

Another issue is that it would run PAC scripts without any JIT support. This could affect scripts that iterate over a list of hosts/IPs or that use regular expressions for pattern matching. Maybe that's fine and we don't really care, but there is some risk.

We are okay with this. We will have a flag to disable the ACG on the socket process for debugging and tweaking purposes, and we can wire things up so we tell the JS Engine it can initialize with the JIT in that case. So if this produce a hardship we do have an escape hatch, and I will let mkaply know so we can make it an enterprise policy if we want.

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #8)

I'm not sure we need to do the complicated thing? The ask here is to just give us an option to tell the JS Engine to initialize without any codegen (so we can enable the security mitigation that blocks ever making writable memory executable.) We don't need to do it all the time - the only places we expect to ask the Engine to initialize without codegen is the Socket Process and potentially the Utility Process if that winds up hosting JS but right now we don't have plans for that AFAIK.

The problem is that certain JS operations just don't work in this mode without the atomics stubs, for example loading/storing typed array elements would crash because the VM uses those. This also means we can't really test this mode in the JS shell with fuzzing. I'll see if I can make this work somehow.

Comment 10

[Jan de Mooij [:jandem]]

I have a prototype to generate GCC/Clang inline assembly for the atomic operations (based on the trampoline code we currently generate at runtime). It seems to work and it would then let us disable JIT codegen completely.

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1732362 part 1 - Use system malloc for JS shell's option parser. r?jonco!

The next patch changes the shell to parse the shell flags before calling JS_Init.
This means we have to use the system malloc instead of JS malloc.

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1732362 part 2 - Initialize JS engine after parsing command line options. r?jonco!

This allows us to have shell flags that affect JS_Init.

Depends on D136722

Comment 13

[Jan de Mooij [:jandem]]

Attachment: Bug 1732362 part 3 - Add an API to disable the JIT backend completely. r?iain!

This adds a mechanism to permanently disable the JIT backend for the process. This
lets us improve the sandbox for the socket process.

Depends on D136723

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/373d232b065a
part 1 - Use system malloc for JS shell's option parser. r=jonco
https://hg.mozilla.org/integration/autoland/rev/e9ac9d3459d3
part 2 - Initialize JS engine after parsing command line options. r=jonco
https://hg.mozilla.org/integration/autoland/rev/ad80819d2534
part 3 - Add an API to disable the JIT backend completely. r=iain

Comment 15

[Jan de Mooij [:jandem]]

Tom, assuming this sticks, you can call JS::DisableJitBackend() before JS_Init/JS_InitWithFailureDiagnostic to completely disable any JIT-related activity for that process.

Comment 16

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Thank you!

Comment 17

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/373d232b065a
https://hg.mozilla.org/mozilla-central/rev/e9ac9d3459d3
https://hg.mozilla.org/mozilla-central/rev/ad80819d2534