samitolvanen created this revision.
Herald added subscribers: pengfei, hiraditya, inglorion.
Herald added a project: All.
samitolvanen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.
With -fsanitize=kcfi, Clang emits a !kcfi_type attribute for all global
functions, making them valid indirect call targets, whether the program
ends up calling them indirectly or not. With LTO, we can "seal" the
program by dropping types from non-address-taken globals, thus limiting
the possible targets that can be called.
Add -fsanitize-kcfi-seal command line flag to Clang that's only allowed
with -flto, and drop types from non-address-taken globals in AsmPrinter
when the kcfi-seal module attribute is set.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138337
Files:
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGen/kcfi.c
clang/test/Driver/fsanitize.c
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/test/CodeGen/X86/kcfi-seal.ll
Index: llvm/test/CodeGen/X86/kcfi-seal.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/kcfi-seal.ll
@@ -0,0 +1,30 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s
+
+@llvm.used = appending global [1 x ptr] [ptr @f3], section "llvm.metadata"
+
+; CHECK-LABEL: __cfi_f1:
+; CHECK-LABEL: f1:
+define void @f1(ptr noundef %x) !kcfi_type !2 {
+ ret void
+}
+
+; CHECK-NOT: __cfi_f2:
+; CHECK-LABEL: f2:
+define void @f2(ptr noundef %x) !kcfi_type !2 {
+ ret void
+}
+
+; CHECK-LABEL: __cfi_f3:
+; CHECK-LABEL: f3:
+define void @f3(ptr noundef %x) !kcfi_type !2 {
+ ret void
+}
+
+define dso_local ptr @g1() {
+ ret ptr @f1
+}
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-seal", i32 1}
+!2 = !{i32 12345678}
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -943,7 +943,8 @@
}
// Emit KCFI type information before patchable-function-prefix nops.
- emitKCFITypeId(*MF);
+ if (needsKCFITypeId(*MF))
+ emitKCFITypeId(*MF);
// Emit M NOPs for -fpatchable-function-entry=N,M where M>0. We arbitrarily
// place prefix data before NOPs.
@@ -1378,6 +1379,14 @@
OutStreamer->popSection();
}
+bool AsmPrinter::needsKCFITypeId(const MachineFunction &MF) {
+ const Function &F = MF.getFunction();
+ // With kcfi-seal, drop the type identifier for non-address-taken functions
+ // even if they have a !kcfi_type attribute to reduce the number of
+ // indirectly callable functions.
+ return !F.getParent()->getModuleFlag("kcfi-seal") || F.hasAddressTaken();
+}
+
void AsmPrinter::emitKCFITypeId(const MachineFunction &MF) {
const Function &F = MF.getFunction();
if (const MDNode *MD = F.getMetadata(LLVMContext::MD_kcfi_type))
Index: llvm/include/llvm/CodeGen/AsmPrinter.h
===================================================================
--- llvm/include/llvm/CodeGen/AsmPrinter.h
+++ llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -412,6 +412,7 @@
void emitBBAddrMapSection(const MachineFunction &MF);
void emitKCFITrapEntry(const MachineFunction &MF, const MCSymbol *Symbol);
+ bool needsKCFITypeId(const MachineFunction &MF);
virtual void emitKCFITypeId(const MachineFunction &MF);
void emitPseudoProbe(const MachineInstr &MI);
Index: clang/test/Driver/fsanitize.c
===================================================================
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -653,6 +653,12 @@
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI
// CHECK-KCFI: "-fsanitize=kcfi"
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fsanitize-kcfi-seal %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-SEAL-NOLTO
+// CHECK-KCFI-SEAL-NOLTO: error: invalid argument '-fsanitize-kcfi-seal' only allowed with '-flto'
+
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fsanitize-kcfi-seal -flto %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-SEAL-LTO
+// CHECK-KCFI-SEAL-LTO: "-fsanitize-kcfi-seal"
+
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fno-sanitize-recover=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-RECOVER
// CHECK-KCFI-RECOVER: error: unsupported argument 'kcfi' to option '-fno-sanitize-recover='
Index: clang/test/CodeGen/kcfi.c
===================================================================
--- clang/test/CodeGen/kcfi.c
+++ clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s --check-prefixes=CHECK,NOSEAL
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -x c++ -o - %s | FileCheck %s --check-prefixes=CHECK,NOSEAL
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-kcfi-seal -flto -o - %s | FileCheck %s --check-prefixes=CHECK,SEAL
#if !__has_feature(kcfi)
#error Missing kcfi?
#endif
@@ -54,5 +55,7 @@
}
// CHECK-DAG: ![[#]] = !{i32 4, !"kcfi", i32 1}
+// NOSEAL-NOT: ![[#]] = !{i32 4, !"kcfi-seal", i32 1}
+// SEAL-DAG: ![[#]] = !{i32 4, !"kcfi-seal", i32 1}
// CHECK-DAG: ![[#TYPE]] = !{i32 [[#HASH]]}
// CHECK-DAG: ![[#TYPE2]] = !{i32 [[#%d,HASH2:]]}
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -725,11 +725,19 @@
options::OPT_fno_sanitize_cfi_canonical_jump_tables, true);
}
- if (AllAddedKinds & SanitizerKind::KCFI && DiagnoseErrors) {
- if (AllAddedKinds & SanitizerKind::CFI)
- D.Diag(diag::err_drv_argument_not_allowed_with)
- << "-fsanitize=kcfi"
- << lastArgumentForMask(D, Args, SanitizerKind::CFI);
+ if (AllAddedKinds & SanitizerKind::KCFI) {
+ KCFISeal = Args.hasArg(options::OPT_fsanitize_kcfi_seal);
+
+ if (DiagnoseErrors) {
+ if (AllAddedKinds & SanitizerKind::CFI)
+ D.Diag(diag::err_drv_argument_not_allowed_with)
+ << "-fsanitize=kcfi"
+ << lastArgumentForMask(D, Args, SanitizerKind::CFI);
+ if (KCFISeal && !D.isUsingLTO())
+ D.Diag(diag::err_drv_argument_only_allowed_with)
+ << "-fsanitize-kcfi-seal"
+ << "-flto";
+ }
}
Stats = Args.hasFlag(options::OPT_fsanitize_stats,
@@ -1221,6 +1229,9 @@
if (CfiCanonicalJumpTables)
CmdArgs.push_back("-fsanitize-cfi-canonical-jump-tables");
+ if (KCFISeal)
+ CmdArgs.push_back("-fsanitize-kcfi-seal");
+
if (Stats)
CmdArgs.push_back("-fsanitize-stats");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -747,8 +747,14 @@
CodeGenOpts.SanitizeCfiCanonicalJumpTables);
}
- if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+ if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
+ // Indicate that we want to drop type identifiers from non-address-taken
+ // globals when compiling with -flto.
+ if (CodeGenOpts.SanitizeKCFISeal && CodeGenOpts.PrepareForLTO &&
+ !CodeGenOpts.PrepareForThinLTO)
+ getModule().addModuleFlag(llvm::Module::Override, "kcfi-seal", 1);
+ }
if (CodeGenOpts.CFProtectionReturn &&
Target.checkCFProtectionReturnSupported(getDiags())) {
Index: clang/include/clang/Driver/SanitizerArgs.h
===================================================================
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -38,6 +38,7 @@
bool CfiCrossDso = false;
bool CfiICallGeneralizePointers = false;
bool CfiCanonicalJumpTables = false;
+ bool KCFISeal = false;
int AsanFieldPadding = 0;
bool SharedRuntime = false;
bool AsanUseAfterScope = true;
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1845,6 +1845,9 @@
Group<f_clang_Group>,
HelpText<"Generalize pointers in CFI indirect call type signature checks">,
MarshallingInfoFlag<CodeGenOpts<"SanitizeCfiICallGeneralizePointers">>;
+def fsanitize_kcfi_seal : Flag<["-"], "fsanitize-kcfi-seal">, Group<f_clang_Group>,
+ HelpText<"Optimize indirect call targets with KCFI (requires LTO)">,
+ MarshallingInfoFlag<CodeGenOpts<"SanitizeKCFISeal">>;
defm sanitize_cfi_canonical_jump_tables : BoolOption<"f", "sanitize-cfi-canonical-jump-tables",
CodeGenOpts<"SanitizeCfiCanonicalJumpTables">, DefaultFalse,
PosFlag<SetTrue, [], "Make">, NegFlag<SetFalse, [CoreOption, NoXarchOption], "Do not make">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -254,6 +254,7 @@
CODEGENOPT(SanitizeMemoryUseAfterDtor, 1, 0) ///< Enable use-after-delete detection
///< in MemorySanitizer
CODEGENOPT(SanitizeCfiCrossDso, 1, 0) ///< Enable cross-dso support in CFI.
+CODEGENOPT(SanitizeKCFISeal, 1, 0) ///< Optimize KCFI indirect call targets with LTO.
CODEGENOPT(SanitizeMinimalRuntime, 1, 0) ///< Use "_minimal" sanitizer runtime for
///< diagnostics.
CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
