Index: docs/ClangCommandLineReference.rst =================================================================== --- docs/ClangCommandLineReference.rst +++ docs/ClangCommandLineReference.rst @@ -740,6 +740,10 @@ Enable control flow integrity (CFI) checks for cross-DSO calls. +.. option:: -fsanitize-cfi-icall-generalize-pointers + +Generalize pointers in function type signatures used for Control Flow Integrity (CFI) indirect call checking + .. option:: -fsanitize-coverage=,..., -fno-sanitize-coverage=,... Specify the type of coverage instrumentation for Sanitizers Index: docs/ControlFlowIntegrity.rst =================================================================== --- docs/ControlFlowIntegrity.rst +++ docs/ControlFlowIntegrity.rst @@ -215,6 +215,23 @@ This scheme is currently only supported on the x86 and x86_64 architectures. +``-fsanitize-cfi-icall-generalize-pointers`` +-------------------------------------------- + +Mismatched pointer types are a common cause of cfi-icall check failures. +Translations units compiled with the ``-fsanitize-cfi-icall-generalize-pointers`` +flag relax pointer type checking for call sites in that translation unit, +applied across all functions compiled with ``-fsanitize=cfi-icall``. + +Specifically, pointers in return and argument types are treated as equivalent as +long as the qualifiers for the type they point to match. For example, ``char*`` +``char**`, and ``int*`` are considered equivalent types. However, ``char*`` and +``const char*`` are considered seperate types. + +``-fsanitize-cfi-icall-generalize-pointers`` is not compatible with +``-fsanitize-cfi-cross-dso``. + + ``-fsanitize=cfi-icall`` and ``-fsanitize=function`` ---------------------------------------------------- Index: docs/UsersManual.rst =================================================================== --- docs/UsersManual.rst +++ docs/UsersManual.rst @@ -1147,6 +1147,11 @@ the behavior of sanitizers in the ``cfi`` group to allow checking of cross-DSO virtual and indirect calls. +.. option:: -fsanitize-cfi-icall-generalize-pointers + + Generalize pointers in return and argument types in function type signatures + checked by Control Flow Integrity indirect call checking. See + :doc:`ControlFlowIntegrity` for more details. .. option:: -fstrict-vtable-pointers Index: include/clang/Driver/Options.td =================================================================== --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -914,6 +914,9 @@ Flags<[CoreOption, DriverOption]>, Group, HelpText<"Disable control flow integrity (CFI) checks for cross-DSO calls.">; +def fsanitize_cfi_icall_generalize_pointers : Flag<["-"], "fsanitize-cfi-icall-generalize-pointers">, + Group, + HelpText<"Generalize pointers in CFI indirect call type signature checks">; def fsanitize_stats : Flag<["-"], "fsanitize-stats">, Group, HelpText<"Enable sanitizer statistics gathering.">; Index: include/clang/Driver/SanitizerArgs.h =================================================================== --- include/clang/Driver/SanitizerArgs.h +++ include/clang/Driver/SanitizerArgs.h @@ -32,6 +32,7 @@ int MsanTrackOrigins = 0; bool MsanUseAfterDtor = false; bool CfiCrossDso = false; + bool CfiICallGeneralizePointers = false; int AsanFieldPadding = 0; bool SharedRuntime = false; bool AsanUseAfterScope = true; Index: include/clang/Frontend/CodeGenOptions.def =================================================================== --- include/clang/Frontend/CodeGenOptions.def +++ include/clang/Frontend/CodeGenOptions.def @@ -154,6 +154,8 @@ CODEGENOPT(SanitizeCfiCrossDso, 1, 0) ///< Enable cross-dso support in CFI. CODEGENOPT(SanitizeMinimalRuntime, 1, 0) ///< Use "_minimal" sanitizer runtime for ///< diagnostics. +CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in + ///< CFI icall function signatures CODEGENOPT(SanitizeCoverageType, 2, 0) ///< Type of sanitizer coverage ///< instrumentation. CODEGENOPT(SanitizeCoverageIndirectCalls, 1, 0) ///< Enable sanitizer coverage Index: lib/CodeGen/CGExpr.cpp =================================================================== --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -4477,7 +4477,12 @@ SanitizerScope SanScope(this); EmitSanitizerStatReport(llvm::SanStat_CFI_ICall); - llvm::Metadata *MD = CGM.CreateMetadataIdentifierForType(QualType(FnType, 0)); + llvm::Metadata *MD; + if (CGM.getCodeGenOpts().SanitizeCfiICallGeneralizePointers) + MD = CGM.GeneralizeFunctionType(QualType(FnType, 0)); + else + MD = CGM.CreateMetadataIdentifierForType(QualType(FnType, 0)); + llvm::Value *TypeId = llvm::MetadataAsValue::get(getLLVMContext(), MD); llvm::Value *CalleePtr = Callee.getFunctionPointer(); Index: lib/CodeGen/CodeGenModule.h =================================================================== --- lib/CodeGen/CodeGenModule.h +++ lib/CodeGen/CodeGenModule.h @@ -497,7 +497,9 @@ /// Mapping from canonical types to their metadata identifiers. We need to /// maintain this mapping because identifiers may be formed from distinct /// MDNodes. - llvm::DenseMap MetadataIdMap; + typedef llvm::DenseMap MetadataTypeMap; + MetadataTypeMap MetadataIdMap; + MetadataTypeMap GeneralizedMetadataIdMap; public: CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts, @@ -1200,6 +1202,14 @@ /// internal identifiers). llvm::Metadata *CreateMetadataIdentifierForType(QualType T); + /// Create a metadata identifier for the given generalized type. This may + /// either be an MDString (for external identifiers) or a distinct unnamed + /// MDNode (for internal identifiers). + llvm::Metadata *CreateMetadataIdentifierGeneralized(QualType T); + + // Apply type generalization to a FunctionType's return and argument types + llvm::Metadata *GeneralizeFunctionType(QualType Ty); + /// Create and attach type metadata to the given function. void CreateFunctionTypeMetadata(const FunctionDecl *FD, llvm::Function *F); Index: lib/CodeGen/CodeGenModule.cpp =================================================================== --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -1150,6 +1150,7 @@ llvm::Metadata *MD = CreateMetadataIdentifierForType(FD->getType()); F->addTypeMetadata(0, MD); + F->addTypeMetadata(0, GeneralizeFunctionType(FD->getType())); // Emit a hash-based bit set entry for cross-DSO calls. if (CodeGenOpts.SanitizeCfiCrossDso) @@ -4541,6 +4542,58 @@ return InternalId; } +llvm::Metadata *CodeGenModule::CreateMetadataIdentifierGeneralized(QualType T) { + llvm::Metadata *&InternalId = GeneralizedMetadataIdMap[T.getCanonicalType()]; + if (InternalId) + return InternalId; + + if (isExternallyVisible(T->getLinkage())) { + std::string OutName; + llvm::raw_string_ostream Out(OutName); + getCXXABI().getMangleContext().mangleTypeName(T, Out); + Out << ".generalized"; + + InternalId = llvm::MDString::get(getLLVMContext(), Out.str()); + } else { + InternalId = llvm::MDNode::getDistinct(getLLVMContext(), + llvm::ArrayRef()); + } + + return InternalId; +} + +// Generalize pointer types to a void pointer with the qualifiers of the +// originally pointed-to type, e.g. 'const char *' and 'char * const *' +// generalize to 'const void *' while 'char *' and 'const char **' generalize to +// 'void *'. +static QualType GeneralizeType(ASTContext &Ctx, QualType Ty) { + if (!Ty->isPointerType()) + return Ty; + + return Ctx.getPointerType( + QualType(Ctx.VoidTy).withFastQualifiers( + Ty->getPointeeType().getQualifiers().getFastQualifiers())); +} + +llvm::Metadata *CodeGenModule::GeneralizeFunctionType(QualType Ty) { + QualType GeneralizedFnType; + if (auto *FnType = Ty->getAs()) { + SmallVector GeneralizedParams; + for (auto &Param : FnType->param_types()) + GeneralizedParams.push_back(GeneralizeType(getContext(), Param)); + + GeneralizedFnType = getContext().getFunctionType( + GeneralizeType(getContext(), FnType->getReturnType()), + GeneralizedParams, FnType->getExtProtoInfo()); + } else if (auto *FnType = Ty->getAs()) { + GeneralizedFnType = getContext().getFunctionNoProtoType( + GeneralizeType(getContext(), FnType->getReturnType())); + } else + llvm_unreachable("Encountered unknown FunctionType"); + + return CreateMetadataIdentifierGeneralized(GeneralizedFnType); +} + /// Returns whether this module needs the "all-vtables" type identifier. bool CodeGenModule::NeedAllVtablesTypeId() const { // Returns true if at least one of vtable-based CFI checkers is enabled and Index: lib/Driver/SanitizerArgs.cpp =================================================================== --- lib/Driver/SanitizerArgs.cpp +++ lib/Driver/SanitizerArgs.cpp @@ -520,6 +520,13 @@ // Without PIE, external function address may resolve to a PLT record, which // can not be verified by the target module. NeedPIE |= CfiCrossDso; + CfiICallGeneralizePointers = + Args.hasArg(options::OPT_fsanitize_cfi_icall_generalize_pointers); + + if (CfiCrossDso && CfiICallGeneralizePointers) + D.Diag(diag::err_drv_argument_not_allowed_with) + << "-fsanitize-cfi-cross-dso" + << "-fsanitize-cfi-icall-generalize-pointers"; } Stats = Args.hasFlag(options::OPT_fsanitize_stats, @@ -807,6 +814,9 @@ if (CfiCrossDso) CmdArgs.push_back("-fsanitize-cfi-cross-dso"); + if (CfiICallGeneralizePointers) + CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers"); + if (Stats) CmdArgs.push_back("-fsanitize-stats"); Index: lib/Frontend/CompilerInvocation.cpp =================================================================== --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -839,6 +839,8 @@ false); Opts.SanitizeMinimalRuntime = Args.hasArg(OPT_fsanitize_minimal_runtime); Opts.SanitizeCfiCrossDso = Args.hasArg(OPT_fsanitize_cfi_cross_dso); + Opts.SanitizeCfiICallGeneralizePointers = + Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers); Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats); if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope, OPT_fno_sanitize_address_use_after_scope)) { Index: test/CodeGen/cfi-icall-cross-dso.c =================================================================== --- test/CodeGen/cfi-icall-cross-dso.c +++ test/CodeGen/cfi-icall-cross-dso.c @@ -46,7 +46,7 @@ // Check that we emit both string and hash based type entries for static void g(), // and don't emit them for the declaration of h(). -// CHECK: define internal void @g({{.*}} !type [[TVOID:![0-9]+]] !type [[TVOID_ID:![0-9]+]] +// CHECK: define internal void @g({{.*}} !type [[TVOID:![0-9]+]] !type [[TVOID_GENERALIZED:![0-9]+]] !type [[TVOID_ID:![0-9]+]] static void g(void) {} // CHECK: declare void @h({{[^!]*$}} @@ -60,9 +60,9 @@ return &h; } -// CHECK: define void @bar({{.*}} !type [[TNOPROTO:![0-9]+]] !type [[TNOPROTO_ID:![0-9]+]] +// CHECK: define void @bar({{.*}} !type [[TNOPROTO:![0-9]+]] !type [[TNOPROTO_GENERALIZED:![0-9]+]] !type [[TNOPROTO_ID:![0-9]+]] // ITANIUM: define available_externally void @foo({{[^!]*$}} -// MS: define linkonce_odr void @foo({{.*}} !type [[TNOPROTO]] !type [[TNOPROTO_ID]] +// MS: define linkonce_odr void @foo({{.*}} !type [[TNOPROTO]] !type [[TNOPROTO_GENERALIZED:![0-9]+]] !type [[TNOPROTO_ID]] inline void foo() {} void bar() { foo(); } @@ -71,11 +71,15 @@ // Check that the type entries are correct. // ITANIUM: [[TVOID]] = !{i64 0, !"_ZTSFvvE"} +// ITANIUM: [[TVOID_GENERALIZED]] = !{i64 0, !"_ZTSFvvE.generalized"} // ITANIUM: [[TVOID_ID]] = !{i64 0, i64 9080559750644022485} // ITANIUM: [[TNOPROTO]] = !{i64 0, !"_ZTSFvE"} +// ITANIUM: [[TNOPROTO_GENERALIZED]] = !{i64 0, !"_ZTSFvE.generalized"} // ITANIUM: [[TNOPROTO_ID]] = !{i64 0, i64 6588678392271548388} // MS: [[TVOID]] = !{i64 0, !"?6AXXZ"} +// MS: [[TVOID_GENERALIZED]] = !{i64 0, !"?6AXXZ.generalized"} // MS: [[TVOID_ID]] = !{i64 0, i64 5113650790573562461} // MS: [[TNOPROTO]] = !{i64 0, !"?6AX@Z"} +// MS: [[TNOPROTO_GENERALIZED]] = !{i64 0, !"?6AX@Z.generalized"} // MS: [[TNOPROTO_ID]] = !{i64 0, i64 4195979634929632483} Index: test/CodeGen/cfi-icall-generalize.c =================================================================== --- /dev/null +++ test/CodeGen/cfi-icall-generalize.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=CHECK --check-prefix=UNGENERALIZED %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -fsanitize-cfi-icall-generalize-pointers -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=GENERALIZED %s + +// Test that const char* is generalized to const void* and that const char** is +// generalized to void* + +// CHECK: define void @f({{.*}} !type [[TYPE:![0-9]+]] !type [[TYPE_GENERALIZED:![0-9]+]] +void f(const char *a, const char **b) { +} + +void g(void (*fp)(const char *, const char **)) { + // UNGENERALIZED: call i1 @llvm.type.test(i8* {{.*}}, metadata !"_ZTSFvPKcPS0_E") + // GENERALIZED: call i1 @llvm.type.test(i8* {{.*}}, metadata !"_ZTSFvPKvPvE.generalized") + fp(0, 0); +} + +// CHECK: [[TYPE]] = !{i64 0, !"_ZTSFvPKcPS0_E"} +// CHECK: [[TYPE_GENERALIZED]] = !{i64 0, !"_ZTSFvPKvPvE.generalized"} Index: test/CodeGen/cfi-icall.c =================================================================== --- test/CodeGen/cfi-icall.c +++ test/CodeGen/cfi-icall.c @@ -3,22 +3,26 @@ // Tests that we assign appropriate identifiers to unprototyped functions. -// CHECK: define void @f({{.*}} !type [[TVOID:![0-9]+]] +// CHECK: define void @f({{.*}} !type [[TVOID:![0-9]+]] !type [[TVOID_GENERALIZED:![0-9]+]] void f() { } void xf(); -// CHECK: define void @g({{.*}} !type [[TINT:![0-9]+]] +// CHECK: define void @g({{.*}} !type [[TINT:![0-9]+]] !type [[TINT_GENERALIZED:![0-9]+]] void g(int b) { void (*fp)() = b ? f : xf; // ITANIUM: call i1 @llvm.type.test(i8* {{.*}}, metadata !"_ZTSFvE") fp(); } -// CHECK: declare !type [[TVOID:![0-9]+]] void @xf({{.*}} +// CHECK: declare !type [[TVOID]] !type [[TVOID_GENERALIZED]] void @xf({{.*}} // ITANIUM-DAG: [[TVOID]] = !{i64 0, !"_ZTSFvE"} +// ITANIUM-DAG: [[TVOID_GENERALIZED]] = !{i64 0, !"_ZTSFvE.generalized"} // ITANIUM-DAG: [[TINT]] = !{i64 0, !"_ZTSFviE"} +// ITANIUM-DAG: [[TINT_GENERALIZED]] = !{i64 0, !"_ZTSFviE.generalized"} // MS-DAG: [[TVOID]] = !{i64 0, !"?6AX@Z"} +// MS-DAG: [[TVOID_GENERALIZED]] = !{i64 0, !"?6AX@Z.generalized"} // MS-DAG: [[TINT]] = !{i64 0, !"?6AXH@Z"} +// MS-DAG: [[TINT_GENERALIZED]] = !{i64 0, !"?6AXH@Z.generalized"} Index: test/CodeGenCXX/cfi-icall.cpp =================================================================== --- test/CodeGenCXX/cfi-icall.cpp +++ test/CodeGenCXX/cfi-icall.cpp @@ -8,19 +8,22 @@ struct S {}; -void f(S *s) { +void f(S s) { } } void g() { - void (*fp)(S *) = f; - // CHECK: call i1 @llvm.type.test(i8* {{.*}}, metadata [[VOIDS:![0-9]+]]) - fp(0); + struct S s; + void (*fp)(S) = f; + // CHECK: call i1 @llvm.type.test(i8* {{.*}}, metadata [[VOIDS1:![0-9]+]]) + fp(s); } -// ITANIUM: define internal void @_ZN12_GLOBAL__N_11fEPNS_1SE({{.*}} !type [[TS:![0-9]+]] -// MS: define internal void @"\01?f@?A@@YAXPEAUS@?A@@@Z"({{.*}} !type [[TS:![0-9]+]] +// ITANIUM: define internal void @_ZN12_GLOBAL__N_11fENS_1SE({{.*}} !type [[TS1:![0-9]+]] !type [[TS2:![0-9]+]] +// MS: define internal void @"\01?f@?A@@YAXUS@?A@@@Z"({{.*}} !type [[TS1:![0-9]+]] !type [[TS2:![0-9]+]] -// CHECK: [[VOIDS]] = distinct !{} -// CHECK: [[TS]] = !{i64 0, [[VOIDS]]} +// CHECK: [[VOIDS1]] = distinct !{} +// CHECK: [[TS1]] = !{i64 0, [[VOIDS1]]} +// CHECK: [[TS2]] = !{i64 0, [[VOIDS2:![0-9]+]]} +// CHECK: [[VOIDS2]] = distinct !{} Index: test/Driver/fsanitize.c =================================================================== --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -471,6 +471,14 @@ // CHECK-CFI-NO-CROSS-DSO: -emit-llvm-bc // CHECK-CFI-NO-CROSS-DSO-NOT: -fsanitize-cfi-cross-dso +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-GENERALIZE-POINTERS +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-CFI-GENERALIZE-POINTERS +// CHECK-CFI-GENERALIZE-POINTERS: -fsanitize-cfi-icall-generalize-pointers +// CHECK-NO-CFI-GENERALIZE-POINTERS-NOT: -fsanitize-cfi-icall-generalize-pointers + +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fsanitize-cfi-cross-dso -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-GENERALIZE-AND-CROSS-DSO +// CHECK-CFI-GENERALIZE-AND-CROSS-DSO: error: invalid argument '-fsanitize-cfi-cross-dso' not allowed with '-fsanitize-cfi-icall-generalize-pointers' + // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS // CHECK-CFI-STATS: -fsanitize-stats