https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/78351
>From 74d320657671ee64650d96cbc33fef7d414c0ec1 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Tue, 16 Jan 2024 21:06:01 +0000 Subject: [PATCH 1/6] Re-exec TSan with no ASLR if memory layout is incompatible TSan's shadow mappings only support 30-bits of ASLR entropy on x86, and it is not practical to support the maximum of 32-bits (due to pointer compression and the overhead of shadow mappings). Instead, this patch changes TSan to re-exec without ASLR if it encounters an incompatible memory layout, as suggested by Dmitry in https://github.com/google/sanitizers/issues/1716. If ASLR is already disabled, it will abort. This patch involves a bit of refactoring, because the old code is: InitializePlatformEarly() InitializeAllocator() InitializePlatform(): CheckAndProtect() but it may already segfault during InitializeAllocator() if the memory layout is incompatible, before we get a chance to check in CheckAndProtect. This patch adds CheckAndProtect during InitializePlatformEarly(), before the allocator is initialized. Naturally, it is necessary to ensure that CheckAndProtect does *not* allow the heap regions to be occupied there, hence we generalize CheckAndProtect to optionally check the heap regions. We keep the original behavior of CheckAndProtect() in InitializePlatform() as a last line of defense. We need to careful not to prematurely abort if ASLR is disabled but TSan was going to re-exec for other reasons (e.g., unlimited stack size); we implement this by moving all the re-exec logic into ReExecIfNeeded(). --- compiler-rt/lib/tsan/rtl/tsan_platform.h | 2 +- .../lib/tsan/rtl/tsan_platform_linux.cpp | 140 ++++++++++++------ .../lib/tsan/rtl/tsan_platform_posix.cpp | 43 +++++- 3 files changed, 136 insertions(+), 49 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform.h b/compiler-rt/lib/tsan/rtl/tsan_platform.h index 84ff4bfade09ac..377f8aeb8d66e0 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform.h +++ b/compiler-rt/lib/tsan/rtl/tsan_platform.h @@ -1024,7 +1024,7 @@ inline uptr RestoreAddr(uptr addr) { void InitializePlatform(); void InitializePlatformEarly(); -void CheckAndProtect(); +bool CheckAndProtect(bool protect, bool ignore_heap, bool print_warnings); void InitializeShadowMemoryPlatform(); void WriteMemoryProfile(char *buf, uptr buf_size, u64 uptime_ns); int ExtractResolvFDs(void *state, int *fds, int nfd); diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index b45adea45b27ad..bc1be49dd41513 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -214,6 +214,88 @@ void InitializeShadowMemoryPlatform() { #endif // #if !SANITIZER_GO +# if !SANITIZER_GO +static void ReExecIfNeeded() { + // Go maps shadow memory lazily and works fine with limited address space. + // Unlimited stack is not a problem as well, because the executable + // is not compiled with -pie. + { + bool reexec = false; + // TSan doesn't play well with unlimited stack size (as stack + // overlaps with shadow memory). If we detect unlimited stack size, + // we re-exec the program with limited stack size as a best effort. + if (StackSizeIsUnlimited()) { + const uptr kMaxStackSize = 32 * 1024 * 1024; + VReport(1, + "Program is run with unlimited stack size, which wouldn't " + "work with ThreadSanitizer.\n" + "Re-execing with stack size limited to %zd bytes.\n", + kMaxStackSize); + SetStackSizeLimitInBytes(kMaxStackSize); + reexec = true; + } + + if (!AddressSpaceIsUnlimited()) { + Report( + "WARNING: Program is run with limited virtual address space," + " which wouldn't work with ThreadSanitizer.\n"); + Report("Re-execing with unlimited virtual address space.\n"); + SetAddressSpaceUnlimited(); + reexec = true; + } + + // ASLR personality check. + int old_personality = personality(0xffffffff); + bool aslr_on = + (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0); + +# if SANITIZER_ANDROID && (defined(__aarch64__) || defined(__x86_64__)) + // After patch "arm64: mm: support ARCH_MMAP_RND_BITS." is introduced in + // linux kernel, the random gap between stack and mapped area is increased + // from 128M to 36G on 39-bit aarch64. As it is almost impossible to cover + // this big range, we should disable randomized virtual space on aarch64. + if (aslr_on) { + VReport(1, + "WARNING: Program is run with randomized virtual address " + "space, which wouldn't work with ThreadSanitizer on Android.\n" + "Re-execing with fixed virtual address space.\n"); + CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); + reexec = true; + } +# endif + + if (reexec) { + // Don't check the address space since we're going to re-exec anyway. + } else if (!CheckAndProtect(false, false, false)) { + if (aslr_on) { + // Disable ASLR if the memory layout was incompatible. + // Alternatively, we could just keep re-execing until we get lucky + // with a compatible randomized layout, but the risk is that if it's + // not an ASLR-related issue, we will be stuck in an infinite loop of + // re-execing (unless we change ReExec to pass a parameter of the + // number of retries allowed.) + VReport(1, + "WARNING: ThreadSanitizer: memory layout is incompatible, " + "possibly due to high-entropy ASLR.\n" + "Re-execing with fixed virtual address space.\n" + "N.B. reducing ASLR entropy is preferable.\n"); + CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); + reexec = true; + } else { + VReport(1, + "FATAL: ThreadSanitizer: memory layout is incompatible, " + "even though ASLR is disabled.\n" + "Please file a bug.\n"); + Die(); + } + } + + if (reexec) + ReExec(); + } +} +# endif + void InitializePlatformEarly() { vmaSize = (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1); @@ -284,6 +366,10 @@ void InitializePlatformEarly() { } # endif # endif + +# if !SANITIZER_GO + ReExecIfNeeded(); +# endif } void InitializePlatform() { @@ -294,52 +380,22 @@ void InitializePlatform() { // is not compiled with -pie. #if !SANITIZER_GO { - bool reexec = false; - // TSan doesn't play well with unlimited stack size (as stack - // overlaps with shadow memory). If we detect unlimited stack size, - // we re-exec the program with limited stack size as a best effort. - if (StackSizeIsUnlimited()) { - const uptr kMaxStackSize = 32 * 1024 * 1024; - VReport(1, "Program is run with unlimited stack size, which wouldn't " - "work with ThreadSanitizer.\n" - "Re-execing with stack size limited to %zd bytes.\n", - kMaxStackSize); - SetStackSizeLimitInBytes(kMaxStackSize); - reexec = true; - } - - if (!AddressSpaceIsUnlimited()) { - Report("WARNING: Program is run with limited virtual address space," - " which wouldn't work with ThreadSanitizer.\n"); - Report("Re-execing with unlimited virtual address space.\n"); - SetAddressSpaceUnlimited(); - reexec = true; - } -#if SANITIZER_ANDROID && (defined(__aarch64__) || defined(__x86_64__)) - // After patch "arm64: mm: support ARCH_MMAP_RND_BITS." is introduced in - // linux kernel, the random gap between stack and mapped area is increased - // from 128M to 36G on 39-bit aarch64. As it is almost impossible to cover - // this big range, we should disable randomized virtual space on aarch64. - // ASLR personality check. - int old_personality = personality(0xffffffff); - if (old_personality != -1 && (old_personality & ADDR_NO_RANDOMIZE) == 0) { - VReport(1, "WARNING: Program is run with randomized virtual address " - "space, which wouldn't work with ThreadSanitizer.\n" - "Re-execing with fixed virtual address space.\n"); - CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); - reexec = true; - } - -#endif -#if SANITIZER_LINUX && (defined(__aarch64__) || defined(__loongarch_lp64)) +# if SANITIZER_LINUX && (defined(__aarch64__) || defined(__loongarch_lp64)) // Initialize the xor key used in {sig}{set,long}jump. InitializeLongjmpXorKey(); -#endif - if (reexec) - ReExec(); +# endif + } + + // Earlier initialization steps already re-exec'ed until we got a compatible + // memory layout, so we don't expect any more issues here. + if (!CheckAndProtect(true, true, true)) { + Printf( + "FATAL: ThreadSanitizer: unexpectedly found incompatible memory " + "layout.\n"); + Printf("FATAL: Please file a bug.\n"); + Die(); } - CheckAndProtect(); InitTlsSize(); #endif // !SANITIZER_GO } diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp index e7dcd664dc0d20..7d5a992dc665ef 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp @@ -94,22 +94,51 @@ static void ProtectRange(uptr beg, uptr end) { } } -void CheckAndProtect() { +// CheckAndProtect will check if the memory layout is compatible with TSan. +// Optionally (if 'protect' is true), it will set the memory regions between +// app memory to be inaccessible. +// 'ignore_heap' means it will not consider heap memory allocations to be a +// conflict. Set this based on whether we are calling CheckAndProtect before +// or after the allocator has initialized the heap. +bool CheckAndProtect(bool protect, bool ignore_heap, bool print_warnings) { // Ensure that the binary is indeed compiled with -pie. MemoryMappingLayout proc_maps(true); MemoryMappedSegment segment; while (proc_maps.Next(&segment)) { - if (IsAppMem(segment.start)) continue; + if (segment.start >= HeapMemBeg() && segment.end <= HeapEnd()) { + if (ignore_heap) { + continue; + } else { + return false; + } + } + + // Note: IsAppMem includes if it is heap memory, hence we must + // put this check after the heap bounds check. + if (IsAppMem(segment.start) && IsAppMem(segment.end - 1)) + continue; + + // Guard page after the heap end if (segment.start >= HeapMemEnd() && segment.start < HeapEnd()) continue; + if (segment.protection == 0) // Zero page or mprotected. continue; + if (segment.start >= VdsoBeg()) // vdso break; - Printf("FATAL: ThreadSanitizer: unexpected memory mapping 0x%zx-0x%zx\n", - segment.start, segment.end); - Die(); + + // Debug output can break tests. Suppress this message in most cases. + if (print_warnings) + Printf( + "WARNING: ThreadSanitizer: unexpected memory mapping 0x%zx-0x%zx\n", + segment.start, segment.end); + + return false; } + if (!protect) + return true; + # if SANITIZER_IOS && !SANITIZER_IOSSIM ProtectRange(HeapMemEnd(), ShadowBeg()); ProtectRange(ShadowEnd(), MetaShadowBeg()); @@ -135,8 +164,10 @@ void CheckAndProtect() { // Older s390x kernels may not support 5-level page tables. TryProtectRange(user_addr_max_l4, user_addr_max_l5); #endif + + return true; } -#endif +# endif } // namespace __tsan >From 7e2e9122be011502998e1cbe9b8d9b7b04799a9f Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Wed, 17 Jan 2024 20:04:18 +0000 Subject: [PATCH 2/6] Update tsan_platform_mac.cpp to use new CheckAndProtect semantics. The overall behavior is unchanged for Mac (i.e., there is no re-exec added). --- compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp index 1aac0fb27520ce..07d83e1a9a9fff 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp @@ -239,7 +239,10 @@ static uptr longjmp_xor_key = 0; void InitializePlatform() { DisableCoreDumperIfNecessary(); #if !SANITIZER_GO - CheckAndProtect(); + if (!CheckAndProtect(true, true, true)) { + Printf("FATAL: ThreadSanitizer: found incompatible memory layout.\n"); + Die(); + } InitializeThreadStateStorage(); >From 1fede13962e3518c0ae4aa9cca886b8612e33ce5 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Thu, 18 Jan 2024 22:28:55 +0000 Subject: [PATCH 3/6] Remove unnecessary braces --- .../lib/tsan/rtl/tsan_platform_linux.cpp | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index bc1be49dd41513..68376d45e0cdc6 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -219,20 +219,19 @@ static void ReExecIfNeeded() { // Go maps shadow memory lazily and works fine with limited address space. // Unlimited stack is not a problem as well, because the executable // is not compiled with -pie. - { - bool reexec = false; - // TSan doesn't play well with unlimited stack size (as stack - // overlaps with shadow memory). If we detect unlimited stack size, - // we re-exec the program with limited stack size as a best effort. - if (StackSizeIsUnlimited()) { - const uptr kMaxStackSize = 32 * 1024 * 1024; - VReport(1, - "Program is run with unlimited stack size, which wouldn't " - "work with ThreadSanitizer.\n" - "Re-execing with stack size limited to %zd bytes.\n", - kMaxStackSize); - SetStackSizeLimitInBytes(kMaxStackSize); - reexec = true; + bool reexec = false; + // TSan doesn't play well with unlimited stack size (as stack + // overlaps with shadow memory). If we detect unlimited stack size, + // we re-exec the program with limited stack size as a best effort. + if (StackSizeIsUnlimited()) { + const uptr kMaxStackSize = 32 * 1024 * 1024; + VReport(1, + "Program is run with unlimited stack size, which wouldn't " + "work with ThreadSanitizer.\n" + "Re-execing with stack size limited to %zd bytes.\n", + kMaxStackSize); + SetStackSizeLimitInBytes(kMaxStackSize); + reexec = true; } if (!AddressSpaceIsUnlimited()) { @@ -292,7 +291,6 @@ static void ReExecIfNeeded() { if (reexec) ReExec(); - } } # endif >From cff874f3f3e7733e3cf9a442caba5ae66a544362 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Thu, 18 Jan 2024 22:38:40 +0000 Subject: [PATCH 4/6] Whitespace --- compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index 68376d45e0cdc6..c172aecbbd06d3 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -232,7 +232,7 @@ static void ReExecIfNeeded() { kMaxStackSize); SetStackSizeLimitInBytes(kMaxStackSize); reexec = true; - } + } if (!AddressSpaceIsUnlimited()) { Report( >From 9b9686e7d15a50d6903398f96a1adcbcfd52aa49 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Thu, 18 Jan 2024 22:43:54 +0000 Subject: [PATCH 5/6] More changes to whitespace --- .../lib/tsan/rtl/tsan_platform_linux.cpp | 96 +++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp index c172aecbbd06d3..9a66a7feb5b395 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp @@ -234,63 +234,63 @@ static void ReExecIfNeeded() { reexec = true; } - if (!AddressSpaceIsUnlimited()) { - Report( - "WARNING: Program is run with limited virtual address space," - " which wouldn't work with ThreadSanitizer.\n"); - Report("Re-execing with unlimited virtual address space.\n"); - SetAddressSpaceUnlimited(); - reexec = true; - } + if (!AddressSpaceIsUnlimited()) { + Report( + "WARNING: Program is run with limited virtual address space," + " which wouldn't work with ThreadSanitizer.\n"); + Report("Re-execing with unlimited virtual address space.\n"); + SetAddressSpaceUnlimited(); + reexec = true; + } - // ASLR personality check. - int old_personality = personality(0xffffffff); - bool aslr_on = - (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0); + // ASLR personality check. + int old_personality = personality(0xffffffff); + bool aslr_on = + (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0); # if SANITIZER_ANDROID && (defined(__aarch64__) || defined(__x86_64__)) - // After patch "arm64: mm: support ARCH_MMAP_RND_BITS." is introduced in - // linux kernel, the random gap between stack and mapped area is increased - // from 128M to 36G on 39-bit aarch64. As it is almost impossible to cover - // this big range, we should disable randomized virtual space on aarch64. + // After patch "arm64: mm: support ARCH_MMAP_RND_BITS." is introduced in + // linux kernel, the random gap between stack and mapped area is increased + // from 128M to 36G on 39-bit aarch64. As it is almost impossible to cover + // this big range, we should disable randomized virtual space on aarch64. + if (aslr_on) { + VReport(1, + "WARNING: Program is run with randomized virtual address " + "space, which wouldn't work with ThreadSanitizer on Android.\n" + "Re-execing with fixed virtual address space.\n"); + CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); + reexec = true; + } +# endif + + if (reexec) { + // Don't check the address space since we're going to re-exec anyway. + } else if (!CheckAndProtect(false, false, false)) { if (aslr_on) { + // Disable ASLR if the memory layout was incompatible. + // Alternatively, we could just keep re-execing until we get lucky + // with a compatible randomized layout, but the risk is that if it's + // not an ASLR-related issue, we will be stuck in an infinite loop of + // re-execing (unless we change ReExec to pass a parameter of the + // number of retries allowed.) VReport(1, - "WARNING: Program is run with randomized virtual address " - "space, which wouldn't work with ThreadSanitizer on Android.\n" - "Re-execing with fixed virtual address space.\n"); + "WARNING: ThreadSanitizer: memory layout is incompatible, " + "possibly due to high-entropy ASLR.\n" + "Re-execing with fixed virtual address space.\n" + "N.B. reducing ASLR entropy is preferable.\n"); CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); reexec = true; + } else { + VReport(1, + "FATAL: ThreadSanitizer: memory layout is incompatible, " + "even though ASLR is disabled.\n" + "Please file a bug.\n"); + Die(); } -# endif - - if (reexec) { - // Don't check the address space since we're going to re-exec anyway. - } else if (!CheckAndProtect(false, false, false)) { - if (aslr_on) { - // Disable ASLR if the memory layout was incompatible. - // Alternatively, we could just keep re-execing until we get lucky - // with a compatible randomized layout, but the risk is that if it's - // not an ASLR-related issue, we will be stuck in an infinite loop of - // re-execing (unless we change ReExec to pass a parameter of the - // number of retries allowed.) - VReport(1, - "WARNING: ThreadSanitizer: memory layout is incompatible, " - "possibly due to high-entropy ASLR.\n" - "Re-execing with fixed virtual address space.\n" - "N.B. reducing ASLR entropy is preferable.\n"); - CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1); - reexec = true; - } else { - VReport(1, - "FATAL: ThreadSanitizer: memory layout is incompatible, " - "even though ASLR is disabled.\n" - "Please file a bug.\n"); - Die(); - } - } + } - if (reexec) - ReExec(); + if (reexec) + ReExec(); } # endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
