Last Comment Bug 677272 - (JITHardening) JIT hardening
(JITHardening)
: JIT hardening
Status: NEW
: sec-want
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 14 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 729824 700822
Blocks: exploit-mitigation b2gSystemSecurity
  Show dependency treegraph
 
Reported: 2011-08-08 09:33 PDT by Chris Leary [:cdleary] (not checking bugmail)
Modified: 2016-04-28 01:23 PDT (History)
43 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
0. WIP: W^X (7.96 KB, patch)
2011-08-08 19:02 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
SunSpider protect overhead (464.88 KB, image/svg+xml)
2011-08-08 19:04 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details
V8-V6 protect overhead (615.15 KB, image/svg+xml)
2011-08-08 19:10 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details
2. Randomize mappings on Windows (7.82 KB, patch)
2011-08-09 01:33 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
3a. WIP: assembler hardening scaffolding (61.64 KB, patch)
2011-08-09 01:46 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
3b. WIP: movi32r blinding (3.67 KB, patch)
2011-08-09 11:59 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
Win32 SunSpider protection overhead (54.56 KB, image/svg+xml)
2011-08-11 17:06 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details
Win32 V8-V6 protection overhead (64.55 KB, image/svg+xml)
2011-08-11 17:07 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details
Plotting script (646 bytes, text/plain)
2011-08-11 17:07 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details
5. WIP: NOP insertion (5.45 KB, patch)
2011-08-11 17:11 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
6. WIP: Random base offset for assemblies (2.06 KB, patch)
2011-08-11 17:12 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
3a. assembler hardening scaffolding. (59.37 KB, patch)
2011-08-23 13:46 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
5. NOP insertion. (5.52 KB, patch)
2011-08-23 13:47 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review-
Details | Diff | Review
6. Random base offset for assemblies (2.73 KB, patch)
2011-08-23 13:47 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
3b. Blinding. (3.02 KB, patch)
2011-08-23 13:48 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
Swap in real rand. (1.13 KB, patch)
2011-08-23 13:49 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
NanoJIT hardening. (2.70 KB, patch)
2011-08-23 13:49 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
5. NOP insertion. (5.78 KB, patch)
2011-08-29 16:14 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
NanoJIT hardening. (2.95 KB, patch)
2011-08-29 16:16 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
3b. Blinding. (3.23 KB, patch)
2011-08-29 16:25 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
3b. Blinding. (3.14 KB, patch)
2011-08-29 16:27 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review
Add JSOPTION_SOFTEN. (18.82 KB, patch)
2011-08-29 16:52 PDT, Chris Leary [:cdleary] (not checking bugmail)
no flags Details | Diff | Review
Add JSOPTION_SOFTEN, disabled by default. (21.61 KB, patch)
2011-08-30 15:29 PDT, Chris Leary [:cdleary] (not checking bugmail)
dmandelin: review+
Details | Diff | Review

Description Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 09:33:19 PDT
Chris Rolph and Yan Ivnitskiy from Matasano have been discussing JIT spray mitigations with us. Quoting Blazakis, 2010: "JIT spraying is the process of coercing the JIT engine to write many executable pages with embedded shellcode."

There are a number of patches and performance results drafted up that implement the following laundry-list of mitigation techniques, as enumerated in their black hat presentation ( http://www.matasano.com/research/jit/ ):

0. W^X permissions on code pages -- this entails changing the permissions on code pages for each IC repatch.
1. Guard pages -- this prevents heap overflows from adjacent rw- pages from crossing into -wx page (writable code) space. This overlaps with W^X and is also made much less feasible by mapping randomization (to follow).
2. Mapping randomization -- VirtualAlloc, unlike mmap, is not subject to the randomization of ASLR. Because the system call permits a requested "target address", we can randomize mappings ourselves with reasonable effectiveness.
3. Constant blinding -- by selecting a magic value to XOR immediate constants with, the attacker can no longer predict what immediate values will present themselves in the instruction stream.
4. Allocation restrictions -- by assuming that web-based workloads conform to a particular allocation pattern, it's possible to cull out allocation patterns for spray-oriented payloads.
5. NOP insertion -- because JIT spray relies on the uniform and predictable nature of the JIT instruction stream, inserting various forms of NOPs at random intervals can cause the attacker to lose control.
6. Random base offset -- JITted code can have a NOP-padding prologue of unknown size to make ROP gadget discovery and use more difficult in terms of page-relative offsets. This can also randomize offset at which an instruction stream starts (i.e. it can avoid being DWORD aligned in the first instruction), but (5) has overlapping benefits there.

This bug will elaborate on each patch and the observed performance impact of the techniques.
Comment 1 Tom Schuster [:evilpie] 2011-08-08 09:47:03 PDT
I gave some thoughts into that yesterday:
0. We would need to change the memory permissions twice for every patching in our IC's. We maybe should investigate separating code that needs patching and those that doesn't need it.
1. This should be free on the performance side (except maybe when stuff doesn't fit into the cache because of that and otherwise would). If all our code was not writable we wouldn't need that.
2. free except we might have some problems because the code is scattered around in memory.
3. This sounds like the biggest problem to me, when we need to store constants in tight code.
4. I think we already kinda do this by throwing away dead code in the GC, maybe we should get a bit more restrictive about this beforehand.
5 & 6. Memory increase

Overall the biggest problem will be memory overhead created by this.
Comment 2 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 09:47:32 PDT
Forgot to note that some of these changes will apply to all platforms, but the primary hardening target is Win32/x86.
Comment 3 Tom Schuster [:evilpie] 2011-08-08 09:49:40 PDT
>Forgot to note that some of these changes will apply to all platforms, but the >primary hardening target is Win32/x86.
From what i gathered most of this stuff is not harder to do on Linux.
Comment 4 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 11:39:31 PDT
Apologies to Chris Rohlf, I misspelled his name in the original post!
Comment 5 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 19:02:20 PDT
Created attachment 551656 [details] [diff] [review]
0. WIP: W^X

This is the patch that I drafted to implement W^X. Obviously, you incur additional overhead from the IC patches, where you're at least forcing the OS to manage page tables. Data to follow, with pretty plots.
Comment 6 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 19:04:46 PDT
Created attachment 551657 [details]
SunSpider protect overhead

16k reprotects in SunSpider, at a mean of ~.8us each, running us about 13.85ms. This plot has a pretty clear single mode, which is a nice contrast to the V8 plot to come.
Comment 7 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 19:10:03 PDT
Created attachment 551658 [details]
V8-V6 protect overhead

V8-V6 has a bimodal look to its 33k repatches. One peak is close to the mean at ~900ns, the other is about a stddev away at ~1.3us. Would be interesting to see what the properties are for those longer repatches; for example, is it a stub vs. inline patch distinction? Or perhaps repatches that cross page boundaries?
Comment 8 Chris Leary [:cdleary] (not checking bugmail) 2011-08-08 22:28:30 PDT
Comment on attachment 551656 [details] [diff] [review]
0. WIP: W^X

Pulling the numbers out from my notebook for ``js -m -a`` runs in the SunSpider harness:

Machine: Lenovo T510 Win7/x86 [i7 Arrandale]
SS:    ~5ms loss  = ~2.5% (w/ ~2ms variance)
V8-V6: ~30ms loss = ~2%   (w/~10ms variance)

Machine: Dell T3500 Linux/x86 [i7 Bloomfield]
SS:    ~8ms loss  = ~3.5% (w/ very little variance)
V8-V6: ~30ms loss = ~2.5% (w/ very little variance)

Previous SVG data was collected via |clock_gettime(CLOCK_MONOTINIC, ...)| timers on the Dell.
Comment 9 Chris Leary [:cdleary] (not checking bugmail) 2011-08-09 01:33:36 PDT
Created attachment 551698 [details] [diff] [review]
2. Randomize mappings on Windows

Per comments, we get 13 bits of randomness in the VirtualAlloc requested address on x86, and 21 bits of randomness on x64. As one would expect, I see no observable performance impact.
Comment 10 Chris Leary [:cdleary] (not checking bugmail) 2011-08-09 01:46:23 PDT
Created attachment 551701 [details] [diff] [review]
3a. WIP: assembler hardening scaffolding

This is the "scaffolding" I use as a basis for testing the other hardening techniques. Hardening is (aggressively) disabled in paths with assumed fixed instruction sequences (like ICs and jump tables). It's important to err on the side of caution here to avoid difficult-to-reproduce crashes as a result of self-inflicted randomness.
Comment 11 Chris Leary [:cdleary] (not checking bugmail) 2011-08-09 11:59:48 PDT
Created attachment 551842 [details] [diff] [review]
3b. WIP: movi32r blinding

This patch only blinds moves of imm32s into a register as a control experiment to avoid register pressure. On the Bloomfield box I see less than 0.5ms slowdown on SunSpider, but a 2-3% regression (~30ms) on V8-V6.
Comment 12 Jan de Mooij [:jandem] 2011-08-10 09:52:44 PDT
FWIW, I think V8 only blinds "large" constants. Noticed this a few months ago when comparing generated code and wondering why it was xor'ing this large integer in the loop test... Not saying that we have to follow them but it may help reduce the perf loss because small constants are probably much more common.
Comment 13 David Mandelin [:dmandelin] 2011-08-10 17:24:49 PDT
Comment on attachment 551698 [details] [diff] [review]
2. Randomize mappings on Windows

Review of attachment 551698 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. r+ with the 3 requested changes.

::: js/src/assembler/jit/ExecutableAllocatorWin.cpp
@@ +41,5 @@
>  
> +void *ExecutableAllocator::computeRandomAllocationAddress()
> +{
> +    /*
> +     * Inspration is V8's OS::Allocate in platform-win32.cc.

s/Inspration/Inspiration

@@ +51,5 @@
> +     * tries to avoid system default DLL mapping space. In the end, we get 13
> +     * bits of randomness in our selection. 
> +     * x64: [2GiB, 16TiB), with 21 bits of randomness.
> +     */
> +    static const uintN pageBits = 16;

s/pageBits/chunkBits

@@ +54,5 @@
> +     */
> +    static const uintN pageBits = 16;
> +#if WTF_CPU_X86_64
> +    static const uintptr_t base = 0x0000000080000000;
> +    static const uintptr_t mask = 0x000003ffffff0000;

I think |mask| is (2 ** 42 - 1), which goes with 4TiB, so either mask or the comment should change.
Comment 14 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:06:53 PDT
Created attachment 552546 [details]
Win32 SunSpider protection overhead

The discreteness of the values here is somewhat odd -- maybe contributed by use of RDTSC instead of clock_gettime, but the numbers sanity check.
Comment 15 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:07:14 PDT
Created attachment 552547 [details]
Win32 V8-V6 protection overhead
Comment 16 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:07:41 PDT
Created attachment 552548 [details]
Plotting script
Comment 17 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:11:24 PDT
Created attachment 552550 [details] [diff] [review]
5. WIP: NOP insertion

First of the patches that hadn't been posted yet. "Guard pages" (1) and "alloc restriction" (4) patches are not in a workable state and so will only be posted as followup bugs, if at all.

This patch regresses V8 by less than 1%, and actually gave a small-but-consistent-despite-the-randomness speedup to sunspider on my box. Maybe because of jump target alignment? Could be noise from elsewhere.
Comment 18 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:12:59 PDT
Created attachment 552552 [details] [diff] [review]
6. WIP: Random base offset for assemblies

Randomize starting location (unknown DWORD alignment) for JIT-assembled code.
Comment 19 Chris Leary [:cdleary] (not checking bugmail) 2011-08-11 17:14:06 PDT
Current WIP is mixing these together into a cocktail that doesn't significantly regress performance, along with enabled NanoJIT hardening flags. Can't get the JaegerMonkey sloshed.
Comment 20 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:46:28 PDT
Created attachment 555186 [details] [diff] [review]
3a. assembler hardening scaffolding.
Comment 21 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:47:03 PDT
Created attachment 555187 [details] [diff] [review]
5. NOP insertion.
Comment 22 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:47:48 PDT
Created attachment 555188 [details] [diff] [review]
6. Random base offset for assemblies
Comment 23 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:48:29 PDT
Created attachment 555189 [details] [diff] [review]
3b. Blinding.
Comment 24 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:49:02 PDT
Created attachment 555190 [details] [diff] [review]
Swap in real rand.
Comment 25 Chris Leary [:cdleary] (not checking bugmail) 2011-08-23 13:49:34 PDT
Created attachment 555191 [details] [diff] [review]
NanoJIT hardening.
Comment 26 David Mandelin [:dmandelin] 2011-08-29 11:15:28 PDT
Comment on attachment 555186 [details] [diff] [review]
3a. assembler hardening scaffolding.

Review of attachment 555186 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK. I do have a concern that this way of controlling when the hardening transformations are applied could be somewhat error-prone. In particular, I imagine the user has to be pretty careful about when hardening is and is not applied in a fairly "manual" way (the AutoHarden class helps here, though). (An example of this would be the register pinning in JM2: that was manual and proved hard to get right.) When we redo this for IonMonkey, think about how to make it safer: perhaps containing these things more, or maybe using assertions to check which hardening is done where. But I think it's OK as-is for JM2.
Comment 27 David Mandelin [:dmandelin] 2011-08-29 11:21:01 PDT
Comment on attachment 555187 [details] [diff] [review]
5. NOP insertion.

Review of attachment 555187 [details] [diff] [review]:
-----------------------------------------------------------------

Basically good but I have some significant nitpicky stuff.

::: js/src/assembler/assembler/X86Assembler.h
@@ +381,5 @@
>  
> +    static const uintN NOP_INST_COUNTDOWN_MIN = 128;
> +    static const uintN NOP_INST_COUNTDOWN_RAND_SPAN = 1023;
> +
> +    int fakeRand() const

A function with 'fake' in the name needs an explanatory comment.

@@ +390,5 @@
> +    void resetNopCountdown()
> +    {
> +        int randVal = fakeRand();
> +        countdown = NOP_INST_COUNTDOWN_MIN + (randVal & NOP_INST_COUNTDOWN_RAND_SPAN);
> +        nopSize = (randVal >> 16) & 0x7;

Why is nopSize generated here instead of at the point where we emit the nops?

@@ +400,5 @@
>              return;
> +
> +        countdown -= 1;
> +        if (countdown >= 0)
> +            return;

if (!hardening || --countdown)
    return

would be more like house style.

@@ +2611,5 @@
>      }
>  
>      bool hardening;
> +    int countdown;
> +    size_t nopSize;

Please add comments explaining what these values are for and what the encoding is.
Comment 28 David Mandelin [:dmandelin] 2011-08-29 11:22:55 PDT
Comment on attachment 555188 [details] [diff] [review]
6. Random base offset for assemblies

Review of attachment 555188 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with requested comment added.

::: js/src/assembler/assembler/ARMAssembler.h
@@ +311,5 @@
>              int m_offset : 31;
>              bool m_used : 1;
>          };
>  
> +        void maybeNop() {}

Please add a comment explaining why this has null implementation.
Comment 29 David Mandelin [:dmandelin] 2011-08-29 11:29:05 PDT
Comment on attachment 555189 [details] [diff] [review]
3b. Blinding.

Review of attachment 555189 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but I want a little further discussion on the API before taking this.

::: js/src/assembler/assembler/X86Assembler.h
@@ +1529,5 @@
>          m_formatter.oneByteOp(OP_MOV_GvEv, dst, base, index, scale, offset);
>          postInst();
>      }
>  
> +    void movl_i32r(int imm, RegisterID dst, bool maybeHarden = true)

This seems like it might be a slightly confusing API, although it's clear enough with the context I have (knowing that you're hardening our JITs :-) ).

By default, I'd prefer having a separate method for the hardened case, maybe movl_i32r_blind, or something like that. But maybe this is better here--can you explain why you did it?

As is, I think it would be better to have the parameter be |bool maybeBlind|, and a comment explaining it is blinded only if hardening is on.
Comment 30 David Mandelin [:dmandelin] 2011-08-29 11:32:26 PDT
(In reply to Jan de Mooij from comment #12)
> FWIW, I think V8 only blinds "large" constants. Noticed this a few months
> ago when comparing generated code and wondering why it was xor'ing this
> large integer in the loop test... Not saying that we have to follow them but
> it may help reduce the perf loss because small constants are probably much
> more common.

I missed this comment because I was just going down the list hitting reviews. But this sounds like a great thing to try--we should avoid regressions if we can, and the small constants be a far harder target for attack.
Comment 31 David Anderson [:dvander] 2011-08-29 11:34:18 PDT
v8 only blinds constants in its untyped compiler, as far as I can tell
Comment 32 David Mandelin [:dmandelin] 2011-08-29 11:37:36 PDT
Comment on attachment 555191 [details] [diff] [review]
NanoJIT hardening.

Review of attachment 555191 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jstracer.cpp
@@ +2839,5 @@
> +            int candidate = rand();
> +            if (candidate < limit)
> +                return candidate;
> +        }
> +    }

What values is this called with? It seems to me that if limit is small, it could take a long time. Could you do a simple analysis of the EV of number of rounds and probability it will be big? If there's a min limit, this should assert harder on limit; e.g. limit == 1 is probably too small.
Comment 33 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 11:55:01 PDT
dveditz brought up that it would also be useful to have a way of preffing off JIT hardening for maximizing reproducibility -- shouldn't be difficult, I'll make a patch today on top of this stack.
Comment 34 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 16:14:33 PDT
Created attachment 556700 [details] [diff] [review]
5. NOP insertion.

Addresses review points. Note that fakeRand is replaced later in the stack with the real rand function.
Comment 35 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 16:16:03 PDT
Created attachment 556702 [details] [diff] [review]
NanoJIT hardening.

Future-proofs bad usage with an assertion.
Comment 36 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 16:25:46 PDT
Created attachment 556707 [details] [diff] [review]
3b. Blinding.

You're right, the recursion/default-param was gross. This simply breaks it up.
Comment 37 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 16:27:21 PDT
Created attachment 556708 [details] [diff] [review]
3b. Blinding.

qref
Comment 38 Chris Leary [:cdleary] (not checking bugmail) 2011-08-29 16:52:06 PDT
Created attachment 556715 [details] [diff] [review]
Add JSOPTION_SOFTEN.

Alloc behavior and RNG seed is per-allocator so that it can be set differently on independent threads.
Comment 39 David Mandelin [:dmandelin] 2011-08-30 07:05:25 PDT
Comment on attachment 556702 [details] [diff] [review]
NanoJIT hardening.

Review of attachment 556702 [details] [diff] [review]:
-----------------------------------------------------------------

I just hope the future-proofing isn't too strict. But it will be easy enough to find that out.
Comment 40 Chris Leary [:cdleary] (not checking bugmail) 2011-08-30 14:33:04 PDT
Talked with Brian about coordinating to give TI (bug 608741) time to bake on trunk before intentionally introducing nondeterminism. I'm going to land this pref'd out once the JSOPTION_SOFTEN patch gets r+. When Brian feels comfortable he'll give me the word to flip the pref on.
Comment 41 Chris Leary [:cdleary] (not checking bugmail) 2011-08-30 15:29:04 PDT
Created attachment 557017 [details] [diff] [review]
Add JSOPTION_SOFTEN, disabled by default.

Disabled by default in libpref.
Comment 42 Chris Leary [:cdleary] (not checking bugmail) 2011-08-30 16:40:45 PDT
Comment on attachment 557017 [details] [diff] [review]
Add JSOPTION_SOFTEN, disabled by default.

Whoops, dropped the r?
Comment 43 Chris Leary [:cdleary] (not checking bugmail) 2011-09-15 12:23:42 PDT
Rebasing over type inference now in order to land.
Comment 44 Chris Leary [:cdleary] (not checking bugmail) 2011-12-09 16:58:02 PST
Articles like this are the price I pay for not landing:

http://arstechnica.com/business/news/2011/12/chrome-sandboxing-makes-it-the-most-secure-browser-vendor-study-claims.ars

Going to ask IT for a dedicated WinXP box on Monday to work through the WinXP-specific issues with bug 700822 in order to unblock everything else. Landing these will be my top priority after bug 686927 lands on IonMonkey (est: ~1.5 weeks).
Comment 45 ch-bugzilla 2011-12-22 04:08:47 PST
I'd like to draw attention to this paper[0] by a group of European researchers on specialized code generation techniques designed to defeat ROP attacks by eliminating usable gadgets from compiled code. There may be some ideas here you can use for further JIT hardening.

[0]
http://iseclab.org/papers/gfree.pdf
Comment 46 John Jensen 2012-02-23 12:44:07 PST
Hi Chris, did this land?
Comment 47 Alon Zakai (:azakai) 2012-03-09 14:27:24 PST
I've heard this landed. Does anyone know if we can close this bug?
Comment 48 Johnathan Nightingale [:johnath] 2012-03-26 10:30:37 PDT
(Re-poke?) cdleary - are we done here?
Comment 49 Johnathan Nightingale [:johnath] 2012-03-26 10:32:49 PDT
(In reply to Johnathan Nightingale [:johnath] from comment #48)
> (Re-poke?) cdleary - are we done here?

http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/init/all.js#669 would seem to suggest yes?
Comment 50 John Jensen 2012-04-05 15:28:54 PDT
Hi there -- to follow up from Johnath's question -- are we done with this work?
Comment 51 David Mandelin [:dmandelin] 2012-04-05 16:35:25 PDT
(In reply to John Jensen from comment #50)
> Hi there -- to follow up from Johnath's question -- are we done with this
> work?

I think there are potentially more things to turn on (e.g., nop insertion), but instead of doing that, I'm inclined to wait for IonMonkey, and reformulate a JIT hardening strategy to work with IM.
Comment 52 Johnathan Nightingale [:johnath] 2012-04-10 14:57:38 PDT
(In reply to David Mandelin from comment #51)
> I think there are potentially more things to turn on (e.g., nop insertion),
> but instead of doing that, I'm inclined to wait for IonMonkey, and
> reformulate a JIT hardening strategy to work with IM.

Ballpark timeline? I get the sense from your reply that you don't think the value outweighs the cost for putting more hardening in now, but we know it's a place that security writers, if not researchers, are sniffing around so I want to make sure we don't write off easy wins now against a better version in ionmonkey that won't be available for a while.
Comment 53 David Mandelin [:dmandelin] 2012-04-10 17:49:24 PDT
(In reply to Johnathan Nightingale [:johnath] from comment #52)
> (In reply to David Mandelin from comment #51)
> > I think there are potentially more things to turn on (e.g., nop insertion),
> > but instead of doing that, I'm inclined to wait for IonMonkey, and
> > reformulate a JIT hardening strategy to work with IM.
> 
> Ballpark timeline? I get the sense from your reply that you don't think the
> value outweighs the cost for putting more hardening in now, but we know it's
> a place that security writers, if not researchers, are sniffing around so I
> want to make sure we don't write off easy wins now against a better version
> in ionmonkey that won't be available for a while.

Ballpark landing timeline is a couple of months.

I'm also disinclined to work too hard on this right now because it's really not obvious how valuable various hardening measures are for actual security--it seems like the marketing value is most of it at this point.

We could at least try turning on nop insertion, as long as the code is in fact all there and in shape. I think the main concern was about what that would do to topcrash debuggability, but I don't think we are actively debugging jitcode we get back anyway. So we just need to check if it works and perf is OK. I won't assign it a high importance, but I did put it on my short-term to-do list.
Comment 54 Johnathan Nightingale [:johnath] 2012-04-11 13:13:57 PDT
Makes sense, thanks.
Comment 55 JK 2012-09-16 01:40:45 PDT
Windows 8 apparently applies ASLR on VirtualAlloc.
Comment 56 Till Schneidereit [:till] 2013-03-27 04:41:57 PDT
Mass-reassigning cdleary's bugs to default. He won't work on any of them, anymore. I guess, at least.

@cdleary: shout if you take issue with this.
Comment 57 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2013-03-27 06:46:20 PDT
David, do you know what the next steps are for this bug?
Comment 58 David Anderson [:dvander] 2013-03-28 12:20:54 PDT
This bug has bitrotted too much to be useful by now, especially with the upcoming removal of JM, it's not worth rebasing. As Dave said in comment #53, the value here is mostly marketing. No optimizing JIT that I know of takes the performance hit required to implement these hardening features.

V8 does some kind of limited constant blinding in their baseline JIT only. We could always consider these things for our baseline JIT, for marketing purposes. The only thing that will actually make the JIT secure is a content sandbox.

Note You need to log in before you can comment on or make changes to this bug.