Index: docs/ControlFlowIntegrity.rst =================================================================== --- docs/ControlFlowIntegrity.rst +++ docs/ControlFlowIntegrity.rst @@ -123,6 +123,28 @@ most compilers and should not have security implications, so we allow it by default. It can be disabled with ``-fsanitize=cfi-cast-strict``. +Indirect Function Call Checking +------------------------------- + +This scheme checks that function calls take place using a function of the +correct dynamic type; that is, the dynamic type of the function must match +the static type used at the call. This CFI scheme can be enabled on its own +using ``-fsanitize=cfi-icall``. + +For this scheme to work, each indirect function call in the program, other +than calls in :ref:`blacklisted ` functions, must call a +function which was either compiled with ``-fsanitize=cfi-icall`` enabled, +or whose address was taken by a function in a translation unit compiled with +``-fsanitize=cfi-icall``. + +If a function in a translation unit compiled with ``-fsanitize=cfi-icall`` +takes the address of a function not compiled with ``-fsanitize=cfi-icall``, +that address may differ from the address taken by a function in a translation +unit not compiled with ``-fsanitize=cfi-icall``. This is technically a +violation of the C and C++ standards, but it should not affect most programs. + +This scheme is currently only supported on the x86 and x86_64 architectures. + .. _cfi-blacklist: Blacklist Index: include/clang/AST/Mangle.h =================================================================== --- include/clang/AST/Mangle.h +++ include/clang/AST/Mangle.h @@ -147,6 +147,9 @@ virtual void mangleCXXVTableBitSet(const CXXRecordDecl *RD, raw_ostream &) = 0; + virtual void mangleFunctionBitSet(const FunctionType *FT, + raw_ostream &) = 0; + /// @} }; Index: include/clang/Basic/Sanitizers.def =================================================================== --- include/clang/Basic/Sanitizers.def +++ include/clang/Basic/Sanitizers.def @@ -84,11 +84,13 @@ // Control Flow Integrity SANITIZER("cfi-cast-strict", CFICastStrict) SANITIZER("cfi-derived-cast", CFIDerivedCast) +SANITIZER("cfi-icall", CFIICall) SANITIZER("cfi-unrelated-cast", CFIUnrelatedCast) SANITIZER("cfi-nvcall", CFINVCall) SANITIZER("cfi-vcall", CFIVCall) SANITIZER_GROUP("cfi", CFI, - CFIDerivedCast | CFIUnrelatedCast | CFINVCall | CFIVCall) + CFIDerivedCast | CFIICall | CFIUnrelatedCast | CFINVCall | + CFIVCall) // Safe Stack SANITIZER("safe-stack", SafeStack) Index: lib/AST/ItaniumMangle.cpp =================================================================== --- lib/AST/ItaniumMangle.cpp +++ lib/AST/ItaniumMangle.cpp @@ -175,6 +175,7 @@ void mangleStringLiteral(const StringLiteral *, raw_ostream &) override; void mangleCXXVTableBitSet(const CXXRecordDecl *RD, raw_ostream &) override; + void mangleFunctionBitSet(const FunctionType *FT, raw_ostream &) override; bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) { // Lambda closure types are already numbered. @@ -4113,6 +4114,21 @@ Mangler.mangleType(QualType(RD->getTypeForDecl(), 0)); } +void ItaniumMangleContextImpl::mangleFunctionBitSet(const FunctionType *FT, + raw_ostream &Out) { + if (!isExternallyVisible(FT->getLinkage())) { + // This part of the identifier needs to be unique across all translation + // units in the linked program. The scheme fails if multiple translation + // units are compiled using the same relative source file path, or if + // multiple translation units are built from the same source file. + SourceManager &SM = getASTContext().getSourceManager(); + Out << "[" << SM.getFileEntryForID(SM.getMainFileID())->getName() << "]"; + } + + CXXNameMangler Mangler(*this, Out); + Mangler.mangleType(QualType(FT, 0)); +} + void ItaniumMangleContextImpl::mangleStringLiteral(const StringLiteral *, raw_ostream &) { llvm_unreachable("Can't mangle string literals"); } Index: lib/AST/MicrosoftMangle.cpp =================================================================== --- lib/AST/MicrosoftMangle.cpp +++ lib/AST/MicrosoftMangle.cpp @@ -162,6 +162,7 @@ void mangleStringLiteral(const StringLiteral *SL, raw_ostream &Out) override; void mangleCXXVTableBitSet(const CXXRecordDecl *RD, raw_ostream &Out) override; + void mangleFunctionBitSet(const FunctionType *FT, raw_ostream &Out) override; bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) { // Lambda closure types are already numbered. if (isLambda(ND)) @@ -2807,6 +2808,22 @@ mangler.mangleName(RD); } +void MicrosoftMangleContextImpl::mangleFunctionBitSet(const FunctionType *FT, + raw_ostream &Out) { + if (!isExternallyVisible(FT->getLinkage())) { + // This part of the identifier needs to be unique across all translation + // units in the linked program. The scheme fails if multiple translation + // units are compiled using the same relative source file path, or if + // multiple translation units are built from the same source file. + SourceManager &SM = getASTContext().getSourceManager(); + Out << "[" << SM.getFileEntryForID(SM.getMainFileID())->getName() << "]"; + } + + MicrosoftCXXNameMangler mangler(*this, Out); + mangler.mangleType(QualType(FT, 0), SourceRange(), + MicrosoftCXXNameMangler::QMM_Result); +} + MicrosoftMangleContext * MicrosoftMangleContext::create(ASTContext &Context, DiagnosticsEngine &Diags) { return new MicrosoftMangleContextImpl(Context, Diags); Index: lib/CodeGen/CGExpr.cpp =================================================================== --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -3383,6 +3383,32 @@ } } + // If we are checking indirect calls and this call is indirect, check that the + // function pointer is a member of the bit set for the function type. + if (SanOpts.has(SanitizerKind::CFIICall) && + (!TargetDecl || !isa(TargetDecl))) { + SanitizerScope SanScope(this); + + std::string OutName; + llvm::raw_string_ostream Out(OutName); + CGM.getCXXABI().getMangleContext().mangleFunctionBitSet(FnType, Out); + + llvm::Value *BitSetName = llvm::MetadataAsValue::get( + getLLVMContext(), llvm::MDString::get(getLLVMContext(), Out.str())); + + llvm::Value *CastedCallee = Builder.CreateBitCast(Callee, Int8PtrTy); + llvm::Value *BitSetTest = + Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::bitset_test), + {CastedCallee, BitSetName}); + + llvm::Constant *StaticData[] = { + EmitCheckSourceLocation(E->getLocStart()), + EmitCheckTypeDescriptor(QualType(FnType, 0)), + }; + EmitCheck(std::make_pair(BitSetTest, SanitizerKind::CFIICall), + "cfi_bad_icall", StaticData, CastedCallee); + } + CallArgList Args; if (Chain) Args.add(RValue::get(Builder.CreateBitCast(Chain, CGM.VoidPtrTy)), Index: lib/CodeGen/CodeGenModule.cpp =================================================================== --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -917,6 +917,24 @@ if (FD->isReplaceableGlobalAllocationFunction()) F->addAttribute(llvm::AttributeSet::FunctionIndex, llvm::Attribute::NoBuiltin); + + // If we are checking indirect calls and this is not a non-static member + // function, emit a bit set entry for the function type. + if (LangOpts.Sanitize.has(SanitizerKind::CFIICall) && + !(isa(FD) && !cast(FD)->isStatic())) { + llvm::NamedMDNode *BitsetsMD = + getModule().getOrInsertNamedMetadata("llvm.bitsets"); + std::string OutName; + llvm::raw_string_ostream Out(OutName); + getCXXABI().getMangleContext().mangleFunctionBitSet( + FD->getType()->getAs(), Out); + + llvm::Metadata *BitsetOps[] = { + llvm::MDString::get(getLLVMContext(), Out.str()), + llvm::ConstantAsMetadata::get(F), + llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(Int64Ty, 0))}; + BitsetsMD->addOperand(llvm::MDTuple::get(getLLVMContext(), BitsetOps)); + } } void CodeGenModule::addUsedGlobal(llvm::GlobalValue *GV) { Index: lib/Driver/ToolChain.cpp =================================================================== --- lib/Driver/ToolChain.cpp +++ lib/Driver/ToolChain.cpp @@ -492,6 +492,6 @@ // Return sanitizers which don't require runtime support and are not // platform or architecture-dependent. using namespace SanitizerKind; - return (Undefined & ~Vptr & ~Function) | CFI | CFICastStrict | + return (Undefined & ~Vptr & ~Function) | (CFI & ~CFIICall) | CFICastStrict | UnsignedIntegerOverflow | LocalBounds; } Index: lib/Driver/ToolChains.cpp =================================================================== --- lib/Driver/ToolChains.cpp +++ lib/Driver/ToolChains.cpp @@ -3633,6 +3633,7 @@ if (IsX86_64 || IsMIPS64 || IsPowerPC64) Res |= SanitizerKind::Memory; if (IsX86 || IsX86_64) { + Res |= SanitizerKind::CFIICall; Res |= SanitizerKind::Function; Res |= SanitizerKind::SafeStack; } Index: test/CodeGen/cfi-icall.c =================================================================== --- /dev/null +++ test/CodeGen/cfi-icall.c @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s | FileCheck --check-prefix=MS %s + +void f() { +} + +void xf(); + +void g(int b) { + void (*fp)() = b ? f : xf; + // ITANIUM: call i1 @llvm.bitset.test(i8* {{.*}}, metadata !"FvE") + fp(); +} + +// ITANIUM-DAG: !{!"FvE", void ()* @f, i64 0} +// ITANIUM-DAG: !{!"FvE", void (...)* @xf, i64 0} +// MS-DAG: !{!"$$A6AX@Z", void ()* @f, i64 0} +// MS-DAG: !{!"$$A6AX@Z", void (...)* @xf, i64 0} Index: test/CodeGenCXX/cfi-icall.cpp =================================================================== --- /dev/null +++ test/CodeGenCXX/cfi-icall.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM %s +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s | FileCheck --check-prefix=MS %s + +namespace { + +struct S {}; + +void f(S *s) { +} + +} + +void g() { + void (*fp)(S *) = f; + // ITANIUM: call i1 @llvm.bitset.test(i8* {{.*}}, metadata !"[{{.*}}cfi-icall.cpp]FvPN12_GLOBAL__N_11SEE") + // MS: call i1 @llvm.bitset.test(i8* {{.*}}, metadata !"[{{.*}}cfi-icall.cpp]$$A6AXPEAUS@?A@@@Z") + fp(0); +} + +// ITANIUM: !{!"[{{.*}}cfi-icall.cpp]FvPN12_GLOBAL__N_11SEE", void (%"struct.(anonymous namespace)::S"*)* @_ZN12_GLOBAL__N_11fEPNS_1SE, i64 0} +// MS: !{!"[{{.*}}cfi-icall.cpp]$$A6AXPEAUS@?A@@@Z", void (%"struct.(anonymous namespace)::S"*)* @"\01?f@?A@@YAXPEAUS@?A@@@Z", i64 0} Index: test/Driver/fsanitize.c =================================================================== --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -234,7 +234,7 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-unrelated-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST // RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-nvcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NVCALL // RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-vcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VCALL -// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-unrelated-cast,cfi-nvcall,cfi-vcall +// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall // CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast // CHECK-CFI-UCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-unrelated-cast // CHECK-CFI-NVCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-nvcall @@ -243,6 +243,9 @@ // RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-derived-cast -fno-lto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOLTO // CHECK-CFI-NOLTO: '-fsanitize=cfi-derived-cast' only allowed with '-flto' +// RUN: %clang -target mips-unknown-linux -fsanitize=cfi-icall %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-ICALL-MIPS +// CHECK-CFI-ICALL-MIPS: unsupported option '-fsanitize=cfi-icall' for target 'mips-unknown-linux' + // RUN: %clang -target x86_64-linux-gnu -fsanitize-trap=address -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-TRAP // CHECK-ASAN-TRAP: error: unsupported argument 'address' to option '-fsanitize-trap'