Skip to content

Commit c6f470c

Browse files
committed
[sanitizer-common] [Darwin] Fix overlapping dyld segment addresses
This fixes two problems: - dyld itself resides within the shared cache. MemoryMappingLayout incorrectly computes the slide for dyld's segments, causing them to (appear to) overlap with other modules. - The MemoryMappingLayout ranges on Darwin are not disjoint due to the fact that the LINKEDIT segments overlap for each module. This makes them disjoint. This adds a check for disjointness, and a runtime warning if this is ever violated (as that suggests issues in the sanitizer memory mapping). There is now a test to ensure that these problems do not recur. rdar://163149325
1 parent 32693d9 commit c6f470c

File tree

3 files changed

+86
-8
lines changed

3 files changed

+86
-8
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ class MemoryMappingLayout : public MemoryMappingLayoutBase {
104104
// Adds all mapped objects into a vector.
105105
void DumpListOfModules(InternalMmapVectorNoCtor<LoadedModule> *modules);
106106

107+
# if SANITIZER_APPLE
108+
// Verify that the memory mapping is well-formed. (e.g. mappings do not
109+
// overlap)
110+
bool Verify();
111+
# endif
112+
107113
protected:
108114
#if SANITIZER_APPLE
109115
virtual const ImageHeader *CurrentImageHeader();

compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) {
8282

8383
MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) {
8484
Reset();
85+
Verify();
8586
}
8687

8788
MemoryMappingLayout::~MemoryMappingLayout() {
@@ -187,6 +188,7 @@ typedef struct dyld_shared_cache_dylib_text_info
187188

188189
extern bool _dyld_get_shared_cache_uuid(uuid_t uuid);
189190
extern const void *_dyld_get_shared_cache_range(size_t *length);
191+
extern intptr_t _dyld_get_image_slide(const struct mach_header* mh);
190192
extern int dyld_shared_cache_iterate_text(
191193
const uuid_t cacheUuid,
192194
void (^callback)(const dyld_shared_cache_dylib_text_info *info));
@@ -255,16 +257,16 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment,
255257
layout_data->current_load_cmd_count--;
256258
if (((const load_command *)lc)->cmd == kLCSegment) {
257259
const SegmentCommand* sc = (const SegmentCommand *)lc;
260+
if (strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == 0) {
261+
// The LINKEDIT sections alias, so we ignore these sections to
262+
// ensure our mappings are disjoint.
263+
return false;
264+
}
265+
258266
uptr base_virt_addr, addr_mask;
259267
if (layout_data->current_image == kDyldImageIdx) {
260-
base_virt_addr = (uptr)get_dyld_hdr();
261-
// vmaddr is masked with 0xfffff because on macOS versions < 10.12,
262-
// it contains an absolute address rather than an offset for dyld.
263-
// To make matters even more complicated, this absolute address
264-
// isn't actually the absolute segment address, but the offset portion
265-
// of the address is accurate when combined with the dyld base address,
266-
// and the mask will give just this offset.
267-
addr_mask = 0xfffff;
268+
base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr());
269+
addr_mask = ~0;
268270
} else {
269271
base_virt_addr =
270272
(uptr)_dyld_get_image_vmaddr_slide(layout_data->current_image);
@@ -445,6 +447,56 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
445447
return false;
446448
}
447449

450+
// NOTE: Verify expects to be called immediately after Reset(), since otherwise
451+
// it may miss some mappings. Verify will Reset() the layout after verification.
452+
bool MemoryMappingLayout::Verify() {
453+
InternalMmapVector<char> module_name(kMaxPathLength);
454+
MemoryMappedSegment segment(module_name.data(), module_name.size());
455+
MemoryMappedSegmentData data;
456+
segment.data_ = &data;
457+
458+
InternalMmapVector<MemoryMappedSegment> segments;
459+
while (Next(&segment)) {
460+
// skip the __PAGEZERO segment, its vmsize is 0
461+
if (segment.filename[0] == '\0' || (segment.start == segment.end))
462+
continue;
463+
464+
segments.push_back(segment);
465+
}
466+
467+
// Verify that none of the segments overlap:
468+
// 1. Sort the segments by the start address
469+
// 2. Check that every segment starts after the previous one ends.
470+
Sort(segments.data(), segments.size(),
471+
[](MemoryMappedSegment& a, MemoryMappedSegment& b) {
472+
return a.start < b.start;
473+
});
474+
475+
// To avoid spam, we only print the report message once-per-process.
476+
static bool invalid_module_map_reported = false;
477+
bool well_formed = true;
478+
479+
for (size_t i = 1; i < segments.size(); i++) {
480+
uptr cur_start = segments[i].start;
481+
uptr prev_end = segments[i - 1].end;
482+
if (cur_start < prev_end) {
483+
well_formed = false;
484+
VReport(2, "Overlapping mappings: cur_start = %p prev_end = %p\n",
485+
(void*)cur_start, (void*)prev_end);
486+
if (!invalid_module_map_reported) {
487+
Report(
488+
"WARN: Invalid dyld module map detected. This is most likely a bug "
489+
"in the sanitizer.\n");
490+
Report("WARN: Backtraces may be unreliable.\n");
491+
invalid_module_map_reported = true;
492+
}
493+
}
494+
}
495+
496+
Reset();
497+
return well_formed;
498+
}
499+
448500
void MemoryMappingLayout::DumpListOfModules(
449501
InternalMmapVectorNoCtor<LoadedModule> *modules) {
450502
Reset();
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// This test simply checks that the "Invalid module map" warning is not printed
2+
// in the output of a backtrace.
3+
4+
// RUN: %clangxx_asan -O0 -g %s -o %t.executable
5+
// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s
6+
7+
// CHECK-NOT: WARN: Invalid module map
8+
9+
#include <cstdlib>
10+
11+
extern "C" void foo(int *a) { *a = 5; }
12+
13+
int main() {
14+
int *a = (int *)malloc(sizeof(int));
15+
if (!a)
16+
return 0;
17+
free(a);
18+
foo(a);
19+
return 0;
20+
}

0 commit comments

Comments
 (0)