Skip to content

Commit 50a5c4f

Browse files
[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
1 parent cff0a46 commit 50a5c4f

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
@@ -1203,13 +1203,14 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
12031203
const uptr left_padding =
12041204
Max<uptr>(granularity, 1ULL << min_shadow_base_alignment);
12051205

1206-
uptr space_size = shadow_size_bytes + left_padding;
1206+
uptr space_size = shadow_size_bytes;
12071207

12081208
uptr largest_gap_found = 0;
12091209
uptr max_occupied_addr = 0;
1210+
12101211
VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
12111212
uptr shadow_start =
1212-
FindAvailableMemoryRange(space_size, alignment, granularity,
1213+
FindAvailableMemoryRange(space_size, alignment, left_padding,
12131214
&largest_gap_found, &max_occupied_addr);
12141215
// If the shadow doesn't fit, restrict the address space to make it fit.
12151216
if (shadow_start == 0) {
@@ -1229,9 +1230,9 @@ uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
12291230
}
12301231
RestrictMemoryToMaxAddress(new_max_vm);
12311232
high_mem_end = new_max_vm - 1;
1232-
space_size = (high_mem_end >> shadow_scale) + left_padding;
1233+
space_size = (high_mem_end >> shadow_scale);
12331234
VReport(2, "FindDynamicShadowStart, space_size = %p\n", (void *)space_size);
1234-
shadow_start = FindAvailableMemoryRange(space_size, alignment, granularity,
1235+
shadow_start = FindAvailableMemoryRange(space_size, alignment, left_padding,
12351236
nullptr, nullptr);
12361237
if (shadow_start == 0) {
12371238
Report("Unable to find a memory range after restricting VM.\n");
@@ -1272,10 +1273,15 @@ uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding,
12721273
mach_msg_type_number_t count = kRegionInfoSize;
12731274
kr = mach_vm_region_recurse(mach_task_self(), &address, &vmsize, &depth,
12741275
(vm_region_info_t)&vminfo, &count);
1275-
if (kr == KERN_INVALID_ADDRESS) {
1276+
1277+
// There are cases where going beyond the processes' max vm does
1278+
// not return KERN_INVALID_ADDRESS so we check for going beyond that
1279+
// max address as well.
1280+
if (kr == KERN_INVALID_ADDRESS || address > max_vm_address) {
12761281
// No more regions beyond "address", consider the gap at the end of VM.
12771282
address = max_vm_address;
12781283
vmsize = 0;
1284+
kr = -1; // break after this iteration.
12791285
} else {
12801286
if (max_occupied_addr) *max_occupied_addr = address + vmsize;
12811287
}

0 commit comments

Comments
 (0)