Add masking of stores for protection against Spectre.

chakra-core · May 8, 2018 · e664e18 · e664e18
1 parent eb4b00b
commit e664e18
Show file tree

Hide file tree

Showing 9 changed files with 279 additions and 61 deletions.
diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp
diff --git a/lib/Backend/Lower.h b/lib/Backend/Lower.h
@@ -649,7 +649,7 @@ class Lowerer
     IR::Instr *     LowerSlotArrayCheck(IR::Instr * instr);
     void            InsertSlotArrayCheck(IR::Instr * instr, StackSym * dstSym, uint32 slotId);
     void            InsertFrameDisplayCheck(IR::Instr * instr, StackSym * dstSym, FrameDisplayCheckRecord * record);
-    static void     InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr);
+    static void     InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr, bool isForStore);
 
     IR::RegOpnd *   LoadIndexFromLikelyFloat(IR::RegOpnd *indexOpnd, const bool skipNegativeCheck, IR::LabelInstr *const notTaggedIntLabel, IR::LabelInstr *const negativeLabel, IR::Instr *const insertBeforeInstr);
 

diff --git a/lib/Backend/LowerMDShared.cpp b/lib/Backend/LowerMDShared.cpp
@@ -7149,7 +7149,7 @@ bool LowererMD::GenerateObjectTest(IR::Opnd * opndSrc, IR::Instr * insertInstr,
         // JNE $labelHelper
         IR::BranchInstr* branchInstr = IR::BranchInstr::New(Js::OpCode::JNE, labelTarget, this->m_func);
         insertInstr->InsertBefore(branchInstr);
-        InsertObjectPoison(opndSrc, branchInstr, insertInstr);
+        InsertObjectPoison(opndSrc, branchInstr, insertInstr, false);
     }
     return true;
 }
@@ -9464,9 +9464,9 @@ LowererMD::LowerTypeof(IR::Instr * typeOfInstr)
 }
 
 void
-LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr)
+LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr, bool isForStore)
 {
-    if (CONFIG_FLAG_RELEASE(PoisonObjects))
+    if ((isForStore && CONFIG_FLAG_RELEASE(PoisonObjectsForStores)) || (!isForStore && CONFIG_FLAG_RELEASE(PoisonObjectsForLoads)))
     {
         Js::OpCode opcode;
         if (branchInstr->m_opcode == Js::OpCode::JNE)
@@ -9475,7 +9475,7 @@ LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchIns
         }
         else
         {
-            AssertOrFailFast(branchInstr->m_opcode == Js::OpCode::JEQ);
+            AssertOrFailFastMsg(branchInstr->m_opcode == Js::OpCode::JEQ, "Unexpected branch type in InsertObjectPoison preceeding instruction");
             opcode = Js::OpCode::CMOVE;
         }
         AssertOrFailFast(branchInstr->m_prev->m_opcode == Js::OpCode::CMP || branchInstr->m_prev->m_opcode == Js::OpCode::TEST);

diff --git a/lib/Backend/LowerMDShared.h b/lib/Backend/LowerMDShared.h
@@ -248,7 +248,7 @@ class LowererMD
             void            GenerateIsJsObjectTest(IR::RegOpnd* instanceReg, IR::Instr* insertInstr, IR::LabelInstr* labelHelper);
             void            LowerTypeof(IR::Instr * typeOfInstr);
 
-     static void            InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr);
+     static void            InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr, bool isForStore);
 public:
             //
             // These methods are simply forwarded to lowererMDArch

diff --git a/lib/Backend/arm64/LowerMD.cpp b/lib/Backend/arm64/LowerMD.cpp
@@ -7954,9 +7954,9 @@ LowererMD::LowerTypeof(IR::Instr* typeOfInstr)
 }
 
 void
-LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr)
+LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr, bool isForStore)
 {
-    if (CONFIG_FLAG_RELEASE(PoisonObjects))
+    if ((isForStore && CONFIG_FLAG_RELEASE(PoisonObjectsForStores)) || (!isForStore && CONFIG_FLAG_RELEASE(PoisonObjectsForLoads)))
     {
         Js::OpCode opcode;
         if (branchInstr->m_opcode == Js::OpCode::BNE)
@@ -7965,7 +7965,7 @@ LowererMD::InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchIns
         }
         else
         {
-            AssertOrFailFast(branchInstr->m_opcode == Js::OpCode::BEQ);
+            AssertOrFailFastMsg(branchInstr->m_opcode == Js::OpCode::BEQ, "Unexpected branch type in InsertObjectPoison preceeding instruction");
             opcode = Js::OpCode::CSELNE;
         }
         AssertOrFailFast(branchInstr->m_prev->m_opcode == Js::OpCode::SUBS || branchInstr->m_prev->m_opcode == Js::OpCode::ANDS);

diff --git a/lib/Backend/arm64/LowerMD.h b/lib/Backend/arm64/LowerMD.h
@@ -270,7 +270,8 @@ class LowererMD
 
             void                GenerateMemInit(IR::RegOpnd * opnd, int32 offset, size_t value, IR::Instr * insertBeforeInstr, bool isZeroed = false);
 
-            static void            InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr);
+            static void            InsertObjectPoison(IR::Opnd* poisonedOpnd, IR::BranchInstr* branchInstr, IR::Instr* insertInstr, bool isForStore);
+
 private:
     static  IR::Instr *     ChangeToAssign(IR::Instr * instr, IRType destType);
     void GenerateFlagInlineCacheCheckForGetterSetter(

diff --git a/lib/Common/ConfigFlagsList.h b/lib/Common/ConfigFlagsList.h
@@ -495,7 +495,13 @@ PHASE(All)
 #define DEFAULT_CONFIG_PoisonFloatArrayLoad (true)
 #define DEFAULT_CONFIG_PoisonTypedArrayLoad (true)
 #define DEFAULT_CONFIG_PoisonStringLoad (true)
-#define DEFAULT_CONFIG_PoisonObjects (true)
+#define DEFAULT_CONFIG_PoisonObjectsForLoads (true)
+
+#define DEFAULT_CONFIG_PoisonVarArrayStore (true)
+#define DEFAULT_CONFIG_PoisonIntArrayStore (true)
+#define DEFAULT_CONFIG_PoisonFloatArrayStore (true)
+#define DEFAULT_CONFIG_PoisonTypedArrayStore (true)
+#define DEFAULT_CONFIG_PoisonObjectsForStores (true)
 
 #ifdef RECYCLER_PAGE_HEAP
 #define DEFAULT_CONFIG_PageHeap             ((Js::Number) PageHeapMode::PageHeapModeOff)
@@ -1244,7 +1250,13 @@ FLAGPR(Boolean, MitigateSpectre, PoisonIntArrayLoad, "Poison loads from Int arra
 FLAGPR(Boolean, MitigateSpectre, PoisonFloatArrayLoad, "Poison loads from Float arrays", DEFAULT_CONFIG_PoisonFloatArrayLoad)
 FLAGPR(Boolean, MitigateSpectre, PoisonTypedArrayLoad, "Poison loads from TypedArrays", DEFAULT_CONFIG_PoisonTypedArrayLoad)
 FLAGPR(Boolean, MitigateSpectre, PoisonStringLoad, "Poison indexed loads from strings", DEFAULT_CONFIG_PoisonStringLoad)
-FLAGPR(Boolean, MitigateSpectre, PoisonObjects, "Poison objects after type checks", DEFAULT_CONFIG_PoisonObjects)
+FLAGPR(Boolean, MitigateSpectre, PoisonObjectsForLoads, "Poison objects after type checks", DEFAULT_CONFIG_PoisonObjectsForLoads)
+
+FLAGPR(Boolean, MitigateSpectre, PoisonVarArrayStore, "Poison stores from Var arrays", DEFAULT_CONFIG_PoisonVarArrayStore)
+FLAGPR(Boolean, MitigateSpectre, PoisonIntArrayStore, "Poison stores from Int arrays", DEFAULT_CONFIG_PoisonIntArrayStore)
+FLAGPR(Boolean, MitigateSpectre, PoisonFloatArrayStore, "Poison stores from Float arrays", DEFAULT_CONFIG_PoisonFloatArrayStore)
+FLAGPR(Boolean, MitigateSpectre, PoisonTypedArrayStore, "Poison stores from TypedArrays", DEFAULT_CONFIG_PoisonTypedArrayStore)
+FLAGPR(Boolean, MitigateSpectre, PoisonObjectsForStores, "Poison objects after type checks", DEFAULT_CONFIG_PoisonObjectsForStores)
 
 FLAGNR(Number,  MinInterpretCount     , "Minimum number of times a function must be interpreted", 0)
 FLAGNR(Number,  MinSimpleJitRunCount  , "Minimum number of times a function must be run in simple jit", 0)

diff --git a/test/typedarray/Uint8ClampedArray2.js b/test/typedarray/Uint8ClampedArray2.js
@@ -0,0 +1,43 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var log = Array(1000000);
+var i = 0;
+
+function test() {
+    var cqjmyu;
+    for (var wetavm = 0; wetavm < 1000; ++wetavm) {
+        cqjmyu = new Uint16Array([1, 1, 1, 1, 1, 1, 1, 1, 1]);
+        cqjmyu_0 = new Uint8ClampedArray(cqjmyu);
+        cqjmyu_0[8] = "5";
+        log[i++] = cqjmyu_0[0];
+    }
+    return cqjmyu[0];
+}
+for(var j =0;j<100;j++) test();
+test();
+test();
+test();
+test();
+test();
+test();
+test();
+test();
+test();
+test();
+test();
+
+var failed = false;
+for(var k = 0; k < i; k++) {
+    if(log[k] != 1) {
+        WScript.Echo("failed at " + k);
+        failed = true;
+        break;
+    }
+}
+if(!failed)
+{
+    WScript.Echo("PASSED");
+}
diff --git a/test/typedarray/rlexe.xml b/test/typedarray/rlexe.xml
@@ -299,6 +299,13 @@ Below test fails with difference in space. Investigate the cause and re-enable t
       <tags>typedarray</tags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>Uint8ClampedArray2.js</files>
+      <tags>typedarray</tags>
+      <compile-flags>-minInterpretCount:1 -maxInterpretCount:1 -off:simpleJit</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>setDifferentTypes.js</files>