1215479 - Use W^X JIT code on all platforms

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[dcoppa]

Attachment: Mark JIT pages as non-writable for the OpenBSD firefox port

[Blocking Requested - why for this release]:


Hi all,

A question:

What's the rationale of having nonWritableJitCode enabled on iOS only?

I've tried rebuilding Firefox 41.0.1 on OpenBSD with the "#if TARGET_OS_IPHONE" conditional removed (see attached patch) and didn't see any major problem during normal usage of the browser...

So, what's the downside of having it enabled on OpenBSD?

Thanks!
David

Comment 1

[Landry Breuil (:gaston)]

From what i understand of the comments in #977805 having this enabled caused issues with asm.js, so was there any discussion on enabling this by default for other tier1 platforms and what would be the downsides ?

Comment 2

[Jan de Mooij [:jandem]]

(In reply to dcoppa from comment #0)
> What's the rationale of having nonWritableJitCode enabled on iOS only?

iOS requires it (because it doesn't allow allocating RWX memory), but it's not enabled elsewhere because it's a performance hit (we should remeasure how much though). This is because we have to toggle pages from executable to writable and back whenever we patch JIT code.

> I've tried rebuilding Firefox 41.0.1 on OpenBSD with the "#if
> TARGET_OS_IPHONE" conditional removed (see attached patch) and didn't see
> any major problem during normal usage of the browser...

Yes this should work.

> So, what's the downside of having it enabled on OpenBSD?

As I said, it could be slower. It'd be interesting to see how much it affects OpenBSD :) You can use the --non-writable-jitcode shell flag and run js/src/octane/run.js to test this.

Comment 3

[Landry Breuil (:gaston)]

This is with js shell from 42.0b6 on OpenBSD/amd64:

default run:
Richards: 18664
DeltaBlue: 29793
Crypto: 17183
RayTrace: 63491
EarleyBoyer: 18760
RegExp: 1609
Splay: 8622
SplayLatency: 10914
NavierStokes: 21010
PdfJS: 4863
Mandreel: 11340
MandreelLatency: 9181
Gameboy: 17626
CodeLoad: 8910
Box2D: 16808
zlib: 18478
Typescript: 14631
----
Score (version 9): 13320

with --non-writable-jitcode:
Richards: 18491
DeltaBlue: 28226
Crypto: 16634
RayTrace: 64749
EarleyBoyer: 18178
RegExp: 1565
Splay: 8035
SplayLatency: 11500
NavierStokes: 20438
PdfJS: 4973
Mandreel: 11232
MandreelLatency: 9164
Gameboy: 17007
CodeLoad: 8967
Box2D: 16073
zlib: 18115
Typescript: 13238
----
Score (version 9): 13038

More runs dont provide exactly the same results, but there's a small penalty. Maybe you can judge how big it is ?

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Landry Breuil (:gaston) from comment #3)
> More runs dont provide exactly the same results, but there's a small
> penalty. Maybe you can judge how big it is ?

Hm 13320 to 13038 is a 2.1% decrease, that's not too bad IMO. It might be worse on other workloads though so it's hard to say.

Maybe we should test this configuration on AWFY so we can see how it affects the various benchmarks on Mac/Linux/Windows. If there are (other?) platforms where the overhead is relatively low we could consider enabling it by default.

Hannes, could we get a --non-writable-jitcode line on AWFY, for the shell builds?

Comment 5

[Jan de Mooij [:jandem]]

Hm I just realized I disabled the implicit interrupt checks in Ion when JIT code is non-writable.

Fixing that will improve performance but it's pretty complicated unfortunately. I'll think about it more.

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> Hm I just realized I disabled the implicit interrupt checks in Ion when JIT
> code is non-writable.
> 
> Fixing that will improve performance but it's pretty complicated
> unfortunately. I'll think about it more.

FWIW, the problem here is that implicit interrupts patch backedges in all Ion code in the runtime, from a signal handler. With non-writable-jitcode, this'd have to unprotect/reprotect all Ion code, this is slow but also complicated because the main thread could be using an AutoWritableJitCode at the same time.

Clearing NI for now; I think we need a solution for this first.

Comment 7

[Landry Breuil (:gaston)]

Fwiw, we've been shipping this enabled to our users w/ 41.0.2 since http://marc.info/?l=openbsd-ports-cvs&m=144524219828017&w=2. Local runtime testing seems to show it 'works', but we'll see over time.

Comment 8

[dcoppa]

Output from some js/src/octane/run.js runs with 41.0.2 + the patch from 1215573 on OpenBSD/amd64:

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14664
DeltaBlue: 18403
Crypto: 17329
RayTrace: 57349
EarleyBoyer: 18588
RegExp: 565
Splay: 2920
SplayLatency: 793
NavierStokes: 23744
PdfJS: 4006
Mandreel: 2016
MandreelLatency: 7122
Gameboy: 4183
CodeLoad: 4173
Box2D: 5681
zlib: 2709
Typescript: 6108
----
Score (version 9): 5948

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 15060
DeltaBlue: 9235
Crypto: 17465
RayTrace: 61789
EarleyBoyer: 21307
RegExp: 436
Splay: 1573
SplayLatency: 483
NavierStokes: 23673
PdfJS: 4010
Mandreel: 2076
MandreelLatency: 8132
Gameboy: 3755
CodeLoad: 4489
Box2D: 5730
zlib: 2745
Typescript: 6105
----
Score (version 9): 5391

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14894
DeltaBlue: 8620
Crypto: 17511
RayTrace: 64749
EarleyBoyer: 19773
RegExp: 459
Splay: 3089
SplayLatency: 586
NavierStokes: 24189
PdfJS: 1296
Mandreel: 2101
MandreelLatency: 9114
Gameboy: 4239
CodeLoad: 3829
Box2D: 6145
zlib: 2726
Typescript: 6202
----
Score (version 9): 5351

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 15159
DeltaBlue: 25398
Crypto: 17335
RayTrace: 65341
EarleyBoyer: 19972
RegExp: 395
Splay: 1366
SplayLatency: 421
NavierStokes: 24388
PdfJS: 2255
Mandreel: 2003
MandreelLatency: 8199
Gameboy: 3548
CodeLoad: 4336
Box2D: 5830
zlib: 2702
Typescript: 6110
----
Score (version 9): 5379

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 15296
DeltaBlue: 17347
Crypto: 17395
RayTrace: 62751
EarleyBoyer: 20707
RegExp: 591
Splay: 2167
SplayLatency: 708
NavierStokes: 24338
PdfJS: 2111
Mandreel: 2024
MandreelLatency: 7802
Gameboy: 3916
CodeLoad: 4725
Box2D: 6342
zlib: 2748
Typescript: 6233
----
Score (version 9): 5776

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14827
DeltaBlue: 1030
Crypto: 17849
RayTrace: 64379
EarleyBoyer: 20371
RegExp: 494
Splay: 708
SplayLatency: 321
NavierStokes: 24413
PdfJS: 2017
Mandreel: 2022
MandreelLatency: 8364
Gameboy: 3464
CodeLoad: 4721
Box2D: 5958
zlib: 2757
Typescript: 5807
----
Score (version 9): 4270

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 13757
DeltaBlue: 1859
Crypto: 17893
RayTrace: 40363
EarleyBoyer: 19773
RegExp: 466
Splay: 1142
SplayLatency: 426
NavierStokes: 24758
PdfJS: 2277
Mandreel: 2049
MandreelLatency: 8080
Gameboy: 3335
CodeLoad: 4792
Box2D: 6174
zlib: 2511
Typescript: 6050
----
Score (version 9): 4475

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14792
DeltaBlue: 905
Crypto: 17807
RayTrace: 64527
EarleyBoyer: 20846
RegExp: 706
Splay: 2371
SplayLatency: 392
NavierStokes: 24313
PdfJS: 3803
Mandreel: 2022
MandreelLatency: 8199
Gameboy: 3803
CodeLoad: 4150
Box2D: 4286
zlib: 2720
Typescript: 5929
----
Score (version 9): 4778

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 15356
DeltaBlue: 6228
Crypto: 15974
RayTrace: 65859
EarleyBoyer: 20652
RegExp: 484
Splay: 1813
SplayLatency: 383
NavierStokes: 24561
PdfJS: 3028
Mandreel: 2088
MandreelLatency: 8522
Gameboy: 3683
CodeLoad: 4256
Box2D: 6398
zlib: 2690
Typescript: 6049
----
Score (version 9): 5204

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14318
DeltaBlue: 3518
Crypto: 17304
RayTrace: 8929
EarleyBoyer: 21104
RegExp: 583
Splay: 2097
SplayLatency: 589
NavierStokes: 24758
PdfJS: 2375
Mandreel: 2110
MandreelLatency: 8364
Gameboy: 4334
CodeLoad: 4237
Box2D: 6360
zlib: 2701
Typescript: 6081
----
Score (version 9): 4666

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14421
DeltaBlue: 3636
Crypto: 11392
RayTrace: 64083
EarleyBoyer: 19929
RegExp: 406
Splay: 1418
SplayLatency: 568
NavierStokes: 24069
PdfJS: 2314
Mandreel: 2109
MandreelLatency: 8119
Gameboy: 3958
CodeLoad: 4532
Box2D: 5796
zlib: 2736
Typescript: 6002
----
Score (version 9): 4818

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14965
DeltaBlue: 14381
Crypto: 18270
RayTrace: 56017
EarleyBoyer: 20435
RegExp: 702
Splay: 1695
SplayLatency: 497
NavierStokes: 24660
PdfJS: 2338
Mandreel: 2107
MandreelLatency: 8002
Gameboy: 4327
CodeLoad: 4469
Box2D: 5353
zlib: 2724
Typescript: 5934
----
Score (version 9): 5538

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14657
DeltaBlue: 1326
Crypto: 17884
RayTrace: 62899
EarleyBoyer: 19596
RegExp: 476
Splay: 582
SplayLatency: 589
NavierStokes: 24217
PdfJS: 2180
Mandreel: 2043
MandreelLatency: 8267
Gameboy: 3877
CodeLoad: 4094
Box2D: 6168
zlib: 2748
Typescript: 6271
----
Score (version 9): 4450

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 13835
DeltaBlue: 19815
Crypto: 15214
RayTrace: 65489
EarleyBoyer: 20062
RegExp: 398
Splay: 2241
SplayLatency: 575
NavierStokes: 24338
PdfJS: 1629
Mandreel: 2083
MandreelLatency: 7876
Gameboy: 3588
CodeLoad: 4096
Box2D: 5691
zlib: 2757
Typescript: 6229
----
Score (version 9): 5377

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 15081
DeltaBlue: 2112
Crypto: 17849
RayTrace: 64897
EarleyBoyer: 19384
RegExp: 550
Splay: 3366
SplayLatency: 752
NavierStokes: 24758
PdfJS: 2191
Mandreel: 2068
MandreelLatency: 8580
Gameboy: 3560
CodeLoad: 4341
Box2D: 6185
zlib: 2753
Typescript: 6172
----
Score (version 9): 5214

$ /usr/pobj/firefox-41.0.2/build-amd64/js/src/shell/js --non-writable-jitcode /usr/pobj/firefox-41.0.2/mozilla-release/js/src/octane/run.js
Richards: 14093
DeltaBlue: 2092
Crypto: 17027
RayTrace: 35854
EarleyBoyer: 19677
RegExp: 449
Splay: 2291
SplayLatency: 602
NavierStokes: 24289
PdfJS: 1703
Mandreel: 2065
MandreelLatency: 8267
Gameboy: 3528
CodeLoad: 4107
Box2D: 5929
zlib: 2728
Typescript: 6065
----
Score (version 9): 4648

Comment 9

[Tom S [:evilpie]]

Please don't post data like this inside the comments again ...

Comment 10

[Shawn Webb]

I've manually removed the #if TARGET_OS_IPHONE and forced nonWritableJitCode to be set to true on HardenedBSD. I'm still seeing pages created as rwx. Output of `procstat -v PID_of_firefox`: http://ix.io/lYV

Comment 11

[Jan de Mooij [:jandem]]

Let's make this a tracking bug for enabling W^X JIT code everywhere.

Comment 12

[Shawn Webb]

I've done some extra debugging on the HardenedBSD side. It turns out that due to the .GNU_STACK section in some shared objects requesting an executable stack, that is why I'm seeing rwx pages. I modified the RTLD (as a hack, not production-ready code) to ignore setting the exec bit from .GNU_STACK on amd64 and "voila!", no more rwx pages! Firefox works perfectly well with W^X JIT on HardenedBSD.

We also use RELRO+BIND_NOW, which causes the RTLD to create rwx mappings initially, then downgrade them to rw or rx (depending on their original setting). This means that firefox will crash when being initialized by the RTLD. Therefore, we still need to disable W^X enforcement for firefox. This isn't a problem Mozilla needs to solve, but rather HardenedBSD. Just an FYI.

Comment 13

[Jan de Mooij [:jandem]]

(In reply to Shawn Webb from comment #12)
> Firefox works perfectly well with W^X JIT on HardenedBSD.

Thanks, that's great to hear. I really appreciate OpenBSD and HardenedBSD testing this in the wild.

Comment 14

[Jan de Mooij [:jandem]]

Hannes, since you're on PTO this is not very urgent, but it'd be really nice to have the "non writable jit code" line on AWFY also for the Windows shell builds :)

I still have some patches in flight but I'm happy with OS X performance (at least the shell benchmarks we're testing). Kraken is 0.3% slower and Octane <= 1% or so. The gap on Sunspider is bigger: any compile-time overhead shows up there, but it's not that big (2-4%). I want to investigate this a bit more though.

If we get similar numbers on Windows and Linux we can probably enable this by default. If Windows is much slower, we need to figure out what to do about it.

Comment 15

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #14)
> If we get similar numbers on Windows and Linux we can probably enable this
> by default. If Windows is much slower, we need to figure out what to do
> about it.

I just downloaded a 32-bit shell build from Treeherder. On a Windows 8 machine the overhead of W^X is comparable to the overhead on OS X (actually a bit better even, I think). Linux also does a bit better than Mac.

So that's good news; I was worried VirtualProtect would be much slower than mprotect. Hopefully AWFY will confirm this.

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Patch

This just flips the flag. Try is green and performance looks fine, so I want to see what happens.

We should remove the flag at some point, because leaving it is risky: an attacker might be able to change it and then get RWX access, but that can wait for now.

Comment 17

[Shawn Webb]

What methods are you all using to measure performance? I'd be happy to do performance testing on HardenedBSD with some guidance.

Comment 18

[Jan de Mooij [:jandem]]

(In reply to Shawn Webb from comment #17)
> What methods are you all using to measure performance? I'd be happy to do
> performance testing on HardenedBSD with some guidance.

We run a number of benchmarks on https://arewefastyet.com/. Benchmarks like Sunspider/Kraken/Octane run in both the JS shell and the browser. The shell has a --non-writable-jitcode flag to enable W^X. To test this in the browser, you need to recompile and set the bool to true manually. You also want to use mozilla-central tip because I landed a number of performance improvements recently.

So if you could run these benchmarks in the browser or shell (with and without W^X) that'd be interesting data. Running them in the browser may be easier if you're not familiar with them, but it requires a recompile.

Comment 19

[Jan de Mooij [:jandem]]

Comment on attachment 8702244 [details] [diff] [review]
Patch

Adding Luke in case Brian is not around.

Comment 20

[Luke Wagner [:luke]]

Comment on attachment 8702244 [details] [diff] [review]
Patch

Great work on all this!

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0dbf1fe665f

Comment 22

[Carsten Book [:Tomcat]]

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

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Release Note Request (optional, but appreciated)
[Why is this notable]:
We probably want this to be part of the release notes. Press is talking about it:
http://www.ghacks.net/2016/01/04/mozilla-enables-wx-in-firefox-46-to-improve-security/
[Suggested wording]: Write XOR Execute security policy enabled for the Javascript JIT Compiler
[Links (documentation, blog post, etc)]: We could use Jandem's blog post: http://jandemooij.nl/blog/2015/12/29/wx-jit-code-enabled-in-firefox/
but I would prefer something more official (not a personal blog)

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

jan, can you post on a mozilla.org blog so we can link to that from relnotes? Thanks.

Comment 25

[Landry Breuil (:gaston)]

Seems this didnt make the final relnotes in the end : https://www.mozilla.org/en-US/firefox/44.0/releasenotes/
Is it too late ?

Comment 26

[Emanuel Hoogeveen [:ehoogeveen]]

This landed for Firefox 46 and was not uplifted, so it should be in the relnotes for 46.

(Could it be uplifted to 45? Probably not, but would be nice since it's the next ESR and presumably what Tor will switch to)

Comment 27

[Landry Breuil (:gaston)]

Woops, my bad, somehow i thought it was in 44. Disregard.

Comment 28

[Jan de Mooij [:jandem]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #24)
> jan, can you post on a mozilla.org blog so we can link to that from
> relnotes? Thanks.

Can we just link to this bug then? That should be good enough probably.

I don't think it's worth copy/pasting my post to the JS blog or something.

Comment 29

[Nicolas B. Pierron [:nbp]]

Maybe somebody can make a short summary on mozilla.org blog and forward to Jan blog post, as we did with an old blog post from Luke:

https://blog.mozilla.org/javascript/2012/10/08/optimizing-javascript-variable-access/

Comment 30

[wing-tung Leung]

We have an internal company web client application based on (JS generation) GWT/GXT (Sencha), which seems break terribly since Firefox Beta 46, and it is still apparent in FF Dev Edition 47.

Strangely enough, it also shows the same behavior in the very latest Chrome 49 release, which contains several security updates too. I haven't read anything specific regarding to W^X protection there.

Haven't spent time yet to test with a custom FF build *WITHOUT* the protection.

Maybe the GWT produced JS is not really W^X protection compatible.

Comment 31

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to wing-tung Leung from comment #30)
> We have an internal company web client application based on (JS generation)
> GWT/GXT (Sencha), which seems break terribly since Firefox Beta 46, and it
> is still apparent in FF Dev Edition 47.
> 
> Strangely enough, it also shows the same behavior in the very latest Chrome
> 49 release, which contains several security updates too. I haven't read
> anything specific regarding to W^X protection there.
> 
> Haven't spent time yet to test with a custom FF build *WITHOUT* the
> protection.
> 
> Maybe the GWT produced JS is not really W^X protection compatible.

Hi, this shouldn't be related to this enhancement: in a nutshell, this provoked Firefox to *crash* if there is a serious security issue (code being overwritten), which should *not* be hit by GWT, unless there is a bug in Firefox itself.

If the issue is reproducing in both the latest Chrome release and in Firefox Beta 46, it *could* also be a bug in your code base, but a bug in both browsers isn't excluded, of course.

If you have a comprehensive minimal test case reproducing the issue you're hitting on Firefox, and/or detailed steps to reproduce the issue on Beta 46, can you please open a new bug, set its component to JavaScript Engine, and we can start looking at it there? This bug here is probably not the source of your issues.