Implement NaCl sandboxing of function calls

[sstankovic]

Summary

[mips] Implement NaCl sandboxing of function calls:

• Add masking instructions before indirect calls (in MC layer).
• Align call + branch delay to the bundle end (in MC layer).

[mseaborn]

lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp

45 ↗

(On Diff #7713)

Please comment these three fields. "Call" and "IsIndirectCall" are only defined if SandboxingCall is true -- so please comment that, and put SandboxingCall first.

46 ↗

(On Diff #7713)

Maybe something like "Pending" or "Saved" rather than "Sandboxing" would better suggest what's going on here?

166 ↗

(On Diff #7713)

Nit: drop the "else" to be consistent with the rest of the function -- it isn't needed if the "if" block returns.

167 ↗

(On Diff #7713)

Maybe comment this block: "Save up the call to emit after we see the branch delay instruction that follows"?

test/MC/Mips/nacl-mask.s

12 ↗

(On Diff #7713)

Is the reason for adding this that EmitBundleLock(true) complains if an alignment hasn't been set already? Or does the test work without adding this .align?

[sstankovic]

Patch is updated.

[sstankovic]

test/MC/Mips/nacl-mask.s

12 ↗

(On Diff #7713)

No, I added this because llvm-mc doesn't call AsmPrinter::runOnMachineFunction, which means that functions are not bundle-aligned. Test expects that function start is bundle-aligned.

[mseaborn]

LGTM, thanks

lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp

135 ↗

(On Diff #7744)

You could check PendingCall here (and for isIndirectJump above and isCall below) and report_fatal_error if it's set, since this would be an invalid instruction in the branch delay slot. Otherwise you might silently emit a mismatched bundle lock/unlock.

152 ↗

(On Diff #7744)

Nice simplification from the previous version.

[sstankovic]

Patch is updated.

[sstankovic]

Closed by commit rL203606 (authored by @sstankovic).

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp

 //===-- MipsNaClELFStreamer.cpp - ELF Object Output for Mips NaCl ---------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements MCELFStreamer for Mips NaCl.  It emits .o object files
 // as required by NaCl's SFI sandbox.  It inserts address-masking instructions
 // before dangerous control-flow and memory access instructions.  It inserts
 // address-masking instructions after instructions that change the stack
 // pointer.  It ensures that the mask and the dangerous instruction are always
-// emitted in the same bundle.
+// emitted in the same bundle.  It aligns call + branch delay to the bundle end,
+// so that return address is always aligned to the start of next bundle.
 //
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "mips-mc-nacl"
 
 #include "Mips.h"
 #include "MipsMCNaCl.h"
 #include "llvm/MC/MCELFStreamer.h"
 
 using namespace llvm;
 
 namespace {
 
 const unsigned IndirectBranchMaskReg = Mips::T6;
 const unsigned LoadStoreStackMaskReg = Mips::T7;
 
 /// Extend the generic MCELFStreamer class so that it can mask dangerous
 /// instructions.
 
 class MipsNaClELFStreamer : public MCELFStreamer {
 public:
   MipsNaClELFStreamer(MCContext &Context, MCAsmBackend &TAB, raw_ostream &OS,
                       MCCodeEmitter *Emitter)
-    : MCELFStreamer(Context, TAB, OS, Emitter) {}
+    : MCELFStreamer(Context, TAB, OS, Emitter), PendingCall(false) {}
 
   ~MipsNaClELFStreamer() {}
 
 private:
+  // Whether we started the sandboxing sequence for calls.  Calls are bundled
+  // with branch delays and aligned to the bundle end.
+  bool PendingCall;
+
   bool isIndirectJump(const MCInst &MI) {
     return MI.getOpcode() == Mips::JR || MI.getOpcode() == Mips::RET;
   }
 
   bool isStackPointerFirstOperand(const MCInst &MI) {
     return (MI.getNumOperands() > 0 && MI.getOperand(0).isReg()
             && MI.getOperand(0).getReg() == Mips::SP);
   }
 
+  bool isCall(unsigned Opcode, bool *IsIndirectCall) {
+    *IsIndirectCall = false;
+
+    switch (Opcode) {
+    default:
+      return false;
+
+    case Mips::JAL:
+    case Mips::BAL_BR:
+    case Mips::BLTZAL:
+    case Mips::BGEZAL:
+      return true;
+
+    case Mips::JALR:
+      *IsIndirectCall = true;
+      return true;
+    }
+  }
+
   void emitMask(unsigned AddrReg, unsigned MaskReg,
                 const MCSubtargetInfo &STI) {
     MCInst MaskInst;
     MaskInst.setOpcode(Mips::AND);
     MaskInst.addOperand(MCOperand::CreateReg(AddrReg));
     MaskInst.addOperand(MCOperand::CreateReg(AddrReg));
     MaskInst.addOperand(MCOperand::CreateReg(MaskReg));
     MCELFStreamer::EmitInstruction(MaskInst, STI);
private:
   }
 
 public:
   /// This function is the one used to emit instruction data into the ELF
   /// streamer.  We override it to mask dangerous instructions.
   virtual void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) {
     // Sandbox indirect jumps.
     if (isIndirectJump(Inst)) {
+      if (PendingCall)
+        report_fatal_error("Dangerous instruction in branch delay slot!");
       sandboxIndirectJump(Inst, STI);
       return;
     }
 
     // Sandbox loads, stores and SP changes.
     unsigned AddrIdx;
     bool IsStore;
     bool IsMemAccess = isBasePlusOffsetMemoryAccess(Inst.getOpcode(), &AddrIdx,
                                                     &IsStore);
     bool IsSPFirstOperand = isStackPointerFirstOperand(Inst);
     if (IsMemAccess || IsSPFirstOperand) {
+      if (PendingCall)
+        report_fatal_error("Dangerous instruction in branch delay slot!");
+
       bool MaskBefore = (IsMemAccess
                          && baseRegNeedsLoadStoreMask(Inst.getOperand(AddrIdx)
                                                           .getReg()));
       bool MaskAfter = IsSPFirstOperand && !IsStore;
       if (MaskBefore || MaskAfter)
         sandboxLoadStoreStackChange(Inst, AddrIdx, STI, MaskBefore, MaskAfter);
       else
         MCELFStreamer::EmitInstruction(Inst, STI);
       return;
     }
 
+    // Sandbox calls by aligning call and branch delay to the bundle end.
+    // For indirect calls, emit the mask before the call.
+    bool IsIndirectCall;
+    if (isCall(Inst.getOpcode(), &IsIndirectCall)) {
+      if (PendingCall)
+        report_fatal_error("Dangerous instruction in branch delay slot!");
+
+      // Start the sandboxing sequence by emitting call.
+      EmitBundleLock(true);
+      if (IsIndirectCall) {
+        unsigned TargetReg = Inst.getOperand(1).getReg();
+        emitMask(TargetReg, IndirectBranchMaskReg, STI);
+      }
+      MCELFStreamer::EmitInstruction(Inst, STI);
+      PendingCall = true;
+      return;
+    }
+    if (PendingCall) {
+      // Finish the sandboxing sequence by emitting branch delay.
+      MCELFStreamer::EmitInstruction(Inst, STI);
+      EmitBundleUnlock();
+      PendingCall = false;
+      return;
+    }
+
     // None of the sandboxing applies, just emit the instruction.
     MCELFStreamer::EmitInstruction(Inst, STI);
   }
 };
 
 } // end anonymous namespace
 
 namespace llvm {

llvm/trunk/test/MC/Mips/nacl-mask.s

 # RUN: llvm-mc -filetype=obj -triple=mipsel-unknown-nacl %s \
 # RUN:  | llvm-objdump -triple mipsel -disassemble -no-show-raw-insn - \
 # RUN:  | FileCheck %s
 
 # This test tests that address-masking sandboxing is added when given assembly
 # input.
 
 
 # Test that address-masking sandboxing is added before indirect branches and
 # returns.
 
+  .align  4
 test1:
   .set  noreorder
 
         jr  $a0
         nop
         jr  $ra
         nop
 

 
 # CHECK:         and     $ra, $ra, $14
 # CHECK-NEXT:    jr      $ra
 
 
 
 # Test that address-masking sandboxing is added before load instructions.
 
+  .align  4
 test2:
   .set  noreorder
 
         lb      $4, 0($1)
         nop
         lbu     $4, 0($2)
         lh      $4, 0($3)
         lhu     $1, 0($4)

 # CHECK:         lw      $4, 0($sp)
 # CHECK-NOT:     and
 # CHECK:         lw      $4, 0($24)
 
 
 
 # Test that address-masking sandboxing is added before store instructions.
 
+  .align  4
 test3:
   .set  noreorder
 
         sb      $4, 0($1)
         nop
         sh      $4, 0($2)
         sw      $4, 0($3)
         swc1    $f0, 0($4)

 # CHECK-NOT:     and
 # CHECK:         sw      $4, 0($24)
 
 
 
 # Test that address-masking sandboxing is added after instructions that change
 # stack pointer.
 
+  .align  4
 test4:
   .set  noreorder
 
         addiu   $sp, $sp, 24
         nop
         addu    $sp, $sp, $1
         lw      $sp, 0($2)
         lw      $sp, 123($sp)

 # CHECK-NEXT:    and     $sp, $sp, $15
 
 # For stores where $sp is destination and base, check that mask is added neither
 # before nor after.
 
 # CHECK-NOT:     and
 # CHECK:         sw      $sp, 123($sp)
 # CHECK-NOT:     and
+
+
+
+# Test that call + branch delay is aligned at bundle end.  Test that mask is
+# added before indirect calls.
+
+  .align  4
+test5:
+  .set  noreorder
+
+        jal func1
+        addiu $4, $zero, 1
+
+        nop
+        bal func2
+        addiu $4, $zero, 2
+
+        nop
+        nop
+        bltzal $t1, func3
+        addiu $4, $zero, 3
+
+        nop
+        nop
+        nop
+        bgezal $t2, func4
+        addiu $4, $zero, 4
+
+        jalr $t9
+        addiu $4, $zero, 5
+
+# CHECK-LABEL:   test5:
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        jal
+# CHECK-NEXT:        addiu   $4, $zero, 1
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        bal
+# CHECK-NEXT:        addiu   $4, $zero, 2
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        bltzal
+# CHECK-NEXT:        addiu   $4, $zero, 3
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        bgezal
+# CHECK-NEXT:        addiu   $4, $zero, 4
+
+# CHECK-NEXT:        nop
+# CHECK-NEXT:        and     $25, $25, $14
+# CHECK-NEXT:        jalr    $25
+# CHECK-NEXT:        addiu   $4, $zero, 5