Closed Bug 1215479 Opened 9 years ago Closed 9 years ago

Use W^X JIT code on all platforms

Categories

(Core :: JavaScript Engine: JIT, defect)

Product:
Core
Component:
JavaScript Engine: JIT
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- fixed
relnote-firefox --- 46+

People

(Reporter: dcoppa, Assigned: jandem)

References

Details

(Keywords: feature, sec-want, Whiteboard: [adv-main46-])

Attachments

(1 file, 1 obsolete file)

Patch
652 bytes, patch
luke
: review+
Details | Diff | Splinter Review
[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
Blocks: 977805
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 ?
(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.
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 ?
(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?
Flags: needinfo?(hv1989)
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.
(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.
Flags: needinfo?(hv1989)
Depends on: 1215573
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.
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
Please don't post data like this inside the comments again ...
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
Let's make this a tracking bug for enabling W^X JIT code everywhere.
Summary: Enabling nonWritableJitCode on OpenBSD → Use W^X JIT code on all platforms
Assignee: nobody → jdemooij
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.
Depends on: 1233818
(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.
Depends on: 1234246
Depends on: 1235046
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.
Flags: needinfo?(hv1989)
(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.
Depends on: 1235201
Depends on: 1235338
Attached patch Patch — — Splinter Review
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.
Attachment #8674824 - Attachment is obsolete: true
Attachment #8702244 - Flags: review?(bhackett1024)
OS: OpenBSD → All
Hardware: x86_64 → All
What methods are you all using to measure performance? I'd be happy to do performance testing on HardenedBSD with some guidance.
(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 on attachment 8702244 [details] [diff] [review] Patch Adding Luke in case Brian is not around.
Attachment #8702244 - Flags: review?(luke)
Comment on attachment 8702244 [details] [diff] [review] Patch Great work on all this!
Attachment #8702244 - Flags: review?(luke) → review+
Attachment #8702244 - Flags: review?(bhackett1024)
Flags: needinfo?(hv1989)
https://hg.mozilla.org/mozilla-central/rev/a0dbf1fe665f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Depends on: 1235868
Keywords: sec-want
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)
relnote-firefox: --- → ?
Keywords: feature
Depends on: 1238554
jan, can you post on a mozilla.org blog so we can link to that from relnotes? Thanks.
relnote-firefox: ? → 46+
Flags: needinfo?(jdemooij)
Seems this didnt make the final relnotes in the end : https://www.mozilla.org/en-US/firefox/44.0/releasenotes/ Is it too late ?
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)
Woops, my bad, somehow i thought it was in 44. Disregard.
(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.
Flags: needinfo?(jdemooij)
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/
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.
(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.
Whiteboard: [adv-main46-]
Depends on: 1298667
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: