Skip to content

Commit b7e4f73

Browse files
committed
[Sanitizers][Apple] Fix logic bugs that break RestrictMemoryToMaxAddress (llvm#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 (cherry picked from commit 50a5c4f)
1 parent a9e3fcf commit b7e4f73

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,13 +1195,14 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
11951195
const uptr left_padding =
11961196
Max<uptr>(granularity, 1ULL << min_shadow_base_alignment);
11971197

1198-
uptr space_size = shadow_size_bytes + left_padding;
1198+
uptr space_size = shadow_size_bytes;
11991199

12001200
uptr largest_gap_found = 0;
12011201
uptr max_occupied_addr = 0;
1202+
12021203
VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
12031204
uptr shadow_start =
1204-
FindAvailableMemoryRange(space_size, alignment, granularity,
1205+
FindAvailableMemoryRange(space_size, alignment, left_padding,
12051206
&largest_gap_found, &max_occupied_addr);
12061207
// If the shadow doesn't fit, restrict the address space to make it fit.
12071208
if (shadow_start == 0) {
@@ -1221,9 +1222,9 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
12211222
}
12221223
RestrictMemoryToMaxAddress(new_max_vm);
12231224
high_mem_end = new_max_vm - 1;
1224-
space_size = (high_mem_end >> shadow_scale) + left_padding;
1225+
space_size = (high_mem_end >> shadow_scale);
12251226
VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
1226-
shadow_start = FindAvailableMemoryRange(space_size, alignment, granularity,
1227+
shadow_start = FindAvailableMemoryRange(space_size, alignment, left_padding,
12271228
nullptr, nullptr);
12281229
if (shadow_start == 0) {
12291230
Report("Unable to find a memory range after restricting VM.\n");
@@ -1264,10 +1265,15 @@ uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding,
12641265
mach_msg_type_number_t count = kRegionInfoSize;
12651266
kr = mach_vm_region_recurse(mach_task_self(), &address, &vmsize, &depth,
12661267
(vm_region_info_t)&vminfo, &count);
1267-
if (kr == KERN_INVALID_ADDRESS) {
1268+
1269+
// There are cases where going beyond the processes' max vm does
1270+
// not return KERN_INVALID_ADDRESS so we check for going beyond that
1271+
// max address as well.
1272+
if (kr == KERN_INVALID_ADDRESS || address > max_vm_address) {
12681273
// No more regions beyond "address", consider the gap at the end of VM.
12691274
address = max_vm_address;
12701275
vmsize = 0;
1276+
kr = -1; // break after this iteration.
12711277
} else {
12721278
if (max_occupied_addr) *max_occupied_addr = address + vmsize;
12731279
}

0 commit comments

Comments
 (0)