From 0c878b6f56c280be34303b35a3de84bfb5c8784a Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Wed, 13 Mar 2024 22:30:37 +0000 Subject: [PATCH 1/4] [msan] Re-exec with no ASLR if memory layout is incompatible on Linux This ports the change from TSan (https://github.com/llvm/llvm-project/commit/0784b1eefa36d4acbb0dacd2d18796e26313b6c5). A key difference is that TSan initializes the allocator prior to CheckAndProtect, while MSan initializes the allocator afterwards; this slightly simplifies the MSan patch. Nonetheless, we need to check that the allocator layout is compatible with ASLR. Since the information is not readily available in msan.h, we duplicate the information from msan_allocator.cpp, and create a new MappingDesc::ALLOCATOR type. Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan' before and after this patch. --- compiler-rt/lib/msan/msan.cpp | 2 +- compiler-rt/lib/msan/msan.h | 2 +- compiler-rt/lib/msan/msan_linux.cpp | 60 +++++++++++++++++++++++------ 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp index 3cdf10c149902..a2fc27de1901b 100644 --- a/compiler-rt/lib/msan/msan.cpp +++ b/compiler-rt/lib/msan/msan.cpp @@ -467,7 +467,7 @@ void __msan_init() { __msan_clear_on_return(); if (__msan_get_track_origins()) VPrintf(1, "msan_track_origins\n"); - if (!InitShadow(__msan_get_track_origins())) { + if (!InitShadowWithReExec(__msan_get_track_origins())) { Printf("FATAL: MemorySanitizer can not mmap the shadow memory.\n"); Printf("FATAL: Make sure to compile with -fPIE and to link with -pie.\n"); Printf("FATAL: Disabling ASLR is known to cause this error.\n"); diff --git a/compiler-rt/lib/msan/msan.h b/compiler-rt/lib/msan/msan.h index 00f448fb377d5..7fb58be67a02c 100644 --- a/compiler-rt/lib/msan/msan.h +++ b/compiler-rt/lib/msan/msan.h @@ -263,7 +263,7 @@ extern bool msan_init_is_running; extern int msan_report_count; bool ProtectRange(uptr beg, uptr end); -bool InitShadow(bool init_origins); +bool InitShadowWithReExec(bool init_origins); char *GetProcSelfMaps(); void InitializeInterceptors(); diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp index b02994c4c1335..41fa29d4216c5 100644 --- a/compiler-rt/lib/msan/msan_linux.cpp +++ b/compiler-rt/lib/msan/msan_linux.cpp @@ -20,6 +20,9 @@ # include # include # include +# if SANITIZER_LINUX +# include +# endif # include # include # include @@ -43,11 +46,13 @@ void ReportMapRange(const char *descr, uptr beg, uptr size) { } } -static bool CheckMemoryRangeAvailability(uptr beg, uptr size) { +static bool CheckMemoryRangeAvailability(uptr beg, uptr size, bool verbose) { if (size > 0) { uptr end = beg + size - 1; if (!MemoryRangeIsAvailable(beg, end)) { - Printf("FATAL: Memory range 0x%zx - 0x%zx is not available.\n", beg, end); + if (verbose) + Printf("FATAL: Memory range 0x%zx - 0x%zx is not available.\n", beg, + end); return false; } } @@ -106,7 +111,7 @@ static void CheckMemoryLayoutSanity() { } } -bool InitShadow(bool init_origins) { +static bool InitShadow(bool init_origins, bool dry_run) { // Let user know mapping parameters first. VPrintf(1, "__msan_init %p\n", reinterpret_cast(&__msan_init)); for (unsigned i = 0; i < kMemoryLayoutSize; ++i) @@ -116,8 +121,9 @@ bool InitShadow(bool init_origins) { CheckMemoryLayoutSanity(); if (!MEM_IS_APP(&__msan_init)) { - Printf("FATAL: Code %p is out of application range. Non-PIE build?\n", - reinterpret_cast(&__msan_init)); + if (!dry_run) + Printf("FATAL: Code %p is out of application range. Non-PIE build?\n", + reinterpret_cast(&__msan_init)); return false; } @@ -142,21 +148,22 @@ bool InitShadow(bool init_origins) { CHECK(type == MappingDesc::APP || type == MappingDesc::ALLOCATOR); if (type == MappingDesc::ALLOCATOR && - !CheckMemoryRangeAvailability(start, size)) + !CheckMemoryRangeAvailability(start, size, !dry_run)) return false; } if (map) { - if (!CheckMemoryRangeAvailability(start, size)) + if (!CheckMemoryRangeAvailability(start, size, !dry_run)) return false; - if (!MmapFixedSuperNoReserve(start, size, kMemoryLayout[i].name)) + if (!dry_run && + !MmapFixedSuperNoReserve(start, size, kMemoryLayout[i].name)) return false; - if (common_flags()->use_madv_dontdump) + if (!dry_run && common_flags()->use_madv_dontdump) DontDumpShadowMemory(start, size); } if (protect) { - if (!CheckMemoryRangeAvailability(start, size)) + if (!CheckMemoryRangeAvailability(start, size, !dry_run)) return false; - if (!ProtectMemoryRange(start, size, kMemoryLayout[i].name)) + if (!dry_run && !ProtectMemoryRange(start, size, kMemoryLayout[i].name)) return false; } } @@ -164,6 +171,37 @@ bool InitShadow(bool init_origins) { return true; } +bool InitShadowWithReExec(bool init_origins) { + // Start with dry run: check layout is ok, but don't print warnings because + // warning messages will cause tests to fail (even if we successfully re-exec + // after the warning). + bool success = InitShadow(__msan_get_track_origins(), true); + if (!success) { +# if SANITIZER_LINUX + // Perhaps ASLR entropy is too high. If ASLR is enabled, re-exec without it. + int old_personality = personality(0xffffffff); + bool aslr_on = + (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0); + + if (aslr_on) { + VReport(1, + "WARNING: MemorySanitizer: 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(); + } +# endif + } + + // The earlier dry run didn't actually map or protect anything. Run again in + // non-dry run mode. + success = InitShadow(__msan_get_track_origins(), false); + + return success; +} + static void MsanAtExit(void) { if (flags()->print_stats && (flags()->atexit || msan_report_count > 0)) ReportStats(); From b714a62d00d9e3c0a2cecce2391ea5a1dcfa3b50 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Wed, 13 Mar 2024 23:12:01 +0000 Subject: [PATCH 2/4] Skip CheckMemoryRangeAvailability if !dry_run, as suggested by Vitaly --- compiler-rt/lib/msan/msan_linux.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp index 41fa29d4216c5..f094bd8a9deea 100644 --- a/compiler-rt/lib/msan/msan_linux.cpp +++ b/compiler-rt/lib/msan/msan_linux.cpp @@ -147,12 +147,12 @@ static bool InitShadow(bool init_origins, bool dry_run) { if (!map && !protect) { CHECK(type == MappingDesc::APP || type == MappingDesc::ALLOCATOR); - if (type == MappingDesc::ALLOCATOR && + if (dry_run && type == MappingDesc::ALLOCATOR && !CheckMemoryRangeAvailability(start, size, !dry_run)) return false; } if (map) { - if (!CheckMemoryRangeAvailability(start, size, !dry_run)) + if (dry_run && !CheckMemoryRangeAvailability(start, size, !dry_run)) return false; if (!dry_run && !MmapFixedSuperNoReserve(start, size, kMemoryLayout[i].name)) @@ -161,7 +161,7 @@ static bool InitShadow(bool init_origins, bool dry_run) { DontDumpShadowMemory(start, size); } if (protect) { - if (!CheckMemoryRangeAvailability(start, size, !dry_run)) + if (dry_run && !CheckMemoryRangeAvailability(start, size, !dry_run)) return false; if (!dry_run && !ProtectMemoryRange(start, size, kMemoryLayout[i].name)) return false; From 9bda315a71658b8c3b27fdaf4bf55908a744f609 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Thu, 14 Mar 2024 23:56:25 +0000 Subject: [PATCH 3/4] Bug fix: if the memory layout is incompatible and we did not re-exec, we must still return false. (The second InitShadow call does not perform the same checks that the first InitShadow call did.) --- compiler-rt/lib/msan/msan_linux.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp index f094bd8a9deea..004bb64c14dff 100644 --- a/compiler-rt/lib/msan/msan_linux.cpp +++ b/compiler-rt/lib/msan/msan_linux.cpp @@ -197,7 +197,7 @@ bool InitShadowWithReExec(bool init_origins) { // The earlier dry run didn't actually map or protect anything. Run again in // non-dry run mode. - success = InitShadow(__msan_get_track_origins(), false); + success = success && InitShadow(__msan_get_track_origins(), false); return success; } From 63d529ff2439a29e42b37e3d3d4219e16c63f7ce Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Fri, 15 Mar 2024 16:36:52 +0000 Subject: [PATCH 4/4] Simplify code per Vitaly's comment --- compiler-rt/lib/msan/msan_linux.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp index 004bb64c14dff..cd2d9f5c720c5 100644 --- a/compiler-rt/lib/msan/msan_linux.cpp +++ b/compiler-rt/lib/msan/msan_linux.cpp @@ -197,9 +197,7 @@ bool InitShadowWithReExec(bool init_origins) { // The earlier dry run didn't actually map or protect anything. Run again in // non-dry run mode. - success = success && InitShadow(__msan_get_track_origins(), false); - - return success; + return success && InitShadow(__msan_get_track_origins(), false); } static void MsanAtExit(void) {