From 7122374327d1e311617b26139d13e21912c34249 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Mon, 3 Nov 2025 14:40:01 -0800 Subject: [PATCH 1/4] [sanitizer-common] [Darwin] Provide warnings for common sandbox issues (#165907) We currently do not handle errors in task_set_exc_guard_behavior. If this fails, mmap can unexpectedly crash. We also do not currently provide a clear warning if no external symbolizers are found. rdar://163798535 (cherry picked from commit 148a42bdd2f252b4366b79fc518356bb06cacac3) --- compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp | 12 +++++++++++- .../sanitizer_symbolizer_posix_libcdep.cpp | 7 +++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp index 0b91c60123d26..a447c2980eed5 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -954,7 +954,17 @@ static void DisableMmapExcGuardExceptions() { RTLD_DEFAULT, "task_set_exc_guard_behavior"); if (set_behavior == nullptr) return; const task_exc_guard_behavior_t task_exc_guard_none = 0; - set_behavior(mach_task_self(), task_exc_guard_none); + kern_return_t res = set_behavior(mach_task_self(), task_exc_guard_none); + if (res != KERN_SUCCESS) { + Report( + "WARN: task_set_exc_guard_behavior returned %d (%s), " + "mmap may fail unexpectedly.\n", + res, mach_error_string(res)); + if (res == KERN_DENIED) + Report( + "HINT: Check that task_set_exc_guard_behavior is allowed by " + "sandbox.\n"); + } } static void VerifyInterceptorsWorking(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp index f8d821e125b7a..7eb0c9756d64a 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp @@ -505,6 +505,13 @@ static void ChooseSymbolizerTools(IntrusiveList *list, } # if SANITIZER_APPLE + if (list->empty()) { + Report( + "WARN: No external symbolizers found. Symbols may be missing or " + "unreliable.\n"); + Report( + "HINT: Is PATH set? Does sandbox allow file-read of /usr/bin/atos?\n"); + } VReport(2, "Using dladdr symbolizer.\n"); list->push_back(new (*allocator) DlAddrSymbolizer()); # endif // SANITIZER_APPLE From 01a8dea499569730532627c72fb22fd40128b9c2 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Thu, 13 Nov 2025 13:42:45 -0800 Subject: [PATCH 2/4] [sanitizer_common] Add darwin-specific MemoryRangeIsAvailable (#167797) The fixes a TOCTOU bug in the code that initializes shadow memory in ASAN: https://github.com/llvm/llvm-project/blob/4b05581bae0e3432cfa514788418fb2fc2144904/compiler-rt/lib/asan/asan_shadow_setup.cpp#L66-L91 1. During initialization, we call `FindDynamicShadowStart` to search the memory mapping for enough space to dynamically allocate shadow memory. 2. We call `MemoryRangeIsAvailable(shadow_start, kHighShadowEnd);`, which goes into `MemoryMappingLayout`. 3. We actually map the shadow with `ReserveShadowMemoryRange`. In step 2, `MemoryMappingLayout` makes various allocations using the internal allocator. This can cause the allocator to map more memory! In some cases, this can actually allocate memory that overlaps with the shadow region returned by` FindDynamicShadowStart` in step 1. This is not actually fatal, but it memory corruption; MAP_FIXED is allowed to overlap other regions, and the effect is any overlapping memory is zeroed. ------ To address this, this PR implements `MemoryRangeIsAvailable` on Darwin without any heap allocations: - Move `IntervalsAreSeparate` into sanitizer_common.h - Guard existing sanitizer_posix implementation of `MemoryRangeIsAvailable` behind !SANITIZER_APPLE - `IsAddressInMappedRegion` in sanitizer_mac becomes `MemoryRangeIsAvailable`, which also checks for overlap with the DYLD shared cache. After this fix, it should be possible to re-land #166005, which triggered this issue on the x86 iOS simulators. rdar://164208439 (cherry picked from commit 6a89439423351be2d63e13e191acd4cb33e0aaff) --- .../lib/sanitizer_common/sanitizer_common.h | 7 ++++ .../lib/sanitizer_common/sanitizer_mac.cpp | 41 ++++++++++++++++--- .../lib/sanitizer_common/sanitizer_mac.h | 2 - .../lib/sanitizer_common/sanitizer_posix.cpp | 15 ++----- .../lib/tsan/rtl/tsan_platform_mac.cpp | 2 +- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index 1dd950fab4551..65e644cf43c67 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -484,6 +484,13 @@ inline uptr Log2(uptr x) { return LeastSignificantSetBitIndex(x); } +inline bool IntervalsAreSeparate(uptr start1, uptr end1, uptr start2, + uptr end2) { + CHECK_LE(start1, end1); + CHECK_LE(start2, end2); + return (end1 < start2) || (end2 < start1); +} + // Don't use std::min, std::max or std::swap, to minimize dependency // on libstdc++. template diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp index a447c2980eed5..43b5cf25c145c 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -103,6 +103,8 @@ extern "C" { natural_t *nesting_depth, vm_region_recurse_info_t info, mach_msg_type_number_t *infoCnt); + + extern const void* _dyld_get_shared_cache_range(size_t* length); } # if !SANITIZER_GO @@ -1397,15 +1399,27 @@ uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding, return 0; } -// Returns true if the address is definitely mapped, and false if it is not -// mapped or could not be determined. -bool IsAddressInMappedRegion(uptr addr) { +// This function (when used during initialization when there is +// only a single thread), can be used to verify that a range +// of memory hasn't already been mapped, and won't be mapped +// later in the shared cache. +// +// If the syscall mach_vm_region_recurse fails (due to sandbox), +// we assume that the memory is not mapped so that execution can continue. +// +// NOTE: range_end is inclusive +// +// WARNING: This function must NOT allocate memory, since it is +// used in InitializeShadowMemory between where we search for +// space for shadow and where we actually allocate it. +bool MemoryRangeIsAvailable(uptr range_start, uptr range_end) { mach_vm_size_t vmsize = 0; natural_t depth = 0; vm_region_submap_short_info_data_64_t vminfo; mach_msg_type_number_t count = VM_REGION_SUBMAP_SHORT_INFO_COUNT_64; - mach_vm_address_t address = addr; + mach_vm_address_t address = range_start; + // First, check if the range is already mapped. kern_return_t kr = mach_vm_region_recurse(mach_task_self(), &address, &vmsize, &depth, (vm_region_info_t)&vminfo, &count); @@ -1417,7 +1431,24 @@ bool IsAddressInMappedRegion(uptr addr) { Report("HINT: Is mach_vm_region_recurse allowed by sandbox?\n"); } - return (kr == KERN_SUCCESS && addr >= address && addr < address + vmsize); + if (kr == KERN_SUCCESS && !IntervalsAreSeparate(address, address + vmsize - 1, + range_start, range_end)) { + // Overlaps with already-mapped memory + return false; + } + + size_t cacheLength; + uptr cacheStart = (uptr)_dyld_get_shared_cache_range(&cacheLength); + + if (cacheStart && + !IntervalsAreSeparate(cacheStart, cacheStart + cacheLength - 1, + range_start, range_end)) { + // Overlaps with shared cache region + return false; + } + + // We believe this address is available. + return true; } // FIXME implement on this platform. diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.h b/compiler-rt/lib/sanitizer_common/sanitizer_mac.h index 789dd8e4d8e9c..b0e4ac7f40745 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.h @@ -76,8 +76,6 @@ struct ThreadEventCallbacks { void InstallPthreadIntrospectionHook(const ThreadEventCallbacks &callbacks); -bool IsAddressInMappedRegion(uptr addr); - } // namespace __sanitizer #endif // SANITIZER_APPLE diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp index 69af6465a62c2..5b2c4e668ca8f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp @@ -225,17 +225,9 @@ void *MapWritableFileToMemory(void *addr, uptr size, fd_t fd, OFF_T offset) { return (void *)p; } -static inline bool IntervalsAreSeparate(uptr start1, uptr end1, - uptr start2, uptr end2) { - CHECK(start1 <= end1); - CHECK(start2 <= end2); - return (end1 < start2) || (end2 < start1); -} - +# if !SANITIZER_APPLE // FIXME: this is thread-unsafe, but should not cause problems most of the time. -// When the shadow is mapped only a single thread usually exists (plus maybe -// several worker threads on Mac, which aren't expected to map big chunks of -// memory). +// When the shadow is mapped only a single thread usually exists bool MemoryRangeIsAvailable(uptr range_start, uptr range_end) { MemoryMappingLayout proc_maps(/*cache_enabled*/true); if (proc_maps.Error()) @@ -251,7 +243,6 @@ bool MemoryRangeIsAvailable(uptr range_start, uptr range_end) { return true; } -#if !SANITIZER_APPLE void DumpProcessMap() { MemoryMappingLayout proc_maps(/*cache_enabled*/true); const sptr kBufSize = 4095; @@ -265,7 +256,7 @@ void DumpProcessMap() { Report("End of process memory map.\n"); UnmapOrDie(filename, kBufSize); } -#endif +# endif const char *GetPwd() { return GetEnv("PWD"); diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp index 62ab0554df08e..5a903782f1497 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp @@ -235,7 +235,7 @@ void InitializePlatformEarly() { } // In some configurations, the max_vm is expanded, but much of this space is // already mapped. TSAN will not work in this configuration. - if (IsAddressInMappedRegion(HiAppMemEnd() - 1)) { + if (!MemoryRangeIsAvailable(HiAppMemEnd() - 1, HiAppMemEnd())) { Report( "ThreadSanitizer: Unsupported virtual memory layout: Address %p is " "already mapped.\n", From c501b4fece7ca20f109513bc0839ad7ba8d0606f Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Thu, 13 Nov 2025 16:11:14 -0800 Subject: [PATCH 3/4] [sanitizer-common] [Darwin] Fix overlapping dyld segment addresses (attempt 2) (#167800) This re-lands #166005, which was reverted due to the issue described in #167797. There are 4 small changes: - Fix LoadedModule leak by calling Clear() on the modules list - Fix internal_strncpy calls that are not null-terminated - Improve test to accept the dylib being loaded from a different path than compiled `{{.*}}[[DYLIB]]` - strcmp => internal_strncmp This should not be merged until after #167797. rdar://163149325 (cherry picked from commit 4fe79a761e98351bf8641ea3055058f6fb2ed9a4) --- .../sanitizer_procmaps_mac.cpp | 82 +++++++++++++++---- .../Darwin/asan-verify-module-map.cpp | 25 ++++++ 2 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index a9533d6fc04ca..360391c87e667 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -42,7 +42,6 @@ struct MemoryMappedSegmentData { const char *current_load_cmd_addr; u32 lc_type; uptr base_virt_addr; - uptr addr_mask; }; template @@ -51,12 +50,60 @@ static void NextSectionLoad(LoadedModule *module, MemoryMappedSegmentData *data, const Section *sc = (const Section *)data->current_load_cmd_addr; data->current_load_cmd_addr += sizeof(Section); - uptr sec_start = (sc->addr & data->addr_mask) + data->base_virt_addr; + uptr sec_start = sc->addr + data->base_virt_addr; uptr sec_end = sec_start + sc->size; module->addAddressRange(sec_start, sec_end, /*executable=*/false, isWritable, sc->sectname); } +static bool VerifyMemoryMapping(MemoryMappingLayout* mapping) { + InternalMmapVector modules; + modules.reserve(128); // matches DumpProcessMap + mapping->DumpListOfModules(&modules); + + InternalMmapVector segments; + for (uptr i = 0; i < modules.size(); ++i) { + for (auto& range : modules[i].ranges()) { + segments.push_back(range); + } + } + + // Verify that none of the segments overlap: + // 1. Sort the segments by the start address + // 2. Check that every segment starts after the previous one ends. + Sort(segments.data(), segments.size(), + [](LoadedModule::AddressRange& a, LoadedModule::AddressRange& b) { + return a.beg < b.beg; + }); + + // To avoid spam, we only print the report message once-per-process. + static bool invalid_module_map_reported = false; + bool well_formed = true; + + for (size_t i = 1; i < segments.size(); i++) { + uptr cur_start = segments[i].beg; + uptr prev_end = segments[i - 1].end; + if (cur_start < prev_end) { + well_formed = false; + VReport(2, "Overlapping mappings: %s start = %p, %s end = %p\n", + segments[i].name, (void*)cur_start, segments[i - 1].name, + (void*)prev_end); + if (!invalid_module_map_reported) { + Report( + "WARN: Invalid dyld module map detected. This is most likely a bug " + "in the sanitizer.\n"); + Report("WARN: Backtraces may be unreliable.\n"); + invalid_module_map_reported = true; + } + } + } + + for (auto& m : modules) m.clear(); + + mapping->Reset(); + return well_formed; +} + void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { // Don't iterate over sections when the caller hasn't set up the // data pointer, when there are no sections, or when the segment @@ -82,6 +129,7 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) { Reset(); + VerifyMemoryMapping(this); } MemoryMappingLayout::~MemoryMappingLayout() { @@ -187,6 +235,7 @@ typedef struct dyld_shared_cache_dylib_text_info extern bool _dyld_get_shared_cache_uuid(uuid_t uuid); extern const void *_dyld_get_shared_cache_range(size_t *length); +extern intptr_t _dyld_get_image_slide(const struct mach_header* mh); extern int dyld_shared_cache_iterate_text( const uuid_t cacheUuid, void (^callback)(const dyld_shared_cache_dylib_text_info *info)); @@ -255,23 +304,21 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, layout_data->current_load_cmd_count--; if (((const load_command *)lc)->cmd == kLCSegment) { const SegmentCommand* sc = (const SegmentCommand *)lc; - uptr base_virt_addr, addr_mask; - if (layout_data->current_image == kDyldImageIdx) { - base_virt_addr = (uptr)get_dyld_hdr(); - // vmaddr is masked with 0xfffff because on macOS versions < 10.12, - // it contains an absolute address rather than an offset for dyld. - // To make matters even more complicated, this absolute address - // isn't actually the absolute segment address, but the offset portion - // of the address is accurate when combined with the dyld base address, - // and the mask will give just this offset. - addr_mask = 0xfffff; - } else { + if (internal_strcmp(sc->segname, "__LINKEDIT") == 0) { + // The LINKEDIT sections are for internal linker use, and may alias + // with the LINKEDIT section for other modules. (If we included them, + // our memory map would contain overlappping sections.) + return false; + } + + uptr base_virt_addr; + if (layout_data->current_image == kDyldImageIdx) + base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr()); + else base_virt_addr = (uptr)_dyld_get_image_vmaddr_slide(layout_data->current_image); - addr_mask = ~0; - } - segment->start = (sc->vmaddr & addr_mask) + base_virt_addr; + segment->start = sc->vmaddr + base_virt_addr; segment->end = segment->start + sc->vmsize; // Most callers don't need section information, so only fill this struct // when required. @@ -281,9 +328,9 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, (const char *)lc + sizeof(SegmentCommand); seg_data->lc_type = kLCSegment; seg_data->base_virt_addr = base_virt_addr; - seg_data->addr_mask = addr_mask; internal_strncpy(seg_data->name, sc->segname, ARRAY_SIZE(seg_data->name)); + seg_data->name[ARRAY_SIZE(seg_data->name) - 1] = 0; } // Return the initial protection. @@ -297,6 +344,7 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, ? kDyldPath : _dyld_get_image_name(layout_data->current_image); internal_strncpy(segment->filename, src, segment->filename_size); + segment->filename[segment->filename_size - 1] = 0; } segment->arch = layout_data->current_arch; internal_memcpy(segment->uuid, layout_data->current_uuid, kModuleUUIDSize); diff --git a/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp new file mode 100644 index 0000000000000..15be1cd6754c3 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp @@ -0,0 +1,25 @@ +// This test simply checks that the "Invalid dyld module map" warning is not printed +// in the output of a backtrace. + +// RUN: %clangxx_asan -DSHARED_LIB -g %s -dynamiclib -o %t.dylib +// RUN: %clangxx_asan -O0 -g %s %t.dylib -o %t.executable +// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s -DDYLIB=%{t:stem}.tmp.dylib + +// CHECK-NOT: WARN: Invalid dyld module map +// CHECK-DAG: 0x{{.*}}-0x{{.*}} {{.*}}[[DYLIB]] +// CHECK-DAG: 0x{{.*}}-0x{{.*}} {{.*}}libsystem + +#ifdef SHARED_LIB +extern "C" void foo(int *a) { *a = 5; } +#else +# include + +extern "C" void foo(int *a); + +int main() { + int *a = (int *)malloc(sizeof(int)); + free(a); + foo(a); + return 0; +} +#endif \ No newline at end of file From b5f6ecd5af7dc5df2d33359dc1b5e5673b6f7f84 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Fri, 21 Nov 2025 07:47:11 -0800 Subject: [PATCH 4/4] [TSan] [Darwin] Fix off by one in TSAN init due to MemoryRangeIsAvailable (#169008) (cherry picked from commit bb2e4686c1c8e4955ff5d18a7baaef3fe14ba36e) --- compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp | 2 +- 1 file changed, 1 insertion(+), 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 5a903782f1497..cc62a21fe015d 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp @@ -235,7 +235,7 @@ void InitializePlatformEarly() { } // In some configurations, the max_vm is expanded, but much of this space is // already mapped. TSAN will not work in this configuration. - if (!MemoryRangeIsAvailable(HiAppMemEnd() - 1, HiAppMemEnd())) { + if (!MemoryRangeIsAvailable(HiAppMemEnd() - 1, HiAppMemEnd() - 1)) { Report( "ThreadSanitizer: Unsupported virtual memory layout: Address %p is " "already mapped.\n",