[clang] d3f8931 - [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]
Marco Elver via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 10 06:09:22 PDT 2020
Author: Marco Elver
Date: 2020-06-10T15:08:42+02:00
New Revision: d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7
URL: https://github.com/llvm/llvm-project/commit/d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7
DIFF: https://github.com/llvm/llvm-project/commit/d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7.diff
LOG: [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]
[ v1 was reverted by c6ec352a6bde1995794c523adc2ebab802ccdf0a due to
modpost failing; v2 fixes this. More info:
https://github.com/ClangBuiltLinux/linux/issues/1045#issuecomment-640381783 ]
This makes -fsanitize=kernel-address emit the correct globals
constructors for the kernel. We had to do the following:
* Disable generation of constructors that rely on linker features such
as dead-global elimination.
* Only instrument globals *not* in explicit sections. The kernel uses
sections for special globals, which we should not touch.
* Do not instrument globals that are prefixed with "__" nor that are
aliased by a symbol that is prefixed with "__". For example, modpost
relies on specially named aliases to find globals and checks their
contents. Unfortunately modpost relies on size stored as ELF debug info
and any padding of globals currently causes the debug info to cause size
reported to be *with* redzone which throws modpost off.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203493
Tested:
* With 'clang/test/CodeGen/asan-globals.cpp'.
* With test_kasan.ko, we can see:
BUG: KASAN: global-out-of-bounds in kasan_global_oob+0xb3/0xba [test_kasan]
* allyesconfig, allmodconfig (x86_64)
Reviewed By: glider
Tags: #clang, #llvm
Differential Revision: https://reviews.llvm.org/D81390
Added:
Modified:
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp
Removed:
################################################################################
diff --git a/clang/test/CodeGen/asan-globals.cpp b/clang/test/CodeGen/asan-globals.cpp
index 93abb0023cfa..cf07710d31da 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -1,40 +1,78 @@
// RUN: echo "int extra_global;" > %t.extra-source.cpp
// RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
// The blacklist file uses regexps, so Windows path backslashes.
// RUN: echo "src:%s" | sed -e 's/\\/\\\\/g' > %t.blacklist-src
// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
int global;
int dyn_init_global = global;
int __attribute__((no_sanitize("address"))) attributed_global;
int blacklisted_global;
+int __attribute__((section("__DATA, __common"))) sectioned_global; // [KASAN] ignore globals in a section
+extern "C" {
+int aliased_global; // [KASAN] ignore globals prefixed by aliases with __-prefix [below]
+extern int __attribute__((alias("aliased_global"))) __global_alias; // [KASAN] aliased_global ignored
+int __special_global; // [KASAN] ignore globals with __-prefix
+}
+
void func() {
static int static_var = 0;
const char *literal = "Hello, world!";
}
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// ASAN: sectioned_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: sectioned_global{{.*}}i32
+// ASAN: @aliased_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: @aliased_global{{.*}}i32
+// ASAN: @__special_global{{.*}}{ i32, [60 x i8] }{{.*}}align 32
+// KASAN: @__special_global{{.*}}i32
+
+// CHECK-LABEL: define internal void @asan.module_ctor
+// ASAN-NEXT: call void @__asan_init
+// ASAN-NEXT: call void @__asan_version_mismatch_check
+// KASAN-NOT: call void @__asan_init
+// KASAN-NOT: call void @__asan_version_mismatch_check
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 8)
+// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @asan.module_dtor
+// CHECK-NEXT: call void @__asan_unregister_globals
+// CHECK-NEXT: ret void
+
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[ALIASED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
// CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
// CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
// CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
-// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5}
+// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
// CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
-// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 5}
+// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
// CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], !"sectioned_global", i1 false, i1 false}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 50}
+// CHECK: ![[ALIASED_GLOBAL]] = !{{{.*}} ![[ALIASED_GLOBAL_LOC:[0-9]+]], !"aliased_global", i1 false, i1 false}
+// CHECK: ![[ALIASED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 5}
+// CHECK: ![[SPECIAL_GLOBAL]] = !{{{.*}} ![[SPECIAL_GLOBAL_LOC:[0-9]+]], !"__special_global", i1 false, i1 false}
+// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 19, i32 5}
// CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 23, i32 14}
// CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"<string literal>", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 25}
+// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 24, i32 25}
-// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[ALIASED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
// BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
// BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
// BLACKLIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, i1 true}
// BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[SECTIONED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[ALIASED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[SPECIAL_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[STATIC_VAR]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[LITERAL]] = !{{{.*}} null, null, i1 false, i1 true}
diff --git a/llvm/include/llvm/Transforms/Utils/ModuleUtils.h b/llvm/include/llvm/Transforms/Utils/ModuleUtils.h
index 07fbb5b55678..65added8b7e1 100644
--- a/llvm/include/llvm/Transforms/Utils/ModuleUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/ModuleUtils.h
@@ -42,6 +42,10 @@ void appendToGlobalDtors(Module &M, Function *F, int Priority,
FunctionCallee declareSanitizerInitFunction(Module &M, StringRef InitName,
ArrayRef<Type *> InitArgTypes);
+/// Creates sanitizer constructor function.
+/// \return Returns pointer to constructor.
+Function *createSanitizerCtor(Module &M, StringRef CtorName);
+
/// Creates sanitizer constructor function, and calls sanitizer's init
/// function from it.
/// \return Returns pair of pointers to constructor, and init functions
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 9c039f0cfcc7..dfb038c2861a 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -589,11 +589,10 @@ struct AddressSanitizer {
AddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseAfterScope = false)
- : UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
- this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
- this->CompileKernel =
- ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+ : CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+ : CompileKernel),
+ Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+ UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
C = &(M.getContext());
LongSize = M.getDataLayout().getPointerSizeInBits();
IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -742,7 +741,11 @@ class ModuleAddressSanitizer {
ModuleAddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseGlobalsGC = true, bool UseOdrIndicator = false)
- : GlobalsMD(*GlobalsMD), UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC),
+ : GlobalsMD(*GlobalsMD),
+ CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+ : CompileKernel),
+ Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+ UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC && !this->CompileKernel),
// Enable aliases as they should have no downside with ODR indicators.
UsePrivateAlias(UseOdrIndicator || ClUsePrivateAlias),
UseOdrIndicator(UseOdrIndicator || ClUseOdrIndicator),
@@ -753,11 +756,7 @@ class ModuleAddressSanitizer {
// argument is designed as workaround. Therefore, disable both
// ClWithComdat and ClUseGlobalsGC unless the frontend says it's ok to
// do globals-gc.
- UseCtorComdat(UseGlobalsGC && ClWithComdat) {
- this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
- this->CompileKernel =
- ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+ UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
C = &(M.getContext());
int LongSize = M.getDataLayout().getPointerSizeInBits();
IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -792,7 +791,8 @@ class ModuleAddressSanitizer {
StringRef InternalSuffix);
Instruction *CreateAsanModuleDtor(Module &M);
- bool ShouldInstrumentGlobal(GlobalVariable *G);
+ bool canInstrumentAliasedGlobal(const GlobalAlias &GA) const;
+ bool shouldInstrumentGlobal(GlobalVariable *G) const;
bool ShouldUseMachOGlobalsSection() const;
StringRef getGlobalMetadataSection() const;
void poisonOneInitializer(Function &GlobalInit, GlobalValue *ModuleName);
@@ -1791,7 +1791,23 @@ void ModuleAddressSanitizer::createInitializerPoisonCalls(
}
}
-bool ModuleAddressSanitizer::ShouldInstrumentGlobal(GlobalVariable *G) {
+bool ModuleAddressSanitizer::canInstrumentAliasedGlobal(
+ const GlobalAlias &GA) const {
+ // In case this function should be expanded to include rules that do not just
+ // apply when CompileKernel is true, either guard all existing rules with an
+ // 'if (CompileKernel) { ... }' or be absolutely sure that all these rules
+ // should also apply to user space.
+ assert(CompileKernel && "Only expecting to be called when compiling kernel");
+
+ // When compiling the kernel, globals that are aliased by symbols prefixed
+ // by "__" are special and cannot be padded with a redzone.
+ if (GA.getName().startswith("__"))
+ return false;
+
+ return true;
+}
+
+bool ModuleAddressSanitizer::shouldInstrumentGlobal(GlobalVariable *G) const {
Type *Ty = G->getValueType();
LLVM_DEBUG(dbgs() << "GLOBAL: " << *G << "\n");
@@ -1838,6 +1854,12 @@ bool ModuleAddressSanitizer::ShouldInstrumentGlobal(GlobalVariable *G) {
}
if (G->hasSection()) {
+ // The kernel uses explicit sections for mostly special global variables
+ // that we should not instrument. E.g. the kernel may rely on their layout
+ // without redzones, or remove them at link time ("discard.*"), etc.
+ if (CompileKernel)
+ return false;
+
StringRef Section = G->getSection();
// Globals from llvm.metadata aren't emitted, do not instrument them.
@@ -1904,6 +1926,13 @@ bool ModuleAddressSanitizer::ShouldInstrumentGlobal(GlobalVariable *G) {
}
}
+ if (CompileKernel) {
+ // Globals that prefixed by "__" are special and cannot be padded with a
+ // redzone.
+ if (G->getName().startswith("__"))
+ return false;
+ }
+
return true;
}
@@ -2233,10 +2262,22 @@ bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
bool *CtorComdat) {
*CtorComdat = false;
- SmallVector<GlobalVariable *, 16> GlobalsToChange;
+ // Build set of globals that are aliased by some GA, where
+ // canInstrumentAliasedGlobal(GA) returns false.
+ SmallPtrSet<const GlobalVariable *, 16> AliasedGlobalBlacklist;
+ if (CompileKernel) {
+ for (auto &GA : M.aliases()) {
+ if (const auto *GV = dyn_cast<GlobalVariable>(GA.getAliasee())) {
+ if (!canInstrumentAliasedGlobal(GA))
+ AliasedGlobalBlacklist.insert(GV);
+ }
+ }
+ }
+ SmallVector<GlobalVariable *, 16> GlobalsToChange;
for (auto &G : M.globals()) {
- if (ShouldInstrumentGlobal(&G)) GlobalsToChange.push_back(&G);
+ if (!AliasedGlobalBlacklist.count(&G) && shouldInstrumentGlobal(&G))
+ GlobalsToChange.push_back(&G);
}
size_t n = GlobalsToChange.size();
@@ -2445,20 +2486,23 @@ int ModuleAddressSanitizer::GetAsanVersion(const Module &M) const {
bool ModuleAddressSanitizer::instrumentModule(Module &M) {
initializeCallbacks(M);
- if (CompileKernel)
- return false;
-
// Create a module constructor. A destructor is created lazily because not all
// platforms, and not all modules need it.
- std::string AsanVersion = std::to_string(GetAsanVersion(M));
- std::string VersionCheckName =
- ClInsertVersionCheck ? (kAsanVersionCheckNamePrefix + AsanVersion) : "";
- std::tie(AsanCtorFunction, std::ignore) = createSanitizerCtorAndInitFunctions(
- M, kAsanModuleCtorName, kAsanInitName, /*InitArgTypes=*/{},
- /*InitArgs=*/{}, VersionCheckName);
+ if (CompileKernel) {
+ // The kernel always builds with its own runtime, and therefore does not
+ // need the init and version check calls.
+ AsanCtorFunction = createSanitizerCtor(M, kAsanModuleCtorName);
+ } else {
+ std::string AsanVersion = std::to_string(GetAsanVersion(M));
+ std::string VersionCheckName =
+ ClInsertVersionCheck ? (kAsanVersionCheckNamePrefix + AsanVersion) : "";
+ std::tie(AsanCtorFunction, std::ignore) =
+ createSanitizerCtorAndInitFunctions(M, kAsanModuleCtorName,
+ kAsanInitName, /*InitArgTypes=*/{},
+ /*InitArgs=*/{}, VersionCheckName);
+ }
bool CtorComdat = true;
- // TODO(glider): temporarily disabled globals instrumentation for KASan.
if (ClGlobals) {
IRBuilder<> IRB(AsanCtorFunction->getEntryBlock().getTerminator());
InstrumentGlobals(IRB, M, &CtorComdat);
diff --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index b2e6d981a622..ef9f18a2289e 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -119,6 +119,15 @@ llvm::declareSanitizerInitFunction(Module &M, StringRef InitName,
AttributeList());
}
+Function *llvm::createSanitizerCtor(Module &M, StringRef CtorName) {
+ Function *Ctor = Function::Create(
+ FunctionType::get(Type::getVoidTy(M.getContext()), false),
+ GlobalValue::InternalLinkage, CtorName, &M);
+ BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
+ ReturnInst::Create(M.getContext(), CtorBB);
+ return Ctor;
+}
+
std::pair<Function *, FunctionCallee> llvm::createSanitizerCtorAndInitFunctions(
Module &M, StringRef CtorName, StringRef InitName,
ArrayRef<Type *> InitArgTypes, ArrayRef<Value *> InitArgs,
@@ -128,11 +137,8 @@ std::pair<Function *, FunctionCallee> llvm::createSanitizerCtorAndInitFunctions(
"Sanitizer's init function expects
diff erent number of arguments");
FunctionCallee InitFunction =
declareSanitizerInitFunction(M, InitName, InitArgTypes);
- Function *Ctor = Function::Create(
- FunctionType::get(Type::getVoidTy(M.getContext()), false),
- GlobalValue::InternalLinkage, CtorName, &M);
- BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
- IRBuilder<> IRB(ReturnInst::Create(M.getContext(), CtorBB));
+ Function *Ctor = createSanitizerCtor(M, CtorName);
+ IRBuilder<> IRB(Ctor->getEntryBlock().getTerminator());
IRB.CreateCall(InitFunction, InitArgs);
if (!VersionCheckName.empty()) {
FunctionCallee VersionCheckFunction = M.getOrInsertFunction(
More information about the cfe-commits
mailing list