[cfi-verify] Validate there are no register clobbers between CFI-check and instruction execution.

Summary:
This patch adds another failure mode for `validateCFIProtection(..)`, wherein any register that affects the indirect control flow instruction is clobbered to between the CFI-check and the instruction's execution.

Also includes a modification to make MCInstrDesc::hasDefOfPhysReg public.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: llvm-commits, pcc, kcc

Differential Revision: https://reviews.llvm.org/D39820

llvm-svn: 318238

include/llvm/MC/MCInstrDesc.h

@@ -173,7 +173,7 @@ public:
   const MCPhysReg *ImplicitDefs; // Registers implicitly defined by this instr
   const MCOperandInfo *OpInfo;   // 'NumOperands' entries about operands
   // Subtarget feature that this is deprecated on, if any
-  // -1 implies this is not deprecated by any single feature. It may still be 
+  // -1 implies this is not deprecated by any single feature. It may still be
   // deprecated due to a "complex" reason, below.
   int64_t DeprecatedFeature;
 
@@ -580,8 +580,6 @@ public:
     return -1;
   }
 
-private:
-
   /// \brief Return true if this instruction defines the specified physical
   /// register, either explicitly or implicitly.
   bool hasDefOfPhysReg(const MCInst &MI, unsigned Reg,

tools/llvm-cfi-verify/lib/FileAnalysis.cpp

@@ -64,6 +64,8 @@ StringRef stringCFIProtectionStatus(CFIProtectionStatus Status) {
     return "FAIL_ORPHANS";
   case CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH:
     return "FAIL_BAD_CONDITIONAL_BRANCH";
+  case CFIProtectionStatus::FAIL_REGISTER_CLOBBERED:
+    return "FAIL_REGISTER_CLOBBERED";
   case CFIProtectionStatus::FAIL_INVALID_INSTRUCTION:
     return "FAIL_INVALID_INSTRUCTION";
   }
@@ -270,9 +272,52 @@ FileAnalysis::validateCFIProtection(const GraphResult &Graph) const {
       return CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH;
   }
 
+  if (indirectCFOperandClobber(Graph) != Graph.BaseAddress)
+    return CFIProtectionStatus::FAIL_REGISTER_CLOBBERED;
+
   return CFIProtectionStatus::PROTECTED;
 }
 
+uint64_t FileAnalysis::indirectCFOperandClobber(const GraphResult &Graph) const {
+  assert(Graph.OrphanedNodes.empty() && "Orphaned nodes should be empty.");
+
+  // Get the set of registers we must check to ensure they're not clobbered.
+  const Instr &IndirectCF = getInstructionOrDie(Graph.BaseAddress);
+  DenseSet<unsigned> RegisterNumbers;
+  for (const auto &Operand : IndirectCF.Instruction) {
+    if (Operand.isReg())
+      RegisterNumbers.insert(Operand.getReg());
+  }
+  assert(RegisterNumbers.size() && "Zero register operands on indirect CF.");
+
+  // Now check all branches to indirect CFs and ensure no clobbering happens.
+  for (const auto &Branch : Graph.ConditionalBranchNodes) {
+    uint64_t Node;
+    if (Branch.IndirectCFIsOnTargetPath)
+      Node = Branch.Target;
+    else
+      Node = Branch.Fallthrough;
+
+    while (Node != Graph.BaseAddress) {
+      const Instr &NodeInstr = getInstructionOrDie(Node);
+      const auto &InstrDesc = MII->get(NodeInstr.Instruction.getOpcode());
+
+      for (unsigned RegNum : RegisterNumbers) {
+        if (InstrDesc.hasDefOfPhysReg(NodeInstr.Instruction, RegNum,
+                                      *RegisterInfo))
+          return Node;
+      }
+
+      const auto &KV = Graph.IntermediateNodes.find(Node);
+      assert((KV != Graph.IntermediateNodes.end()) &&
+             "Could not get next node.");
+      Node = KV->second;
+    }
+  }
+
+  return Graph.BaseAddress;
+}
+
 void FileAnalysis::printInstruction(const Instr &InstrMeta,
                                     raw_ostream &OS) const {
   Printer->printInst(&InstrMeta.Instruction, OS, "", *SubtargetInfo.get());

tools/llvm-cfi-verify/lib/FileAnalysis.h

@@ -59,6 +59,9 @@ enum class CFIProtectionStatus {
   // There is a path to the instruction from a conditional branch that does not
   // properly check the destination for this vcall/icall.
   FAIL_BAD_CONDITIONAL_BRANCH,
+  // One of the operands of the indirect CF instruction is modified between the
+  // CFI-check and execution.
+  FAIL_REGISTER_CLOBBERED,
   // The instruction referenced does not exist. This normally indicates an
   // error in the program, where you try and validate a graph that was created
   // in a different FileAnalysis object.
@@ -145,6 +148,13 @@ public:
   // flow instruction in this file.
   CFIProtectionStatus validateCFIProtection(const GraphResult &Graph) const;
 
+  // Returns the first place the operand register is clobbered between the CFI-
+  // check and the indirect CF instruction execution. If the register is not
+  // modified, returns the address of the indirect CF instruction. The result is
+  // undefined if the provided graph does not fall under either the
+  // FAIL_REGISTER_CLOBBERED or PROTECTED status (see CFIProtectionStatus).
+  uint64_t indirectCFOperandClobber(const GraphResult& Graph) const;
+
   // Prints an instruction to the provided stream using this object's pretty-
   // printers.
   void printInstruction(const Instr &InstrMeta, raw_ostream &OS) const;

tools/llvm-cfi-verify/lib/GraphBuilder.cpp

@@ -301,6 +301,7 @@ void GraphBuilder::buildFlowGraphImpl(const FileAnalysis &Analysis,
     BranchNode.Target = 0;
     BranchNode.Fallthrough = 0;
     BranchNode.CFIProtection = false;
+    BranchNode.IndirectCFIsOnTargetPath = (BranchTarget == Address);
 
     if (BranchTarget == Address)
       BranchNode.Target = Address;

tools/llvm-cfi-verify/lib/GraphBuilder.h

@@ -59,6 +59,7 @@ struct ConditionalBranchNode {
   //    is a CFI trap, and...
   //  - The exit point of the other basic block is an undirect CF instruction.
   bool CFIProtection;
+  bool IndirectCFIsOnTargetPath;
 };
 
 // The canonical graph result structure returned by GraphBuilder. The members

unittests/tools/llvm-cfi-verify/FileAnalysis.cpp

@@ -741,6 +741,72 @@ TEST_F(BasicFileAnalysisTest, UndefSearchLengthOneTestFarAway) {
   SearchLengthForUndef = PrevSearchLengthForUndef;
 }
 
+TEST_F(BasicFileAnalysisTest, CFIProtectionClobberSinglePathExplicit) {
+  if (!SuccessfullyInitialised)
+    return;
+  Analysis.parseSectionContents(
+      {
+          0x75, 0x02,                         // 0: jne 4 [+2]
+          0x0f, 0x0b,                         // 2: ud2
+          0x48, 0x05, 0x00, 0x00, 0x00, 0x00, // 4: add $0x0, %rax
+          0xff, 0x10,                         // 10: callq *(%rax)
+      },
+      0xDEADBEEF);
+  GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 10);
+  EXPECT_EQ(CFIProtectionStatus::FAIL_REGISTER_CLOBBERED,
+            Analysis.validateCFIProtection(Result));
+}
+
+TEST_F(BasicFileAnalysisTest, CFIProtectionClobberSinglePathExplicit2) {
+  if (!SuccessfullyInitialised)
+    return;
+  Analysis.parseSectionContents(
+      {
+          0x75, 0x02,             // 0: jne 4 [+2]
+          0x0f, 0x0b,             // 2: ud2
+          0x48, 0x83, 0xc0, 0x00, // 4: add $0x0, %rax
+          0xff, 0x10,             // 8: callq *(%rax)
+      },
+      0xDEADBEEF);
+  GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 8);
+  EXPECT_EQ(CFIProtectionStatus::FAIL_REGISTER_CLOBBERED,
+            Analysis.validateCFIProtection(Result));
+}
+
+TEST_F(BasicFileAnalysisTest, CFIProtectionClobberSinglePathImplicit) {
+  if (!SuccessfullyInitialised)
+    return;
+  Analysis.parseSectionContents(
+      {
+          0x75, 0x02,                   // 0: jne 4 [+2]
+          0x0f, 0x0b,                   // 2: ud2
+          0x05, 0x00, 0x00, 0x00, 0x00, // 4: add $0x0, %eax
+          0xff, 0x10,                   // 9: callq *(%rax)
+      },
+      0xDEADBEEF);
+  GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 9);
+  EXPECT_EQ(CFIProtectionStatus::FAIL_REGISTER_CLOBBERED,
+            Analysis.validateCFIProtection(Result));
+}
+
+TEST_F(BasicFileAnalysisTest, CFIProtectionClobberDualPathImplicit) {
+  if (!SuccessfullyInitialised)
+    return;
+  Analysis.parseSectionContents(
+      {
+          0x75, 0x04, // 0: jne 6 [+4]
+          0x0f, 0x31, // 2: rdtsc (note: affects eax)
+          0xff, 0x10, // 4: callq *(%rax)
+          0x0f, 0x0b, // 6: ud2
+          0x75, 0xf9, // 8: jne 2 [-7]
+          0x0f, 0x0b, // 10: ud2
+      },
+      0xDEADBEEF);
+  GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4);
+  EXPECT_EQ(CFIProtectionStatus::FAIL_REGISTER_CLOBBERED,
+            Analysis.validateCFIProtection(Result));
+}
+
 } // anonymous namespace
 } // end namespace cfi_verify
 } // end namespace llvm