diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2424,6 +2424,8 @@ QualType elementType) { assert(requiresArrayCookie(expr)); + unsigned AS = newPtr.getAddressSpace(); + // The cookie is always at the start of the buffer. Address cookie = newPtr; @@ -2435,7 +2437,20 @@ // The second element is the element count. cookie = CGF.Builder.CreateConstInBoundsGEP(cookie, 1); - CGF.Builder.CreateStore(numElements, cookie); + llvm::Instruction *SI = CGF.Builder.CreateStore(numElements, cookie); + + // Handle poisoning the array cookie in asan + if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 && + (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() || + CGM.getCodeGenOpts().SanitizeAddressPoisonCustomArrayCookie)) { + // The store to the CookiePtr does not need to be instrumented. + CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI); + llvm::FunctionType *FTy = + llvm::FunctionType::get(CGM.VoidTy, cookie.getType(), false); + llvm::FunctionCallee F = + CGM.CreateRuntimeFunction(FTy, "__asan_poison_cxx_array_cookie"); + CGF.Builder.CreateCall(F, cookie.getRawPointer(CGF)); + } // Finally, compute a pointer to the actual data buffer by skipping // over the cookie completely. @@ -2446,13 +2461,28 @@ llvm::Value *ARMCXXABI::readArrayCookieImpl(CodeGenFunction &CGF, Address allocPtr, CharUnits cookieSize) { + unsigned AS = allocPtr.getAddressSpace(); + // The number of elements is at offset sizeof(size_t) relative to // the allocated pointer. Address numElementsPtr = CGF.Builder.CreateConstInBoundsByteGEP(allocPtr, CGF.getSizeSize()); numElementsPtr = CGF.Builder.CreateElementBitCast(numElementsPtr, CGF.SizeTy); - return CGF.Builder.CreateLoad(numElementsPtr); + + if (!CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) || AS != 0) + return CGF.Builder.CreateLoad(numElementsPtr); + + // In asan mode emit a function call instead of a regular load and let the + // run-time deal with it: if the shadow is properly poisoned return the + // cookie, otherwise return 0 to avoid an infinite loop calling DTORs. + // We can't simply ignore this load using nosanitize metadata because + // the metadata may be lost. + llvm::FunctionType *FTy = + llvm::FunctionType::get(CGF.SizeTy, CGF.SizeTy->getPointerTo(0), false); + llvm::FunctionCallee F = + CGM.CreateRuntimeFunction(FTy, "__asan_load_cxx_array_cookie"); + return CGF.Builder.CreateCall(F, numElementsPtr.getRawPointer(CGF)); } /*********************** Static local initialization **************************/ diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp --- a/compiler-rt/lib/asan/asan_poisoning.cpp +++ b/compiler-rt/lib/asan/asan_poisoning.cpp @@ -259,6 +259,10 @@ if (!flags()->poison_array_cookie) return; uptr s = MEM_TO_SHADOW(p); *reinterpret_cast(s) = kAsanArrayCookieMagic; + // The ARM64 cookie has a second "elementSize" entry so poison it as well + #if SANITIZER_ARM64 + *(reinterpret_cast(s)-1) = kAsanArrayCookieMagic; + #endif } extern "C" SANITIZER_INTERFACE_ATTRIBUTE diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp @@ -1,18 +1,13 @@ // REQUIRES: asan-64-bits // RUN: %clangxx_asan -O3 %s -o %t -// RUN: not %run %t 2>&1 | FileCheck %s +// RUN: not %run %t 2>&1 | FileCheck %s // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1 | FileCheck %s // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1 | FileCheck %s --check-prefix=NO_COOKIE -// UNSUPPORTED: ios - -// Added to allow enabling of tests but needs investigation (rdar://91448627) -// XFAIL: (darwin && (arm64-target-arch || arm64e-target-arch)) - #include #include struct C { - int x; + size_t x; ~C() { fprintf(stderr, "ZZZZZZZZ\n"); exit(1); @@ -21,10 +16,10 @@ int main(int argc, char **argv) { C *buffer = new C[argc]; - buffer[-2].x = 10; + buffer[-1].x = 10; // CHECK: AddressSanitizer: heap-buffer-overflow // CHECK: in main {{.*}}new_array_cookie_test.cpp:[[@LINE-2]] -// CHECK: is located 0 bytes inside of 12-byte region +// CHECK: is located {{0 bytes inside of 12|8 bytes inside of 24}}-byte region // NO_COOKIE: ZZZZZZZZ delete [] buffer; } diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp @@ -3,17 +3,12 @@ // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1 | FileCheck %s --check-prefix=NO_COOKIE -// UNSUPPORTED: ios - -// Added to allow enabling of tests but needs investigation (rdar://91448627) -// XFAIL: (darwin && (arm64-target-arch || arm64e-target-arch)) - #include #include #include int dtor_counter; struct C { - int x; + size_t x; ~C() { dtor_counter++; fprintf(stderr, "DTOR %d\n", dtor_counter); @@ -22,7 +17,7 @@ __attribute__((noinline)) void Delete(C *c) { delete[] c; } __attribute__((no_sanitize_address)) void Write42ToCookie(C *c) { - long *p = reinterpret_cast(c); + size_t *p = reinterpret_cast(c); p[-1] = 42; } diff --git a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp --- a/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp +++ b/compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp @@ -1,10 +1,18 @@ -// Test that we do not poison the array cookie if the operator new is defined -// inside the class. -// RUN: %clangxx_asan %s -o %t && %run %t +// For arrays allocated by classes that define their own custom "new", ASAN +// should NOT poison the array cookie unless the compilation unit is compiled +// with "-fsanitize-address-poison-custom-array-cookie". +// Here we test that: +// 1) w/o sanitizer, the array cookie location IS writable +// 2) w/sanitizer, the array cookie location IS writeable by default +// 3) w/sanitizer, the flag "-fno-sanitize-address-poison-custom-array-cookie" +// is a NOP (because this is the default) and finally, +// 4) w/sanitizer AND "-fsanitize-address-poison-custom-array-cookie", the +// array cookie location is NOT writable (ASAN successfully stopped it) // -// XFAIL: arm - -// UNSUPPORTED: ios +// RUN: %clangxx %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT +// RUN: %clangxx_asan %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT +// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_NOT +// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=SANITIZE_YES #include #include @@ -22,19 +30,21 @@ } }; -Foo::~Foo() {} void *Foo::allocated; -Foo *getFoo(size_t n) { - return new Foo[n]; +Foo *getFoo(size_t s) { + return new Foo[s]; } int main() { Foo *foo = getFoo(10); - fprintf(stderr, "foo : %p\n", foo); - fprintf(stderr, "alloc: %p\n", Foo::allocated); - assert(reinterpret_cast(foo) == - reinterpret_cast(Foo::allocated) + sizeof(void*)); - *reinterpret_cast(Foo::allocated) = 42; + + *reinterpret_cast(Foo::allocated) = 42; +// SANITIZE_YES: AddressSanitizer: heap-buffer-overflow +// SANITIZE_YES: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]] +// SANITIZE_YES: is located 0 bytes inside of {{18|26}}-byte region + printf("Unsurprisingly, we were able to write to the array cookie\n"); +// SANITIZE_NOT: Unsurprisingly, we were able to write to the array cookie + return 0; }