Index: tools/llvm-cfi-verify/CMakeLists.txt =================================================================== --- tools/llvm-cfi-verify/CMakeLists.txt +++ tools/llvm-cfi-verify/CMakeLists.txt @@ -5,6 +5,7 @@ AllTargetsDisassemblers AllTargetsInfos CFIVerify + DebugInfoDWARF MC MCParser Object Index: tools/llvm-cfi-verify/LLVMBuild.txt =================================================================== --- tools/llvm-cfi-verify/LLVMBuild.txt +++ tools/llvm-cfi-verify/LLVMBuild.txt @@ -19,4 +19,4 @@ type = Tool name = llvm-cfi-verify parent = Tools -required_libraries = all-targets MC MCDisassembler MCParser Support +required_libraries = all-targets DebugInfoDWARF MC MCDisassembler MCParser Support Index: tools/llvm-cfi-verify/lib/FileAnalysis.h =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.h +++ tools/llvm-cfi-verify/lib/FileAnalysis.h @@ -12,6 +12,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/BinaryFormat/ELF.h" +#include "llvm/DebugInfo/DWARF/DWARFContext.h" #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCDisassembler/MCDisassembler.h" @@ -43,6 +44,8 @@ namespace llvm { namespace cfi_verify { +extern bool IgnoreDWARF; + // Disassembler and analysis tool for machine code files. Keeps track of non- // sequential control flows, including indirect control flow instructions. class FileAnalysis { @@ -66,6 +69,12 @@ 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; @@ -114,6 +123,8 @@ const MCInstrInfo *getMCInstrInfo() const; const MCInstrAnalysis *getMCInstrAnalysis() const; + DWARFContext *getDWARF(); + protected: // Construct a blank object with the provided triple and features. Used in // testing, where a sub class will dependency inject protected methods to @@ -156,8 +167,12 @@ std::unique_ptr MIA; std::unique_ptr Printer; + // DWARF debug information. + std::unique_ptr DWARF; + // A mapping between the virtual memory address to the instruction metadata - // struct. + // struct. TODO(hctim): Reimplement this as a sorted vector to avoid per- + // insertion allocation. std::map Instructions; // Contains a mapping between a specific address, and a list of instructions Index: tools/llvm-cfi-verify/lib/FileAnalysis.cpp =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.cpp +++ tools/llvm-cfi-verify/lib/FileAnalysis.cpp @@ -8,8 +8,10 @@ //===----------------------------------------------------------------------===// #include "FileAnalysis.h" +#include "GraphBuilder.h" #include "llvm/BinaryFormat/ELF.h" +#include "llvm/DebugInfo/DWARF/DWARFContext.h" #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCDisassembler/MCDisassembler.h" @@ -41,6 +43,21 @@ namespace llvm { namespace cfi_verify { +bool IgnoreDWARF; + +static cl::opt IgnoreDWARFArg( + "ignore-dwarf", + cl::desc( + "Ignore all DWARF data. This relaxes the requirements for all " + "statically linked libraries to have been compiled with '-g', but " + "will result in false positives for 'CFI unprotected' instructions."), + cl::location(IgnoreDWARF), cl::init(false)); + +cl::opt DWARFSearchRange( + "dwarf-search-range", + cl::desc("Address search range used to determine if instruction is valid."), + cl::init(0x10)); + Expected FileAnalysis::Create(StringRef Filename) { // Open the filename provided. Expected> BinaryOrErr = @@ -76,6 +93,32 @@ 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 = @@ -215,6 +258,8 @@ return MIA.get(); } +DWARFContext *FileAnalysis::getDWARF() { return DWARF.get(); } + Error FileAnalysis::initialiseDisassemblyMembers() { std::string TripleName = ObjectTriple.getTriple(); ArchName = ""; @@ -226,7 +271,8 @@ if (!ObjectTarget) return make_error( (Twine("Couldn't find target \"") + ObjectTriple.getTriple() + - "\", failed with error: " + ErrorString).str()); + "\", failed with error: " + ErrorString) + .str()); RegisterInfo.reset(ObjectTarget->createMCRegInfo(TripleName)); if (!RegisterInfo) @@ -266,6 +312,28 @@ } Error FileAnalysis::parseCodeSections() { + if (!IgnoreDWARF) { + DWARF.reset(DWARFContext::create(*Object).release()); + if (!DWARF) + return make_error("Could not create DWARF information.", + inconvertibleErrorCode()); + + bool LineInfoValid = false; + + for (auto &Unit : DWARF->compile_units()) { + const auto &LineTable = DWARF->getLineTableForUnit(Unit.get()); + if (LineTable && !LineTable->Rows.empty()) { + LineInfoValid = true; + break; + } + } + + if (!LineInfoValid) + return make_error( + "DWARF line information missing. Did you compile with '-g'?", + inconvertibleErrorCode()); + } + for (const object::SectionRef &Section : Object->sections()) { // Ensure only executable sections get analysed. if (!(object::ELFSectionRef(Section).getFlags() & ELF::SHF_EXECINSTR)) @@ -302,6 +370,13 @@ InstrMeta.VMAddress = VMAddress; InstrMeta.InstructionSize = InstructionSize; InstrMeta.Valid = ValidInstruction; + + // Check if this instruction exists in the range of the DWARF metadata. + if (DWARF && + DWARF->getLineInfoForAddressRange(InstrMeta.VMAddress, DWARFSearchRange) + .empty()) + continue; + addInstruction(InstrMeta); if (!ValidInstruction) Index: tools/llvm-cfi-verify/lib/GraphBuilder.cpp =================================================================== --- tools/llvm-cfi-verify/lib/GraphBuilder.cpp +++ tools/llvm-cfi-verify/lib/GraphBuilder.cpp @@ -221,6 +221,13 @@ continue; } + // Call instructions are not valid in the upwards traversal. + if (ParentDesc.isCall()) { + Result.IntermediateNodes[ParentMeta.VMAddress] = Address; + Result.OrphanedNodes.push_back(ParentMeta.VMAddress); + continue; + } + // Evaluate the branch target to ascertain whether this XRef is the result // of a fallthrough or the target of a branch. uint64_t BranchTarget; 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 @@ -22,6 +22,7 @@ #include "llvm/BinaryFormat/ELF.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Error.h" +#include "llvm/Support/FormatVariadic.h" #include @@ -34,28 +35,64 @@ ExitOnError ExitOnErr; -void printIndirectCFInstructions(const FileAnalysis &Verifier) { - for (uint64_t Address : Verifier.getIndirectInstructions()) { - const auto &InstrMeta = Verifier.getInstructionOrDie(Address); - outs() << format_hex(Address, 2) << " |" - << Verifier.getMCInstrInfo()->getName( +void printIndirectCFInstructions(FileAnalysis &Analysis) { + uint64_t ProtectedCount = 0; + uint64_t UnprotectedCount = 0; + DWARFContext *DWARF = Analysis.getDWARF(); + + for (uint64_t Address : Analysis.getIndirectInstructions()) { + const auto &InstrMeta = Analysis.getInstructionOrDie(Address); + + if (Analysis.isIndirectInstructionCFIProtected(Address)) { + outs() << "P "; + ProtectedCount++; + } else { + outs() << "U "; + UnprotectedCount++; + } + + outs() << format_hex(Address, 2) << " | " + << Analysis.getMCInstrInfo()->getName( InstrMeta.Instruction.getOpcode()) << " "; - InstrMeta.Instruction.print(outs()); outs() << "\n"; + + if (DWARF) { + for (const auto &LineKV : + DWARF->getLineInfoForAddressRange(Address, 0x10)) { + outs() << " " << format_hex(LineKV.first, 2) << " = " + << LineKV.second.FileName << ":" << LineKV.second.Line << ":" + << LineKV.second.Column << " (" << LineKV.second.FunctionName + << ")\n"; + } + } } + + if (ProtectedCount || UnprotectedCount) + outs() << formatv( + "Unprotected: {0} ({1:P}), Protected: {2} ({3:P})\n", UnprotectedCount, + (((double)UnprotectedCount) / (UnprotectedCount + ProtectedCount)), + ProtectedCount, + (((double)ProtectedCount) / (UnprotectedCount + ProtectedCount))); + else + outs() << "No indirect CF instructions found.\n"; } int main(int argc, char **argv) { - cl::ParseCommandLineOptions(argc, argv); + cl::ParseCommandLineOptions( + argc, argv, + "Identifies whether Control Flow Integrity protects all indirect control " + "flow instructions in the provided object file, DSO or binary.\nNote: " + "Anything statically linked into the provided file *must* be compiled " + "with '-g'. This can be relaxed through the '--ignore-dwarf' flag."); InitializeAllTargetInfos(); InitializeAllTargetMCs(); InitializeAllAsmParsers(); InitializeAllDisassemblers(); - FileAnalysis Verifier = ExitOnErr(FileAnalysis::Create(InputFilename)); - printIndirectCFInstructions(Verifier); + FileAnalysis Analysis = ExitOnErr(FileAnalysis::Create(InputFilename)); + printIndirectCFInstructions(Analysis); return EXIT_SUCCESS; } Index: unittests/tools/llvm-cfi-verify/CMakeLists.txt =================================================================== --- unittests/tools/llvm-cfi-verify/CMakeLists.txt +++ unittests/tools/llvm-cfi-verify/CMakeLists.txt @@ -5,6 +5,7 @@ AllTargetsDisassemblers AllTargetsInfos CFIVerify + DebugInfoDWARF MC MCParser Object Index: unittests/tools/llvm-cfi-verify/FileAnalysis.cpp =================================================================== --- unittests/tools/llvm-cfi-verify/FileAnalysis.cpp +++ unittests/tools/llvm-cfi-verify/FileAnalysis.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "../tools/llvm-cfi-verify/lib/FileAnalysis.h" +#include "../tools/llvm-cfi-verify/lib/GraphBuilder.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -481,6 +482,177 @@ EXPECT_TRUE(Analysis.getDirectControlFlowXRefs(*InstrMetaPtr).empty()); } +TEST_F(BasicFileAnalysisTest, CFIProtectionInvalidTargets) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x90, // 0: nop + 0x0f, 0x0b, // 1: ud2 + 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)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionBasicFallthroughToUd2) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x02, // 0: jne 4 [+2] + 0x0f, 0x0b, // 2: ud2 + 0xff, 0x10, // 4: callq *(%rax) + }, + 0xDEADBEEF); + EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionBasicJumpToUd2) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x02, // 0: jne 4 [+2] + 0xff, 0x10, // 2: callq *(%rax) + 0x0f, 0x0b, // 4: ud2 + }, + 0xDEADBEEF); + EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathUd2) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x03, // 0: jne 5 [+3] + 0x90, // 2: nop + 0xff, 0x10, // 3: callq *(%rax) + 0x0f, 0x0b, // 5: ud2 + 0x75, 0xf9, // 7: jne 2 [-7] + 0x0f, 0x0b, // 9: ud2 + }, + 0xDEADBEEF); + EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathSingleUd2) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x05, // 0: jne 7 [+5] + 0x90, // 2: nop + 0xff, 0x10, // 3: callq *(%rax) + 0x75, 0xfb, // 5: jne 2 [-5] + 0x0f, 0x0b, // 7: ud2 + }, + 0xDEADBEEF); + EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionDualFailLimitUpwards) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x06, // 0: jne 8 [+6] + 0x90, // 2: nop + 0x90, // 3: nop + 0x90, // 4: nop + 0x90, // 5: nop + 0xff, 0x10, // 6: callq *(%rax) + 0x0f, 0x0b, // 8: ud2 + }, + 0xDEADBEEF); + uint64_t PrevSearchLengthForConditionalBranch = + SearchLengthForConditionalBranch; + SearchLengthForConditionalBranch = 2; + + EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 6)); + + SearchLengthForConditionalBranch = PrevSearchLengthForConditionalBranch; +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionDualFailLimitDownwards) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x02, // 0: jne 4 [+2] + 0xff, 0x10, // 2: callq *(%rax) + 0x90, // 4: nop + 0x90, // 5: nop + 0x90, // 6: nop + 0x90, // 7: nop + 0x0f, 0x0b, // 8: ud2 + }, + 0xDEADBEEF); + uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; + SearchLengthForUndef = 2; + + EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + + SearchLengthForUndef = PrevSearchLengthForUndef; +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionGoodAndBadPaths) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0xeb, 0x02, // 0: jmp 4 [+2] + 0x75, 0x02, // 2: jne 6 [+2] + 0xff, 0x10, // 4: callq *(%rax) + 0x0f, 0x0b, // 6: ud2 + }, + 0xDEADBEEF); + EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionWithUnconditionalJumpInFallthrough) { + if (!SuccessfullyInitialised) + return; + Analysis.parseSectionContents( + { + 0x75, 0x04, // 0: jne 6 [+4] + 0xeb, 0x00, // 2: jmp 4 [+0] + 0xff, 0x10, // 4: callq *(%rax) + 0x0f, 0x0b, // 6: ud2 + }, + 0xDEADBEEF); + EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); +} + +TEST_F(BasicFileAnalysisTest, CFIProtectionComplexExample) { + if (!SuccessfullyInitialised) + return; + // See unittests/GraphBuilder.cpp::BuildFlowGraphComplexExample for this + // graph. + Analysis.parseSectionContents( + { + 0x75, 0x12, // 0: jne 20 [+18] + 0xeb, 0x03, // 2: jmp 7 [+3] + 0x75, 0x10, // 4: jne 22 [+16] + 0x90, // 6: nop + 0x90, // 7: nop + 0x90, // 8: nop + 0xff, 0x10, // 9: callq *(%rax) + 0xeb, 0xfc, // 11: jmp 9 [-4] + 0x75, 0xfa, // 13: jne 9 [-6] + 0xe8, 0x78, 0x56, 0x34, 0x12, // 15: callq OUTOFBOUNDS [+0x12345678] + 0x90, // 20: nop + 0x90, // 21: nop + 0x0f, 0x0b, // 22: ud2 + }, + 0xDEADBEEF); + EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 9)); +} + } // anonymous namespace } // end namespace cfi_verify } // end namespace llvm