Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dfsan] Re-exec with no ASLR if memory layout is incompatible on Linux #85674

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

thurstond
Copy link
Contributor

DFSan's shadow mappings are incompatible with 32 bits of ASLR entropy ('sudo sysctl vm.mmap_rnd_bits=32; ninja check-dfsan') and it is difficult to fix this via increasing the size of the shadow mappings, due to the overhead of shadow memory. This patch works around the issue by detecting if the memory layout is incompatible, and if so, re-exec'ing without ASLR.

DFSan and MSan share copy-pasted shadow memory code, hence this workaround is ported from MSan:

  • "[msan] Re-exec with no ASLR if memory layout is incompatible on Linux" (58f7251)
  • "[msan] Add 'MappingDesc::ALLOCATOR' type and check it is available" (af2bf86)
    (which in turn are inspired by TSan: "Re-exec TSan with no ASLR if memory layout is incompatible on Linux" (0784b1e ))

aeubanks had remarked in #85142 (comment) that this issue occurs in Chromium: https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview

DFSan's shadow mappings are incompatible with 32 bits of ASLR entropy ('sudo sysctl vm.mmap_rnd_bits=32; ninja check-dfsan') and it is difficult to fix this via increasing the size of the shadow mappings, due to the overhead of shadow memory. This patch works around the issue by detecting if the memory layout is incompatible, and if so, re-exec'ing without ASLR.

DFSan and MSan share copy-pasted shadow memory code, hence this workaround is ported from MSan:
- "[msan] Re-exec with no ASLR if memory layout is incompatible on Linux" (llvm@58f7251)
- "[msan] Add 'MappingDesc::ALLOCATOR' type and check it is available" (llvm@af2bf86)
(which in turn are inspired by TSan: "Re-exec TSan with no ASLR if memory layout is incompatible on Linux" (llvm@0784b1e ))

aeubanks had remarked in llvm#85142 (comment) that this issue occurs in Chromium: https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview
@llvmbot
Copy link
Member

llvmbot commented Mar 18, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Thurston Dang (thurstond)

Changes

DFSan's shadow mappings are incompatible with 32 bits of ASLR entropy ('sudo sysctl vm.mmap_rnd_bits=32; ninja check-dfsan') and it is difficult to fix this via increasing the size of the shadow mappings, due to the overhead of shadow memory. This patch works around the issue by detecting if the memory layout is incompatible, and if so, re-exec'ing without ASLR.

DFSan and MSan share copy-pasted shadow memory code, hence this workaround is ported from MSan:

  • "[msan] Re-exec with no ASLR if memory layout is incompatible on Linux" (58f7251)
  • "[msan] Add 'MappingDesc::ALLOCATOR' type and check it is available" (af2bf86)
    (which in turn are inspired by TSan: "Re-exec TSan with no ASLR if memory layout is incompatible on Linux" (0784b1e ))

aeubanks had remarked in #85142 (comment) that this issue occurs in Chromium: https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview


Full diff: https://github.com/llvm/llvm-project/pull/85674.diff

3 Files Affected:

  • (modified) compiler-rt/lib/dfsan/dfsan.cpp (+53-13)
  • (modified) compiler-rt/lib/dfsan/dfsan_allocator.cpp (+3)
  • (modified) compiler-rt/lib/dfsan/dfsan_platform.h (+18-6)
diff --git a/compiler-rt/lib/dfsan/dfsan.cpp b/compiler-rt/lib/dfsan/dfsan.cpp
index 5e85c8fda3e230..7d435fb140ea97 100644
--- a/compiler-rt/lib/dfsan/dfsan.cpp
+++ b/compiler-rt/lib/dfsan/dfsan.cpp
@@ -33,6 +33,9 @@
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_report_decorator.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
+#if SANITIZER_LINUX
+#  include <sys/personality.h>
+#endif
 
 using namespace __dfsan;
 
@@ -1127,11 +1130,12 @@ static void CheckMemoryLayoutSanity() {
 
 // TODO: CheckMemoryRangeAvailability is based on msan.
 // Consider refactoring these into a shared implementation.
-static bool CheckMemoryRangeAvailability(uptr beg, uptr size) {
+static bool CheckMemoryRangeAvailability(uptr beg, uptr size, bool verbose) {
   if (size > 0) {
     uptr end = beg + size - 1;
     if (!MemoryRangeIsAvailable(beg, end)) {
-      Printf("FATAL: Memory range %p - %p is not available.\n", beg, end);
+      if (verbose)
+        Printf("FATAL: Memory range %p - %p is not available.\n", beg, end);
       return false;
     }
   }
@@ -1163,7 +1167,7 @@ static bool ProtectMemoryRange(uptr beg, uptr size, const char *name) {
 
 // TODO: InitShadow is based on msan.
 // Consider refactoring these into a shared implementation.
-bool InitShadow(bool init_origins) {
+bool InitShadow(bool init_origins, bool dry_run) {
   // Let user know mapping parameters first.
   VPrintf(1, "dfsan_init %p\n", (void *)&__dfsan::dfsan_init);
   for (unsigned i = 0; i < kMemoryLayoutSize; ++i)
@@ -1173,8 +1177,9 @@ bool InitShadow(bool init_origins) {
   CheckMemoryLayoutSanity();
 
   if (!MEM_IS_APP(&__dfsan::dfsan_init)) {
-    Printf("FATAL: Code %p is out of application range. Non-PIE build?\n",
-           (uptr)&__dfsan::dfsan_init);
+    if (!dry_run)
+      Printf("FATAL: Code %p is out of application range. Non-PIE build?\n",
+             (uptr)&__dfsan::dfsan_init);
     return false;
   }
 
@@ -1195,20 +1200,26 @@ bool InitShadow(bool init_origins) {
     bool protect = type == MappingDesc::INVALID ||
                    (!init_origins && type == MappingDesc::ORIGIN);
     CHECK(!(map && protect));
-    if (!map && !protect)
-      CHECK(type == MappingDesc::APP);
+    if (!map && !protect) {
+      CHECK(type == MappingDesc::APP || type == MappingDesc::ALLOCATOR);
+
+      if (dry_run && type == MappingDesc::ALLOCATOR &&
+          !CheckMemoryRangeAvailability(start, size, !dry_run))
+        return false;
+    }
     if (map) {
-      if (!CheckMemoryRangeAvailability(start, size))
+      if (dry_run && !CheckMemoryRangeAvailability(start, size, !dry_run))
         return false;
-      if (!MmapFixedSuperNoReserve(start, size, kMemoryLayout[i].name))
+      if (!dry_run &&
+          !MmapFixedSuperNoReserve(start, size, kMemoryLayout[i].name))
         return false;
-      if (common_flags()->use_madv_dontdump)
+      if (!dry_run && common_flags()->use_madv_dontdump)
         DontDumpShadowMemory(start, size);
     }
     if (protect) {
-      if (!CheckMemoryRangeAvailability(start, size))
+      if (dry_run && !CheckMemoryRangeAvailability(start, size, !dry_run))
         return false;
-      if (!ProtectMemoryRange(start, size, kMemoryLayout[i].name))
+      if (!dry_run && !ProtectMemoryRange(start, size, kMemoryLayout[i].name))
         return false;
     }
   }
@@ -1216,6 +1227,35 @@ bool InitShadow(bool init_origins) {
   return true;
 }
 
+bool InitShadowWithReExec(bool init_origins) {
+  // Start with dry run: check layout is ok, but don't print warnings because
+  // warning messages will cause tests to fail (even if we successfully re-exec
+  // after the warning).
+  bool success = InitShadow(init_origins, true);
+  if (!success) {
+#if SANITIZER_LINUX
+    // Perhaps ASLR entropy is too high. If ASLR is enabled, re-exec without it.
+    int old_personality = personality(0xffffffff);
+    bool aslr_on =
+        (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0);
+
+    if (aslr_on) {
+      VReport(1,
+              "WARNING: DataflowSanitizer: memory layout is incompatible, "
+              "possibly due to high-entropy ASLR.\n"
+              "Re-execing with fixed virtual address space.\n"
+              "N.B. reducing ASLR entropy is preferable.\n");
+      CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
+      ReExec();
+    }
+#endif
+  }
+
+  // The earlier dry run didn't actually map or protect anything. Run again in
+  // non-dry run mode.
+  return success && InitShadow(init_origins, false);
+}
+
 static void DFsanInit(int argc, char **argv, char **envp) {
   CHECK(!dfsan_init_is_running);
   if (dfsan_inited)
@@ -1229,7 +1269,7 @@ static void DFsanInit(int argc, char **argv, char **envp) {
 
   CheckASLR();
 
-  InitShadow(dfsan_get_track_origins());
+  InitShadowWithReExec(dfsan_get_track_origins());
 
   initialize_interceptors();
 
diff --git a/compiler-rt/lib/dfsan/dfsan_allocator.cpp b/compiler-rt/lib/dfsan/dfsan_allocator.cpp
index df8be2cf5ae04c..63475f434cd100 100644
--- a/compiler-rt/lib/dfsan/dfsan_allocator.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_allocator.cpp
@@ -37,6 +37,9 @@ struct DFsanMapUnmapCallback {
   void OnUnmap(uptr p, uptr size) const { dfsan_set_label(0, (void *)p, size); }
 };
 
+// Note: to ensure that the allocator is compatible with the application memory
+// layout (especially with high-entropy ASLR), kSpaceBeg and kSpaceSize must be
+// duplicated as MappingDesc::ALLOCATOR in dfsan_platform.h.
 #if defined(__aarch64__)
 const uptr kAllocatorSpace = 0xE00000000000ULL;
 #else
diff --git a/compiler-rt/lib/dfsan/dfsan_platform.h b/compiler-rt/lib/dfsan/dfsan_platform.h
index b849b4b528ad29..01f0de47d960d3 100644
--- a/compiler-rt/lib/dfsan/dfsan_platform.h
+++ b/compiler-rt/lib/dfsan/dfsan_platform.h
@@ -27,10 +27,19 @@ using __sanitizer::uptr;
 struct MappingDesc {
   uptr start;
   uptr end;
-  enum Type { INVALID, APP, SHADOW, ORIGIN } type;
+  enum Type {
+    INVALID = 1,
+    ALLOCATOR = 2,
+    APP = 4,
+    SHADOW = 8,
+    ORIGIN = 16,
+  } type;
   const char *name;
 };
 
+// Note: MappingDesc::ALLOCATOR entries are only used to check for memory
+// layout compatibility. The actual allocation settings are in
+// dfsan_allocator.cpp, which need to be kept in sync.
 #if SANITIZER_LINUX && SANITIZER_WORDSIZE == 64
 
 #  if defined(__aarch64__)
@@ -53,7 +62,8 @@ const MappingDesc kMemoryLayout[] = {
     {0X0B00000000000, 0X0C00000000000, MappingDesc::SHADOW, "shadow-10-13"},
     {0X0C00000000000, 0X0D00000000000, MappingDesc::INVALID, "invalid"},
     {0X0D00000000000, 0X0E00000000000, MappingDesc::ORIGIN, "origin-10-13"},
-    {0X0E00000000000, 0X1000000000000, MappingDesc::APP, "app-15"},
+    {0X0E00000000000, 0X0E40000000000, MappingDesc::ALLOCATOR, "allocator"},
+    {0X0E40000000000, 0X1000000000000, MappingDesc::APP, "app-15"},
 };
 #    define MEM_TO_SHADOW(mem) ((uptr)mem ^ 0xB00000000000ULL)
 #    define SHADOW_TO_ORIGIN(shadow) (((uptr)(shadow)) + 0x200000000000ULL)
@@ -76,7 +86,8 @@ const MappingDesc kMemoryLayout[] = {
     {0x510000000000ULL, 0x600000000000ULL, MappingDesc::APP, "app-2"},
     {0x600000000000ULL, 0x610000000000ULL, MappingDesc::ORIGIN, "origin-1"},
     {0x610000000000ULL, 0x700000000000ULL, MappingDesc::INVALID, "invalid"},
-    {0x700000000000ULL, 0x800000000000ULL, MappingDesc::APP, "app-3"}};
+    {0x700000000000ULL, 0x740000000000ULL, MappingDesc::ALLOCATOR, "allocator"},
+    {0x740000000000ULL, 0x800000000000ULL, MappingDesc::APP, "app-3"}};
 #    define MEM_TO_SHADOW(mem) (((uptr)(mem)) ^ 0x500000000000ULL)
 #    define SHADOW_TO_ORIGIN(mem) (((uptr)(mem)) + 0x100000000000ULL)
 #  endif
@@ -93,20 +104,21 @@ const uptr kMemoryLayoutSize = sizeof(kMemoryLayout) / sizeof(kMemoryLayout[0]);
 __attribute__((optimize("unroll-loops")))
 #endif
 inline bool
-addr_is_type(uptr addr, MappingDesc::Type mapping_type) {
+addr_is_type(uptr addr, int mapping_types) {
 // It is critical for performance that this loop is unrolled (because then it is
 // simplified into just a few constant comparisons).
 #ifdef __clang__
 #  pragma unroll
 #endif
   for (unsigned i = 0; i < kMemoryLayoutSize; ++i)
-    if (kMemoryLayout[i].type == mapping_type &&
+    if ((kMemoryLayout[i].type & mapping_types) &&
         addr >= kMemoryLayout[i].start && addr < kMemoryLayout[i].end)
       return true;
   return false;
 }
 
-#define MEM_IS_APP(mem) addr_is_type((uptr)(mem), MappingDesc::APP)
+#define MEM_IS_APP(mem) \
+  (addr_is_type((uptr)(mem), MappingDesc::APP | MappingDesc::ALLOCATOR))
 #define MEM_IS_SHADOW(mem) addr_is_type((uptr)(mem), MappingDesc::SHADOW)
 #define MEM_IS_ORIGIN(mem) addr_is_type((uptr)(mem), MappingDesc::ORIGIN)
 

@aeubanks
Copy link
Contributor

thanks for doing this!

likely leading to a segfault down the line)
Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(looks right to me and if this passes tests with the ASLR change then lgtm)

@thurstond
Copy link
Contributor Author

(looks right to me and if this passes tests with the ASLR change then lgtm)

Thanks for the review!

The test results are better with this patch.

At any ASLR entropy setting, custom.cpp fails with or without this patch.

With 32 bits of ASLR entropy:

  • before this patch: there's 31 additional tests (including release_shadow_space.c) that fail
  • after this patch: 1 test (release_shadow_space.c) occasionally fails, but I think it's a flaky test that needs adjustment to the margin of error (Assertion 'after_fixed_mmap <= before + delta + mmap_origin_chain_kb' failed; "The assert checks the memory up to a delta."). In any case, this test fails before the patch, so this patch is an improvement.

@thurstond thurstond merged commit 62ed009 into llvm:main Mar 20, 2024
4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
llvm#85674)

DFSan's shadow mappings are incompatible with 32 bits of ASLR entropy
('sudo sysctl vm.mmap_rnd_bits=32; ninja check-dfsan') and it is
difficult to fix this via increasing the size of the shadow mappings,
due to the overhead of shadow memory. This patch works around the issue
by detecting if the memory layout is incompatible, and if so,
re-exec'ing without ASLR.

DFSan and MSan share copy-pasted shadow memory code, hence this
workaround is ported from MSan:
- "[msan] Re-exec with no ASLR if memory layout is incompatible on
Linux"
(llvm@58f7251)
- "[msan] Add 'MappingDesc::ALLOCATOR' type and check it is available"
(llvm@af2bf86)
(which in turn are inspired by TSan: "Re-exec TSan with no ASLR if
memory layout is incompatible on Linux"
(llvm@0784b1e
))

aeubanks had remarked in
llvm#85142 (comment)
that this issue occurs in Chromium:
https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants