Index: tools/llvm-cfi-verify/lib/FileAnalysis.h =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.h +++ tools/llvm-cfi-verify/lib/FileAnalysis.h @@ -44,8 +44,29 @@ namespace llvm { namespace cfi_verify { +struct GraphResult; + extern bool IgnoreDWARFFlag; +enum CFIProtectionStatus { + // This instruction is protected by CFI. + PROTECTED, + // The instruction is not an indirect control flow instruction, and thus + // shouldn't be protected. + FAIL_NOT_INDIRECT_CF, + // There is a path to the instruction that was unexpected. + FAIL_ORPHANS, + // 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, + // 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. + FAIL_INVALID_INSTRUCTION, +}; + +StringRef stringCFIProtectionStatus(CFIProtectionStatus Status); + // Disassembler and analysis tool for machine code files. Keeps track of non- // sequential control flows, including indirect control flow instructions. class FileAnalysis { @@ -69,12 +90,6 @@ FileAnalysis(const FileAnalysis &) = delete; FileAnalysis(FileAnalysis &&Other) = default; - // Check whether the provided instruction is CFI protected in this file. - // Returns false if this instruction doesn't exist in this file, if it's not - // an indirect control flow instruction, or isn't CFI protected. Returns true - // otherwise. - bool isIndirectInstructionCFIProtected(uint64_t Address) const; - // Returns the instruction at the provided address. Returns nullptr if there // is no instruction at the provided address. const Instr *getInstruction(uint64_t Address) const; @@ -122,19 +137,13 @@ const MCRegisterInfo *getRegisterInfo() const; const MCInstrInfo *getMCInstrInfo() const; const MCInstrAnalysis *getMCInstrAnalysis() const; - symbolize::LLVMSymbolizer &getSymbolizer(); - - // Returns true if this class is using DWARF line tables for elimination. - bool hasLineTableInfo() const; - // Returns the line table information for the range {Address +- - // DWARFSearchRange}. Returns an empty table if the address has no valid line - // table information, or this analysis object has DWARF handling disabled. - DILineInfoTable getLineInfoForAddressRange(uint64_t Address); + // Returns the inlining information for the provided address. + Expected symbolizeInlinedCode(uint64_t Address); - // Returns whether the provided address has valid line information for - // instructions in the range of Address +- DWARFSearchRange. - bool hasValidLineInfoForAddressRange(uint64_t Address); + // Returns whether the provided Graph represents a protected indirect control + // flow instruction in this file. + CFIProtectionStatus validateCFIProtection(const GraphResult &Graph) const; protected: // Construct a blank object with the provided triple and features. Used in Index: tools/llvm-cfi-verify/lib/FileAnalysis.cpp =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.cpp +++ tools/llvm-cfi-verify/lib/FileAnalysis.cpp @@ -54,6 +54,22 @@ "will result in false positives for 'CFI unprotected' instructions."), cl::location(IgnoreDWARFFlag), cl::init(false)); +StringRef stringCFIProtectionStatus(CFIProtectionStatus Status) { + switch (Status) { + case PROTECTED: + return "PROTECTED"; + case FAIL_NOT_INDIRECT_CF: + return "FAIL_NOT_INDIRECT_CF"; + case FAIL_ORPHANS: + return "FAIL_ORPHANS"; + case FAIL_BAD_CONDITIONAL_BRANCH: + return "FAIL_BAD_CONDITIONAL_BRANCH"; + case FAIL_INVALID_INSTRUCTION: + return "FAIL_INVALID_INSTRUCTION"; + } + llvm_unreachable("Attempted to stringify an unknown enum value."); +} + Expected FileAnalysis::Create(StringRef Filename) { // Open the filename provided. Expected> BinaryOrErr = @@ -89,32 +105,6 @@ const SubtargetFeatures &Features) : ObjectTriple(ObjectTriple), Features(Features) {} -bool FileAnalysis::isIndirectInstructionCFIProtected(uint64_t Address) const { - const Instr *InstrMetaPtr = getInstruction(Address); - if (!InstrMetaPtr) - return false; - - const auto &InstrDesc = MII->get(InstrMetaPtr->Instruction.getOpcode()); - - if (!InstrDesc.mayAffectControlFlow(InstrMetaPtr->Instruction, *RegisterInfo)) - return false; - - if (!usesRegisterOperand(*InstrMetaPtr)) - return false; - - auto Flows = GraphBuilder::buildFlowGraph(*this, Address); - - if (!Flows.OrphanedNodes.empty()) - return false; - - for (const auto &BranchNode : Flows.ConditionalBranchNodes) { - if (!BranchNode.CFIProtection) - return false; - } - - return true; -} - const Instr * FileAnalysis::getPrevInstructionSequential(const Instr &InstrMeta) const { std::map::const_iterator KV = @@ -254,7 +244,34 @@ return MIA.get(); } -LLVMSymbolizer &FileAnalysis::getSymbolizer() { return *Symbolizer; } +Expected FileAnalysis::symbolizeInlinedCode(uint64_t Address) { + assert(Symbolizer != nullptr && "Symbolizer is invalid."); + return Symbolizer->symbolizeInlinedCode(Object->getFileName(), Address); +} + +CFIProtectionStatus +FileAnalysis::validateCFIProtection(const GraphResult &Graph) const { + const Instr *InstrMetaPtr = getInstruction(Graph.BaseAddress); + if (!InstrMetaPtr) + return FAIL_INVALID_INSTRUCTION; + + const auto &InstrDesc = MII->get(InstrMetaPtr->Instruction.getOpcode()); + if (!InstrDesc.mayAffectControlFlow(InstrMetaPtr->Instruction, *RegisterInfo)) + return FAIL_NOT_INDIRECT_CF; + + if (!usesRegisterOperand(*InstrMetaPtr)) + return FAIL_NOT_INDIRECT_CF; + + if (!Graph.OrphanedNodes.empty()) + return FAIL_ORPHANS; + + for (const auto &BranchNode : Graph.ConditionalBranchNodes) { + if (!BranchNode.CFIProtection) + return FAIL_BAD_CONDITIONAL_BRANCH; + } + + return PROTECTED; +} Error FileAnalysis::initialiseDisassemblyMembers() { std::string TripleName = ObjectTriple.getTriple(); Index: tools/llvm-cfi-verify/llvm-cfi-verify.cpp =================================================================== --- tools/llvm-cfi-verify/llvm-cfi-verify.cpp +++ tools/llvm-cfi-verify/llvm-cfi-verify.cpp @@ -18,6 +18,7 @@ //===----------------------------------------------------------------------===// #include "lib/FileAnalysis.h" +#include "lib/GraphBuilder.h" #include "llvm/BinaryFormat/ELF.h" #include "llvm/Support/CommandLine.h" @@ -46,13 +47,15 @@ uint64_t ExpectedUnprotected = 0; uint64_t UnexpectedUnprotected = 0; - symbolize::LLVMSymbolizer &Symbolizer = Analysis.getSymbolizer(); std::map BlameCounter; for (uint64_t Address : Analysis.getIndirectInstructions()) { const auto &InstrMeta = Analysis.getInstructionOrDie(Address); + GraphResult Graph = GraphBuilder::buildFlowGraph(Analysis, Address); - bool CFIProtected = Analysis.isIndirectInstructionCFIProtected(Address); + CFIProtectionStatus ProtectionStatus = + Analysis.validateCFIProtection(Graph); + bool CFIProtected = (ProtectionStatus == PROTECTED); if (CFIProtected) outs() << "P "; @@ -72,7 +75,7 @@ continue; } - auto InliningInfo = Symbolizer.symbolizeInlinedCode(InputFilename, Address); + auto InliningInfo = Analysis.symbolizeInlinedCode(Address); if (!InliningInfo || InliningInfo->getNumberOfFrames() == 0) { errs() << "Failed to symbolise " << format_hex(Address, 2) << " with line tables from " << InputFilename << "\n"; Index: unittests/tools/llvm-cfi-verify/FileAnalysis.cpp =================================================================== --- unittests/tools/llvm-cfi-verify/FileAnalysis.cpp +++ unittests/tools/llvm-cfi-verify/FileAnalysis.cpp @@ -493,10 +493,14 @@ 0x75, 0x00, // 3: jne 5 [+0] }, 0xDEADBEEF); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 1)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADC0DE)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF); + EXPECT_EQ(FAIL_NOT_INDIRECT_CF, Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 1); + EXPECT_EQ(FAIL_NOT_INDIRECT_CF, Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(FAIL_NOT_INDIRECT_CF, Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADC0DE); + EXPECT_EQ(FAIL_INVALID_INSTRUCTION, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicFallthroughToUd2) { @@ -509,7 +513,8 @@ 0xff, 0x10, // 4: callq *(%rax) }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicJumpToUd2) { @@ -522,7 +527,8 @@ 0x0f, 0x0b, // 4: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 2); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathUd2) { @@ -538,7 +544,8 @@ 0x0f, 0x0b, // 9: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathSingleUd2) { @@ -553,7 +560,8 @@ 0x0f, 0x0b, // 7: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualFailLimitUpwards) { @@ -574,7 +582,8 @@ SearchLengthForConditionalBranch; SearchLengthForConditionalBranch = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 6)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 6); + EXPECT_EQ(FAIL_ORPHANS, Analysis.validateCFIProtection(Result)); SearchLengthForConditionalBranch = PrevSearchLengthForConditionalBranch; } @@ -596,7 +605,8 @@ uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 2); + EXPECT_EQ(FAIL_BAD_CONDITIONAL_BRANCH, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -612,7 +622,8 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(FAIL_ORPHANS, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionWithUnconditionalJumpInFallthrough) { @@ -626,7 +637,8 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionComplexExample) { @@ -653,7 +665,8 @@ 0xDEADBEEF); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 5; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 9)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 9); + EXPECT_EQ(FAIL_ORPHANS, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -670,7 +683,8 @@ 0x688118); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x68811d)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0x68811d); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -699,11 +713,14 @@ 0x775e0e); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(FAIL_BAD_CONDITIONAL_BRANCH, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = 2; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = 3; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(PROTECTED, Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; }