[llvm] r318238 - [cfi-verify] Validate there are no register clobbers between CFI-check and instruction execution.
Mitch Phillips via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 14 16:35:26 PST 2017
Author: hctim
Date: Tue Nov 14 16:35:26 2017
New Revision: 318238
URL: http://llvm.org/viewvc/llvm-project?rev=318238&view=rev
Log:
[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
Modified:
llvm/trunk/include/llvm/MC/MCInstrDesc.h
llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h
llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.cpp
llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.h
llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp
Modified: llvm/trunk/include/llvm/MC/MCInstrDesc.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrDesc.h?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCInstrDesc.h (original)
+++ llvm/trunk/include/llvm/MC/MCInstrDesc.h Tue Nov 14 16:35:26 2017
@@ -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,
Modified: llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp (original)
+++ llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp Tue Nov 14 16:35:26 2017
@@ -64,6 +64,8 @@ StringRef stringCFIProtectionStatus(CFIP
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(cons
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());
Modified: llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h (original)
+++ llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h Tue Nov 14 16:35:26 2017
@@ -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;
Modified: llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.cpp?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.cpp (original)
+++ llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.cpp Tue Nov 14 16:35:26 2017
@@ -301,6 +301,7 @@ void GraphBuilder::buildFlowGraphImpl(co
BranchNode.Target = 0;
BranchNode.Fallthrough = 0;
BranchNode.CFIProtection = false;
+ BranchNode.IndirectCFIsOnTargetPath = (BranchTarget == Address);
if (BranchTarget == Address)
BranchNode.Target = Address;
Modified: llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.h?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.h (original)
+++ llvm/trunk/tools/llvm-cfi-verify/lib/GraphBuilder.h Tue Nov 14 16:35:26 2017
@@ -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
Modified: llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp?rev=318238&r1=318237&r2=318238&view=diff
==============================================================================
--- llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp (original)
+++ llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp Tue Nov 14 16:35:26 2017
@@ -741,6 +741,72 @@ TEST_F(BasicFileAnalysisTest, UndefSearc
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
More information about the llvm-commits
mailing list