[compiler-rt] 50a5c4f - [Sanitizers][Apple] Fix logic bugs that break RestrictMemoryToMaxAddress (#124712)

[via llvm-commits]

Author: thetruestblue
Date: 2025-01-30T15:55:15-08:00
New Revision: 50a5c4f6b9ea8046f90aefdffb8170d1ffb790cd

URL: https://github.com/llvm/llvm-project/commit/50a5c4f6b9ea8046f90aefdffb8170d1ffb790cd
DIFF: https://github.com/llvm/llvm-project/commit/50a5c4f6b9ea8046f90aefdffb8170d1ffb790cd.diff

LOG: [Sanitizers][Apple] Fix logic bugs that break RestrictMemoryToMaxAddress (#124712)

There are two logic bugs breaking RestrictMemoryToMaxAddress.
1. adding left_padding within MapDynamicShadow.
- RoundUpTo((uptr)free_begin + left_padding, alignment) already adjusts
for left padding. Adding this additionally within MapDynamicShadow
causes us to allocate a page larger than necessary.
- This incorrect calculation also means RestrictMemoryToMaxAddress will
never find a big enough gap.


2. There is also an issue with the expectation of hitting
KERN_INVALID_ADDRESS when we are beyond the addressable regions.
- For most embedded scenarios, we exceed vm_max_address without getting
KREN_INVALID_ADDRESS so we setting max_occupied_address to a memory
region the process doesn't have access to, beyond the max address, and
that space is never marked as available so we never find a valid gap in
those regions.
- At some point previous it seems the assumption was once we were beyond
the Max address we could expect KREN_INVALID_ADDRESS, which is no longer
true up through the extended space not given to most processes.
- Because of this, the check` if (new_max_vm < max_occupied_addr)` will
always fail and we will never restrict the address on smaller devices.
- Additionally because of the extra page added by adding left_padding,
and how we only minimally restrict the vm, there's a chance we restrict
the vm only enough for the correctly calculated size of shadow. In these
cases, restricting the vm max address and will always fail due to the
extra page added to space size.

credit to @delcypher for the left_padding diagnosis, remembered his old
radar and PR when investigating this. https://reviews.llvm.org/D85389

Will monitor closely for fall out.

rdar://66603866

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index d15f30c61b5863..0b8a75391136df 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -1203,13 +1203,14 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
   const uptr left_padding =
       Max<uptr>(granularity, 1ULL << min_shadow_base_alignment);
 
-  uptr space_size = shadow_size_bytes + left_padding;
+  uptr space_size = shadow_size_bytes;
 
   uptr largest_gap_found = 0;
   uptr max_occupied_addr = 0;
+
   VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
   uptr shadow_start =
-      FindAvailableMemoryRange(space_size, alignment, granularity,
+      FindAvailableMemoryRange(space_size, alignment, left_padding,
                                &largest_gap_found, &max_occupied_addr);
   // If the shadow doesn't fit, restrict the address space to make it fit.
   if (shadow_start == 0) {
@@ -1229,9 +1230,9 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
     }
     RestrictMemoryToMaxAddress(new_max_vm);
     high_mem_end = new_max_vm - 1;
-    space_size = (high_mem_end >> shadow_scale) + left_padding;
+    space_size = (high_mem_end >> shadow_scale);
     VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
-    shadow_start = FindAvailableMemoryRange(space_size, alignment, granularity,
+    shadow_start = FindAvailableMemoryRange(space_size, alignment, left_padding,
                                             nullptr, nullptr);
     if (shadow_start == 0) {
       Report("Unable to find a memory range after restricting VM.\n");
@@ -1272,10 +1273,15 @@ uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding,
     mach_msg_type_number_t count = kRegionInfoSize;
     kr = mach_vm_region_recurse(mach_task_self(), &address, &vmsize, &depth,
                                 (vm_region_info_t)&vminfo, &count);
-    if (kr == KERN_INVALID_ADDRESS) {
+
+    // There are cases where going beyond the processes' max vm does
+    // not return KERN_INVALID_ADDRESS so we check for going beyond that
+    // max address as well.
+    if (kr == KERN_INVALID_ADDRESS || address > max_vm_address) {
       // No more regions beyond "address", consider the gap at the end of VM.
       address = max_vm_address;
       vmsize = 0;
+      kr = -1;  // break after this iteration.
     } else {
       if (max_occupied_addr) *max_occupied_addr = address + vmsize;
     }