Implemented aligning and poisoning global redzones BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4374 R=kschimpf@google.com, stichnot@chromium.org Review URL: https://codereview.chromium.org/2108083002 .
diff --git a/runtime/szrt_asan.c b/runtime/szrt_asan.c index 7a056c4..797f1b9 100644 --- a/runtime/szrt_asan.c +++ b/runtime/szrt_asan.c
@@ -45,14 +45,14 @@ static char *shadow_offset = NULL; -void __asan_init(void); +void __asan_init(int, void **, int *); void __asan_check(char *, int); void *__asan_malloc(size_t); void __asan_free(char *); void __asan_poison(char *, int); void __asan_unpoison(char *, int); -void __asan_init(void) { +void __asan_init(int n_rzs, void **rzs, int *rz_sizes) { // ensure the redzones are large enough to hold metadata assert(RZ_SIZE >= sizeof(void *) && RZ_SIZE >= sizeof(size_t)); assert(shadow_offset == NULL); @@ -71,6 +71,13 @@ fprintf(stderr, "could not protect bad region\n"); else printf("protected bad region\n"); + + // poison global redzones + printf("poisioning %d global redzones\n", n_rzs); + for (int i = 0; i < n_rzs; i++) { + printf("(%d) poisoning redzone of size %d at %p\n", i, rz_sizes[i], rzs[i]); + __asan_poison(rzs[i], rz_sizes[i]); + } } void __asan_check(char *ptr, int size) {
diff --git a/src/IceASanInstrumentation.cpp b/src/IceASanInstrumentation.cpp index 83ebc19..8b54fba 100644 --- a/src/IceASanInstrumentation.cpp +++ b/src/IceASanInstrumentation.cpp
@@ -31,7 +31,9 @@ namespace { constexpr SizeT RzSize = 32; -const std::string RzPrefix = "__$rz"; +constexpr const char *RzPrefix = "__$rz"; +constexpr const char *RzArrayName = "__$rz_array"; +constexpr const char *RzSizesName = "__$rz_sizes"; const llvm::NaClBitcodeRecord::RecordVector RzContents = llvm::NaClBitcodeRecord::RecordVector(RzSize, 'R'); @@ -42,6 +44,15 @@ const string_map FuncSubstitutions = {{"malloc", "__asan_malloc"}, {"free", "__asan_free"}}; +llvm::NaClBitcodeRecord::RecordVector sizeToByteVec(SizeT Size) { + llvm::NaClBitcodeRecord::RecordVector SizeContents; + for (unsigned i = 0; i < sizeof(Size); ++i) { + SizeContents.emplace_back(Size % (1 << CHAR_BIT)); + Size >>= CHAR_BIT; + } + return SizeContents; +} + } // end of anonymous namespace ICE_TLS_DEFINE_FIELD(std::vector<InstCall *> *, ASanInstrumentation, @@ -51,46 +62,69 @@ // types of the redzones and their associated globals match so that they are // laid out together in memory. void ASanInstrumentation::instrumentGlobals(VariableDeclarationList &Globals) { - if (DidInsertRedZones) + if (DidProcessGlobals) return; VariableDeclarationList NewGlobals; // Global holding pointers to all redzones auto *RzArray = VariableDeclaration::create(&NewGlobals); - // Global holding the size of RzArray - auto *RzArraySizeVar = VariableDeclaration::create(&NewGlobals); - SizeT RzArraySize = 0; + // Global holding sizes of all redzones + auto *RzSizes = VariableDeclaration::create(&NewGlobals); - RzArray->setName(Ctx, nextRzName()); - RzArraySizeVar->setName(Ctx, nextRzName()); + RzArray->setName(Ctx, RzArrayName); + RzSizes->setName(Ctx, RzSizesName); RzArray->setIsConstant(true); - RzArraySizeVar->setIsConstant(true); + RzSizes->setIsConstant(true); NewGlobals.push_back(RzArray); - NewGlobals.push_back(RzArraySizeVar); + NewGlobals.push_back(RzSizes); for (VariableDeclaration *Global : Globals) { - VariableDeclaration *RzLeft = - createRz(&NewGlobals, RzArray, RzArraySize, Global); - VariableDeclaration *RzRight = - createRz(&NewGlobals, RzArray, RzArraySize, Global); + assert(Global->getAlignment() <= RzSize); + VariableDeclaration *RzLeft = VariableDeclaration::create(&NewGlobals); + VariableDeclaration *RzRight = VariableDeclaration::create(&NewGlobals); + RzLeft->setName(Ctx, nextRzName()); + RzRight->setName(Ctx, nextRzName()); + SizeT Alignment = std::max(RzSize, Global->getAlignment()); + SizeT RzLeftSize = Alignment; + SizeT RzRightSize = + RzSize + Utils::OffsetToAlignment(Global->getNumBytes(), Alignment); + if (Global->hasNonzeroInitializer()) { + RzLeft->addInitializer(VariableDeclaration::DataInitializer::create( + &NewGlobals, llvm::NaClBitcodeRecord::RecordVector(RzLeftSize, 'R'))); + RzRight->addInitializer(VariableDeclaration::DataInitializer::create( + &NewGlobals, + llvm::NaClBitcodeRecord::RecordVector(RzRightSize, 'R'))); + } else { + RzLeft->addInitializer(VariableDeclaration::ZeroInitializer::create( + &NewGlobals, RzLeftSize)); + RzRight->addInitializer(VariableDeclaration::ZeroInitializer::create( + &NewGlobals, RzRightSize)); + } + RzLeft->setIsConstant(Global->getIsConstant()); + RzRight->setIsConstant(Global->getIsConstant()); + RzLeft->setAlignment(Alignment); + Global->setAlignment(Alignment); + RzRight->setAlignment(1); + RzArray->addInitializer(VariableDeclaration::RelocInitializer::create( + &NewGlobals, RzLeft, RelocOffsetArray(0))); + RzArray->addInitializer(VariableDeclaration::RelocInitializer::create( + &NewGlobals, RzRight, RelocOffsetArray(0))); + RzSizes->addInitializer(VariableDeclaration::DataInitializer::create( + &NewGlobals, sizeToByteVec(RzLeftSize))); + RzSizes->addInitializer(VariableDeclaration::DataInitializer::create( + &NewGlobals, sizeToByteVec(RzRightSize))); + NewGlobals.push_back(RzLeft); NewGlobals.push_back(Global); NewGlobals.push_back(RzRight); + RzGlobalsNum += 2; } - // update the contents of the RzArraySize global - llvm::NaClBitcodeRecord::RecordVector SizeContents; - for (unsigned i = 0; i < sizeof(RzArraySize); i++) { - SizeContents.emplace_back(RzArraySize % (1 << CHAR_BIT)); - RzArraySize >>= CHAR_BIT; - } - RzArraySizeVar->addInitializer( - VariableDeclaration::DataInitializer::create(&NewGlobals, SizeContents)); - // Replace old list of globals, without messing up arena allocators Globals.clear(); Globals.merge(&NewGlobals); - DidInsertRedZones = true; + DidProcessGlobals = true; + GlobalsDoneCV.notify_all(); // Log the new set of globals if (BuildDefs::dump() && (getFlags().getVerbose() & IceV_GlobalInit)) { @@ -108,26 +142,6 @@ return Name.str(); } -VariableDeclaration * -ASanInstrumentation::createRz(VariableDeclarationList *List, - VariableDeclaration *RzArray, SizeT &RzArraySize, - VariableDeclaration *Global) { - auto *Rz = VariableDeclaration::create(List); - Rz->setName(Ctx, nextRzName()); - if (Global->hasNonzeroInitializer()) { - Rz->addInitializer( - VariableDeclaration::DataInitializer::create(List, RzContents)); - } else { - Rz->addInitializer( - VariableDeclaration::ZeroInitializer::create(List, RzSize)); - } - Rz->setIsConstant(Global->getIsConstant()); - RzArray->addInitializer(VariableDeclaration::RelocInitializer::create( - List, Rz, RelocOffsetArray(0))); - ++RzArraySize; - return Rz; -} - // Check for an alloca signaling the presence of local variables and add a // redzone if it is found void ASanInstrumentation::instrumentFuncStart(LoweringContext &Context) { @@ -288,11 +302,22 @@ void ASanInstrumentation::instrumentStart(Cfg *Func) { Constant *ShadowMemInit = Ctx->getConstantExternSym(Ctx->getGlobalString("__asan_init")); - constexpr SizeT NumArgs = 0; + constexpr SizeT NumArgs = 3; constexpr Variable *Void = nullptr; constexpr bool NoTailCall = false; auto *Call = InstCall::create(Func, NumArgs, Void, ShadowMemInit, NoTailCall); Func->getEntryNode()->getInsts().push_front(Call); + + // wait to get the final count of global redzones + if (!DidProcessGlobals) { + GlobalsLock.lock(); + while (!DidProcessGlobals) + GlobalsDoneCV.wait(GlobalsLock); + GlobalsLock.release(); + } + Call->addArg(ConstantInteger32::create(Ctx, IceType_i32, RzGlobalsNum)); + Call->addArg(Ctx->getConstantSym(0, Ctx->getGlobalString(RzArrayName))); + Call->addArg(Ctx->getConstantSym(0, Ctx->getGlobalString(RzSizesName))); } // TODO(tlively): make this more efficient with swap idiom
diff --git a/src/IceASanInstrumentation.h b/src/IceASanInstrumentation.h index de250d4..4757f24 100644 --- a/src/IceASanInstrumentation.h +++ b/src/IceASanInstrumentation.h
@@ -23,6 +23,8 @@ #include "IceGlobalInits.h" #include "IceInstrumentation.h" +#include <condition_variable> + namespace Ice { class ASanInstrumentation : public Instrumentation { @@ -31,17 +33,15 @@ ASanInstrumentation &operator=(const ASanInstrumentation &) = delete; public: - ASanInstrumentation(GlobalContext *Ctx) : Instrumentation(Ctx), RzNum(0) { + ASanInstrumentation(GlobalContext *Ctx) + : Instrumentation(Ctx), RzNum(0), + GlobalsLock(GlobalsMutex, std::defer_lock) { ICE_TLS_INIT_FIELD(LocalDtors); } void instrumentGlobals(VariableDeclarationList &Globals) override; private: std::string nextRzName(); - VariableDeclaration *createRz(VariableDeclarationList *List, - VariableDeclaration *RzArray, - SizeT &RzArraySize, - VariableDeclaration *Global); void instrumentFuncStart(LoweringContext &Context) override; void instrumentCall(LoweringContext &Context, InstCall *Instr) override; void instrumentRet(LoweringContext &Context, InstRet *Instr) override; @@ -51,8 +51,12 @@ void instrumentStart(Cfg *Func) override; void finishFunc(Cfg *Func) override; ICE_TLS_DECLARE_FIELD(std::vector<InstCall *> *, LocalDtors); - bool DidInsertRedZones = false; std::atomic<uint32_t> RzNum; + bool DidProcessGlobals = false; + SizeT RzGlobalsNum = 0; + std::mutex GlobalsMutex; + std::unique_lock<std::mutex> GlobalsLock; + std::condition_variable GlobalsDoneCV; }; } // end of namespace Ice
diff --git a/tests_lit/asan_tests/errors.ll b/tests_lit/asan_tests/errors.ll index e311c85..dd665cc 100644 --- a/tests_lit/asan_tests/errors.ll +++ b/tests_lit/asan_tests/errors.ll
@@ -18,19 +18,19 @@ ; RUN: %t.pexe -o %t && %t 1 2 2>&1 | FileCheck %s ; check with a one off the end global access -; RUIN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ -; RUIN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ -; RUIN: %t.pexe -o %t && %t 1 2 3 2>&1 | FileCheck %s +; RUN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ +; RUN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ +; RUN: %t.pexe -o %t && %t 1 2 3 2>&1 | FileCheck %s ; check with a many off the end global access -; RUIN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ -; RUIN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ -; RUIN: %t.pexe -o %t && %t 1 2 3 4 2>&1 | FileCheck %s +; RUN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ +; RUN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ +; RUN: %t.pexe -o %t && %t 1 2 3 4 2>&1 | FileCheck %s ; check with a one before the front global access -; RUIN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ -; RUIN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ -; RUIN: %t.pexe -o %t && %t 1 2 3 4 5 2>&1 | FileCheck %s +; RUN: llvm-as %s -o - | pnacl-freeze > %t.pexe && %S/../../pydir/szbuild.py \ +; RUN: --fsanitize-address --sz="-allow-externally-defined-symbols" \ +; RUN: %t.pexe -o %t && %t 1 2 3 4 5 2>&1 | FileCheck %s declare external void @exit(i32) @@ -68,18 +68,33 @@ %argc = load i32, i32* %argcptr, align 1 switch i32 %argc, label %error [i32 1, label %one_local i32 2, label %many_local - i32 3, label %neg_local] + i32 3, label %neg_local + i32 4, label %one_global + i32 5, label %many_global + i32 6, label %neg_global] one_local: - ; Access one past the end - %__0 = call i32 @access(i32 1, i32 0) + ; Access one past the end of a local + call i32 @access(i32 1, i32 0) br label %error many_local: - ; Access five past the end - %__1 = call i32 @access(i32 1, i32 4) + ; Access five past the end of a local + call i32 @access(i32 1, i32 4) br label %error neg_local: - ; Access one before the beginning - %__2 = call i32 @access(i32 1, i32 -1) + ; Access one before the beginning of a local + call i32 @access(i32 1, i32 -1) + br label %error +one_global: + ; Access one past the end of a global + call i32 @access(i32 0, i32 0) + br label %error +many_global: + ; Access five past the end of a global + call i32 @access(i32 0, i32 4) + br label %error +neg_global: + ; Access one before the beginning of a global + call i32 @access(i32 0, i32 -1) br label %error error: call void @exit(i32 1)
diff --git a/tests_lit/asan_tests/globalredzones.ll b/tests_lit/asan_tests/globalredzones.ll index 623ce60..154ce87 100644 --- a/tests_lit/asan_tests/globalredzones.ll +++ b/tests_lit/asan_tests/globalredzones.ll
@@ -10,25 +10,49 @@ ; The array of redzones ; DUMP-LABEL: ========= Instrumented Globals ========= -; DUMP: @__$rz0 = internal constant <{ i32, i32, i32, i32, i32, i32 }> -; DUMP: <{ i32 ptrtoint ([32 x i8]* @__$rz2 to i32), i32 ptrtoint ([32 x i8]* @__$rz3 to i32), -; DUMP: i32 ptrtoint ([32 x i8]* @__$rz4 to i32), i32 ptrtoint ([32 x i8]* @__$rz5 to i32), -; DUMP: i32 ptrtoint ([32 x i8]* @__$rz6 to i32), i32 ptrtoint ([32 x i8]* @__$rz7 to i32) }> -; DUMP-NEXT: @__$rz1 = internal constant [4 x i8] c"\06\00\00\00" +; DUMP: @__$rz_array = internal constant <{ i32, i32, i32, i32, i32, i32 }> +; DUMP: <{ i32 ptrtoint ([32 x i8]* @__$rz0 to i32), i32 ptrtoint ([32 x i8]* @__$rz1 to i32), +; DUMP: i32 ptrtoint ([32 x i8]* @__$rz2 to i32), i32 ptrtoint ([32 x i8]* @__$rz3 to i32), +; DUMP: i32 ptrtoint ([32 x i8]* @__$rz4 to i32), i32 ptrtoint ([32 x i8]* @__$rz5 to i32) }> -; CHECK-LABEL: .type __$rz0,%object +; (SPACE is 32 ascii) +; DUMP-NEXT: @__$rz_sizes = internal constant <{ [4 x i8], [4 x i8], [4 x i8], [4 x i8], [4 x i8], +; DUMP-SAME: [4 x i8] }> <{ [4 x i8] c" \00\00\00", [4 x i8] c" \00\00\00", [4 x i8] c" \00\00\00", +; DUMP-SAME: [4 x i8] c" \00\00\00", [4 x i8] c" \00\00\00", [4 x i8] c" \00\00\00" }> + +; CHECK-LABEL: .type __$rz_array,%object ; CHECK-NEXT: .section .rodata -; CHECK-NEXT: __$rz0: +; CHECK-NEXT: __$rz_array: +; CHECK-NEXT: .long __$rz0 +; CHECK-NEXT: .long __$rz1 ; CHECK-NEXT: .long __$rz2 ; CHECK-NEXT: .long __$rz3 ; CHECK-NEXT: .long __$rz4 ; CHECK-NEXT: .long __$rz5 -; CHECK-NEXT: .long __$rz6 -; CHECK-NEXT: .long __$rz7 -; CHECK-LABEL: .type __$rz1,%object +; CHECK-LABEL: .type __$rz_sizes,%object ; CHECK-NEXT: .section .rodata -; CHECK-NEXT: __$rz1: -; CHECK-NEXT: .byte 6 +; CHECK-NEXT: __$rz_sizes: +; CHECK-NEXT: .byte 32 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 32 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 32 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 32 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 32 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte 32 ; CHECK-NEXT: .byte 0 ; CHECK-NEXT: .byte 0 ; CHECK-NEXT: .byte 0 @@ -36,53 +60,59 @@ ; A zero-initialized global @zeroInitGlobal = internal global [32 x i8] zeroinitializer -; DUMP-NEXT: @__$rz2 = internal global [32 x i8] zeroinitializer +; DUMP-NEXT: @__$rz0 = internal global [32 x i8] zeroinitializer ; DUMP-NEXT: @zeroInitGlobal = internal global [32 x i8] zeroinitializer -; DUMP-NEXT: @__$rz3 = internal global [32 x i8] zeroinitializer +; DUMP-NEXT: @__$rz1 = internal global [32 x i8] zeroinitializer -; CHECK-LABEL: .type __$rz2,%object +; CHECK-LABEL: .type __$rz0,%object ; CHECK-NEXT: .section .bss -; CHECK-NEXT: __$rz2: +; CHECK-NEXT: .p2align 5 +; CHECK-NEXT: __$rz0: ; CHECK-LABEL: .type zeroInitGlobal,%object ; CHECK-NEXT: .section .bss +; CHECK-NEXT: .p2align 5 ; CHECK-NEXT: zeroInitGlobal: -; CHECK-LABEL: .type __$rz3,%object +; CHECK-LABEL: .type __$rz1,%object ; CHECK-NEXT: .section .bss -; CHECK-NEXT: __$rz3: +; CHECK-NEXT: __$rz1: ; A constant-initialized global @constInitGlobal = internal constant [32 x i8] c"ABCDEFGHIJKLMNOPQRSTUVWXYZ012345" -; CHECK-LABEL: .type __$rz4,%object +; CHECK-LABEL: .type __$rz2,%object ; CHECK-NEXT: .section .rodata -; CHECK-NEXT: __$rz4: +; CHECK-NEXT: .p2align 5 +; CHECK-NEXT: __$rz2: ; CHECK-LABEL: .type constInitGlobal,%object ; CHECK-NEXT: .section .rodata +; CHECK-NEXT: .p2align 5 ; CHECK-NEXT: constInitGlobal: -; CHECK-LABEL: .type __$rz5,%object +; CHECK-LABEL: .type __$rz3,%object ; CHECK-NEXT: .section .rodata -; CHECK-NEXT: __$rz5: +; CHECK-NEXT: __$rz3: -; DUMP-NEXT: @__$rz4 = internal constant [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" +; DUMP-NEXT: @__$rz2 = internal constant [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" ; DUMP-NEXT: @constInitGlobal = internal constant [32 x i8] c"ABCDEFGHIJKLMNOPQRSTUVWXYZ012345" -; DUMP-NEXT: @__$rz5 = internal constant [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" +; DUMP-NEXT: @__$rz3 = internal constant [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" ; A regular global @regInitGlobal = internal global [32 x i8] c"ABCDEFGHIJKLMNOPQRSTUVWXYZ012345" -; DUMP-NEXT: @__$rz6 = internal global [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" +; DUMP-NEXT: @__$rz4 = internal global [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" ; DUMP-NEXT: @regInitGlobal = internal global [32 x i8] c"ABCDEFGHIJKLMNOPQRSTUVWXYZ012345" -; DUMP-NEXT: @__$rz7 = internal global [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" +; DUMP-NEXT: @__$rz5 = internal global [32 x i8] c"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR" -; CHECK-LABEL: .type __$rz6,%object +; CHECK-LABEL: .type __$rz4,%object ; CHECK-NEXT: .section .data -; CHECK-NEXT: __$rz6: +; CHECK-NEXT: .p2align 5 +; CHECK-NEXT: __$rz4: ; CHECK-LABEL: .type regInitGlobal,%object ; CHECK-NEXT: .section .data +; CHECK-NEXT: .p2align 5 ; CHECK-NEXT: regInitGlobal: -; CHECK-LABEL: .type __$rz7,%object +; CHECK-LABEL: .type __$rz5,%object ; CHECK-NEXT: .section .data -; CHECK-NEXT: __$rz7: +; CHECK-NEXT: __$rz5: define internal void @func() { ret void
diff --git a/tests_lit/asan_tests/startinitcall.ll b/tests_lit/asan_tests/startinitcall.ll index dff9b51..a30f149 100644 --- a/tests_lit/asan_tests/startinitcall.ll +++ b/tests_lit/asan_tests/startinitcall.ll
@@ -25,6 +25,6 @@ ; DUMP-LABEL: ================ Instrumented CFG ================ ; DUMP-NEXT: define void @_start() { ; DUMP-NEXT: __0: -; DUMP-NEXT: call void @__asan_init() +; DUMP-NEXT: call void @__asan_init(i32 0, i32 @__$rz_array, i32 @__$rz_sizes) ; DUMP-NEXT: ret void ; DUMP-NEXT: }