Bug 1433111 - Zero the payload if the Value tag does not match the expected tag. r=jandem
authorNicolas B. Pierron <nicolas.b.pierron@gmail.com>
Fri, 02 Feb 2018 13:39:24 +0000
changeset 403193 048033244192
parent 403192 a96699a3f15a
child 403194 a5b741fac29b
push id33416
push userarchaeopteryx@coole-files.de
push dateFri, 09 Feb 2018 22:32:39 +0000
treeherdermozilla-central@c2cddb0cbb20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1433111
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1433111 - Zero the payload if the Value tag does not match the expected tag. r=jandem
js/public/Value.h
js/src/jit/JitOptions.cpp
js/src/jit/JitOptions.h
js/src/jit/x86-shared/Assembler-x86-shared.h
js/src/jit/x86-shared/BaseAssembler-x86-shared.h
js/src/jit/x86/CodeGenerator-x86.cpp
js/src/jit/x86/Lowering-x86.cpp
js/src/jit/x86/MacroAssembler-x86.h
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -306,16 +306,20 @@ CanonicalizeNaN(double d)
  * ===================
  * To mitigate Spectre attacks, we do the following:
  *
  * - On 64-bit platforms, when unboxing a Value, we XOR the bits with the
  *   expected type tag (instead of masking the payload bits). This guarantees
  *   that toString, toObject, toSymbol will return an invalid pointer (because
  *   some high bits will be set) when called on a Value with a different type
  *   tag.
+ *
+ * - On 32-bit platforms,when unboxing an object/string/symbol Value, we use a
+ *   conditional move (not speculated) to zero the payload register if the type
+ *   doesn't match.
  */
 class MOZ_NON_PARAM alignas(8) Value
 {
   public:
 #if defined(JS_NUNBOX32)
     using PayloadType = uint32_t;
 #elif defined(JS_PUNBOX64)
     using PayloadType = uint64_t;
--- a/js/src/jit/JitOptions.cpp
+++ b/js/src/jit/JitOptions.cpp
@@ -229,16 +229,17 @@ DefaultJitOptions::DefaultJitOptions()
     if (const char* env = getenv(forcedRegisterAllocatorEnv)) {
         forcedRegisterAllocator = LookupRegisterAllocator(env);
         if (!forcedRegisterAllocator.isSome())
             Warn(forcedRegisterAllocatorEnv, env);
     }
 
     SET_DEFAULT(spectreIndexMasking, true);
     SET_DEFAULT(spectreStringMitigations, true);
+    SET_DEFAULT(spectreValueMasking, true);
 
     // Toggles whether unboxed plain objects can be created by the VM.
     SET_DEFAULT(disableUnboxedObjects, false);
 
     // Test whether Atomics are allowed in asm.js code.
     SET_DEFAULT(asmJSAtomicsEnable, false);
 
     // Toggles the optimization whereby offsets are folded into loads and not
--- a/js/src/jit/JitOptions.h
+++ b/js/src/jit/JitOptions.h
@@ -89,18 +89,22 @@ struct DefaultJitOptions
     uint32_t branchPruningEffectfulInstFactor;
     uint32_t branchPruningThreshold;
     uint32_t wasmBatchIonThreshold;
     uint32_t wasmBatchBaselineThreshold;
     mozilla::Maybe<uint32_t> forcedDefaultIonWarmUpThreshold;
     mozilla::Maybe<uint32_t> forcedDefaultIonSmallFunctionWarmUpThreshold;
     mozilla::Maybe<IonRegisterAllocator> forcedRegisterAllocator;
 
+    // Spectre mitigation flags. Each mitigation has its own flag in order to
+    // measure the effectiveness of each mitigation with various proof of
+    // concept.
     bool spectreIndexMasking;
     bool spectreStringMitigations;
+    bool spectreValueMasking;
 
     // The options below affect the rest of the VM, and not just the JIT.
     bool disableUnboxedObjects;
 
     DefaultJitOptions();
     bool isSmallFunction(JSScript* script) const;
     void setEagerCompilation();
     void setCompilerWarmUpThreshold(uint32_t warmUpThreshold);
--- a/js/src/jit/x86-shared/Assembler-x86-shared.h
+++ b/js/src/jit/x86-shared/Assembler-x86-shared.h
@@ -1338,16 +1338,19 @@ class AssemblerX86Shared : public Assemb
             break;
           case Operand::MEM_SCALE:
             masm.addw_rm(src.encoding(), dest.disp(), dest.base(), dest.index(), dest.scale());
             break;
           default:
             MOZ_CRASH("unexpected operand kind");
         }
     }
+    void sbbl(Register src, Register dest) {
+        masm.sbbl_rr(src.encoding(), dest.encoding());
+    }
     void subl(Register src, Register dest) {
         masm.subl_rr(src.encoding(), dest.encoding());
     }
     void subl(const Operand& src, Register dest) {
         switch (src.kind()) {
           case Operand::REG:
             masm.subl_rr(src.reg(), dest.encoding());
             break;
@@ -1625,16 +1628,19 @@ class AssemblerX86Shared : public Assemb
     void andl(const Operand& src, Register dest) {
         switch (src.kind()) {
           case Operand::REG:
             masm.andl_rr(src.reg(), dest.encoding());
             break;
           case Operand::MEM_REG_DISP:
             masm.andl_mr(src.disp(), src.base(), dest.encoding());
             break;
+          case Operand::MEM_SCALE:
+            masm.andl_mr(src.disp(), src.base(), src.index(), src.scale(), dest.encoding());
+            break;
           default:
             MOZ_CRASH("unexpected operand kind");
         }
     }
     void bsrl(const Register& src, const Register& dest) {
         masm.bsrl_rr(src.encoding(), dest.encoding());
     }
     void bsfl(const Register& src, const Register& dest) {
@@ -2109,16 +2115,19 @@ class AssemblerX86Shared : public Assemb
     void push(const Operand& src) {
         switch (src.kind()) {
           case Operand::REG:
             masm.push_r(src.reg());
             break;
           case Operand::MEM_REG_DISP:
             masm.push_m(src.disp(), src.base());
             break;
+          case Operand::MEM_SCALE:
+            masm.push_m(src.disp(), src.base(), src.index(), src.scale());
+            break;
           default:
             MOZ_CRASH("unexpected operand kind");
         }
     }
     void push(Register src) {
         masm.push_r(src.encoding());
     }
     void push(const Address& src) {
--- a/js/src/jit/x86-shared/BaseAssembler-x86-shared.h
+++ b/js/src/jit/x86-shared/BaseAssembler-x86-shared.h
@@ -298,16 +298,21 @@ public:
         m_formatter.immediate32(imm);
     }
 
     void push_m(int32_t offset, RegisterID base)
     {
         spew("push       " MEM_ob, ADDR_ob(offset, base));
         m_formatter.oneByteOp(OP_GROUP5_Ev, offset, base, GROUP5_OP_PUSH);
     }
+    void push_m(int32_t offset, RegisterID base, RegisterID index, int scale)
+    {
+        spew("push       " MEM_obs, ADDR_obs(offset, base, index, scale));
+        m_formatter.oneByteOp(OP_GROUP5_Ev, offset, base, index, scale, GROUP5_OP_PUSH);
+    }
 
     void pop_m(int32_t offset, RegisterID base)
     {
         spew("pop        " MEM_ob, ADDR_ob(offset, base));
         m_formatter.oneByteOp(OP_GROUP1A_Ev, offset, base, GROUP1A_OP_POP);
     }
 
     void push_flags()
@@ -918,16 +923,22 @@ public:
     }
 
     void andl_mr(int32_t offset, RegisterID base, RegisterID dst)
     {
         spew("andl       " MEM_ob ", %s", ADDR_ob(offset, base), GPReg32Name(dst));
         m_formatter.oneByteOp(OP_AND_GvEv, offset, base, dst);
     }
 
+    void andl_mr(int32_t offset, RegisterID base, RegisterID index, int scale, RegisterID dst)
+    {
+        spew("andl       " MEM_obs ", %s", ADDR_obs(offset, base, index, scale), GPReg32Name(dst));
+        m_formatter.oneByteOp(OP_AND_GvEv, offset, base, index, scale, dst);
+    }
+
     void andl_rm(RegisterID src, int32_t offset, RegisterID base)
     {
         spew("andl       %s, " MEM_ob, GPReg32Name(src), ADDR_ob(offset, base));
         m_formatter.oneByteOp(OP_AND_EvGv, offset, base, src);
     }
 
     void andw_rm(RegisterID src, int32_t offset, RegisterID base)
     {
@@ -1227,16 +1238,22 @@ public:
             m_formatter.oneByteOp(OP_GROUP1_EvIb, offset, base, index, scale, GROUP1_OP_OR);
             m_formatter.immediate8s(imm);
         } else {
             m_formatter.oneByteOp(OP_GROUP1_EvIz, offset, base, index, scale, GROUP1_OP_OR);
             m_formatter.immediate16(imm);
         }
     }
 
+    void sbbl_rr(RegisterID src, RegisterID dst)
+    {
+        spew("sbbl       %s, %s", GPReg32Name(src), GPReg32Name(dst));
+        m_formatter.oneByteOp(OP_SBB_GvEv, src, dst);
+    }
+
     void subl_rr(RegisterID src, RegisterID dst)
     {
         spew("subl       %s, %s", GPReg32Name(src), GPReg32Name(dst));
         m_formatter.oneByteOp(OP_SUB_GvEv, src, dst);
     }
 
     void subw_rr(RegisterID src, RegisterID dst)
     {
--- a/js/src/jit/x86/CodeGenerator-x86.cpp
+++ b/js/src/jit/x86/CodeGenerator-x86.cpp
@@ -116,30 +116,38 @@ CodeGeneratorX86::visitBoxFloatingPoint(
     masm.moveValue(TypedOrValueRegister(box->type(), in), out);
 }
 
 void
 CodeGeneratorX86::visitUnbox(LUnbox* unbox)
 {
     // Note that for unbox, the type and payload indexes are switched on the
     // inputs.
+    Operand type = ToOperand(unbox->type());
+    Operand payload = ToOperand(unbox->payload());
+    Register output = ToRegister(unbox->output());
     MUnbox* mir = unbox->mir();
 
     JSValueTag tag = MIRTypeToTag(mir->type());
     if (mir->fallible()) {
-        masm.cmp32(ToOperand(unbox->type()), Imm32(tag));
+        masm.cmp32(type, Imm32(tag));
         bailoutIf(Assembler::NotEqual, unbox->snapshot());
     } else {
 #ifdef DEBUG
         Label ok;
-        masm.branch32(Assembler::Equal, ToOperand(unbox->type()), Imm32(tag), &ok);
+        masm.branch32(Assembler::Equal, type, Imm32(tag), &ok);
         masm.assumeUnreachable("Infallible unbox type mismatch");
         masm.bind(&ok);
 #endif
     }
+
+    // Note: If spectreValueMasking is disabled, then this instruction will
+    // default to a no-op as long as the lowering allocate the same register for
+    // the output and the payload.
+    masm.unboxNonDouble(type, payload, output, ValueTypeFromMIRType(mir->type()));
 }
 
 void
 CodeGeneratorX86::visitCompareB(LCompareB* lir)
 {
     MCompare* mir = lir->mir();
 
     const ValueOperand lhs = ToValue(lir, LCompareB::Lhs);
--- a/js/src/jit/x86/Lowering-x86.cpp
+++ b/js/src/jit/x86/Lowering-x86.cpp
@@ -114,28 +114,39 @@ LIRGeneratorX86::visitUnbox(MUnbox* unbo
         if (unbox->fallible())
             assignSnapshot(lir, unbox->bailoutKind());
         define(lir, unbox);
         return;
     }
 
     // Swap the order we use the box pieces so we can re-use the payload register.
     LUnbox* lir = new(alloc()) LUnbox;
-    lir->setOperand(0, usePayloadInRegisterAtStart(inner));
-    lir->setOperand(1, useType(inner, LUse::ANY));
+    bool reusePayloadReg = !JitOptions.spectreValueMasking ||
+        unbox->type() == MIRType::Int32 ||
+        unbox->type() == MIRType::Boolean;
+    if (reusePayloadReg) {
+        lir->setOperand(0, usePayloadInRegisterAtStart(inner));
+        lir->setOperand(1, useType(inner, LUse::ANY));
+    } else {
+        lir->setOperand(0, usePayload(inner, LUse::REGISTER));
+        lir->setOperand(1, useType(inner, LUse::ANY));
+    }
 
     if (unbox->fallible())
         assignSnapshot(lir, unbox->bailoutKind());
 
     // Types and payloads form two separate intervals. If the type becomes dead
     // before the payload, it could be used as a Value without the type being
     // recoverable. Unbox's purpose is to eagerly kill the definition of a type
     // tag, so keeping both alive (for the purpose of gcmaps) is unappealing.
     // Instead, we create a new virtual register.
-    defineReuseInput(lir, unbox, 0);
+    if (reusePayloadReg)
+        defineReuseInput(lir, unbox, 0);
+    else
+        define(lir, unbox);
 }
 
 void
 LIRGeneratorX86::visitReturn(MReturn* ret)
 {
     MDefinition* opd = ret->getOperand(0);
     MOZ_ASSERT(opd->type() == MIRType::Value);
 
--- a/js/src/jit/x86/MacroAssembler-x86.h
+++ b/js/src/jit/x86/MacroAssembler-x86.h
@@ -675,25 +675,77 @@ class MacroAssemblerX86 : public MacroAs
         }
     }
     void boxNonDouble(JSValueType type, Register src, const ValueOperand& dest) {
         if (src != dest.payloadReg())
             movl(src, dest.payloadReg());
         movl(ImmType(type), dest.typeReg());
     }
 
-    void unboxNonDouble(const ValueOperand& src, Register dest, JSValueType type) {
-        if (src.payloadReg() != dest)
-            movl(src.payloadReg(), dest);
+    void unboxNonDouble(const ValueOperand& src, Register dest, JSValueType type, Register scratch = InvalidReg) {
+        unboxNonDouble(Operand(src.typeReg()), Operand(src.payloadReg()), dest, type, scratch);
+    }
+    void unboxNonDouble(const Operand& tag, const Operand& payload, Register dest, JSValueType type, Register scratch = InvalidReg) {
+        auto movPayloadToDest = [&]() {
+            if (payload.kind() != Operand::REG || !payload.containsReg(dest))
+                movl(payload, dest);
+        };
+        if (!JitOptions.spectreValueMasking) {
+            movPayloadToDest();
+            return;
+        }
+
+        // Spectre mitigation: We zero the payload if the tag does not match the
+        // expected type and if this is a pointer type.
+        if (type == JSVAL_TYPE_INT32 || type == JSVAL_TYPE_BOOLEAN) {
+            movPayloadToDest();
+            return;
+        }
+
+        if (!tag.containsReg(dest) && !payload.containsReg(dest)) {
+            // We zero the destination register and move the payload into it if
+            // the tag corresponds to the given type.
+            xorl(dest, dest);
+            cmpl(Imm32(JSVAL_TYPE_TO_TAG(type)), tag);
+            cmovCCl(Condition::Equal, payload, dest);
+            return;
+        }
+
+        if (scratch == InvalidReg || scratch == dest ||
+            tag.containsReg(scratch) || payload.containsReg(scratch))
+        {
+            // UnboxedLayout::makeConstructorCode calls extractObject with a
+            // scratch register which aliases the tag register, thus we cannot
+            // assert the above condition.
+            scratch = InvalidReg;
+        }
+
+        // The destination register aliases one of the operands. We create a
+        // zero value either in a scratch register or on the stack and use it
+        // to reset the destination register after reading both the tag and the
+        // payload.
+        Operand zero(Address(esp, 0));
+        if (scratch == InvalidReg) {
+            push(Imm32(0));
+        } else {
+            xorl(scratch, scratch);
+            zero = Operand(scratch);
+        }
+        cmpl(Imm32(JSVAL_TYPE_TO_TAG(type)), tag);
+        movPayloadToDest();
+        cmovCCl(Condition::NotEqual, zero, dest);
+        if (scratch == InvalidReg) {
+            addl(Imm32(sizeof(void*)), esp);
+        }
     }
     void unboxNonDouble(const Address& src, Register dest, JSValueType type) {
-        movl(payloadOf(src), dest);
+        unboxNonDouble(tagOf(src), payloadOf(src), dest, type);
     }
     void unboxNonDouble(const BaseIndex& src, Register dest, JSValueType type) {
-        movl(payloadOf(src), dest);
+        unboxNonDouble(tagOf(src), payloadOf(src), dest, type);
     }
     void unboxInt32(const ValueOperand& src, Register dest) {
         unboxNonDouble(src, dest, JSVAL_TYPE_INT32);
     }
     void unboxInt32(const Address& src, Register dest) {
         unboxNonDouble(src, dest, JSVAL_TYPE_INT32);
     }
     void unboxBoolean(const ValueOperand& src, Register dest) {
@@ -766,27 +818,30 @@ class MacroAssemblerX86 : public MacroAs
 
     void notBoolean(const ValueOperand& val) {
         xorl(Imm32(1), val.payloadReg());
     }
 
     // Extended unboxing API. If the payload is already in a register, returns
     // that register. Otherwise, provides a move to the given scratch register,
     // and returns that.
-    Register extractObject(const Address& address, Register scratch) {
-        movl(payloadOf(address), scratch);
-        return scratch;
+    Register extractObject(const Address& address, Register dest) {
+        unboxObject(address, dest);
+        return dest;
     }
     Register extractObject(const ValueOperand& value, Register scratch) {
+        unboxNonDouble(value, value.payloadReg(), JSVAL_TYPE_OBJECT, scratch);
         return value.payloadReg();
     }
     Register extractString(const ValueOperand& value, Register scratch) {
+        unboxNonDouble(value, value.payloadReg(), JSVAL_TYPE_STRING, scratch);
         return value.payloadReg();
     }
     Register extractSymbol(const ValueOperand& value, Register scratch) {
+        unboxNonDouble(value, value.payloadReg(), JSVAL_TYPE_SYMBOL, scratch);
         return value.payloadReg();
     }
     Register extractInt32(const ValueOperand& value, Register scratch) {
         return value.payloadReg();
     }
     Register extractBoolean(const ValueOperand& value, Register scratch) {
         return value.payloadReg();
     }