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

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

Merged
merged 4 commits into from
Mar 15, 2024

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Mar 13, 2024

This ports the change from TSan (0784b1e).

A key difference is that TSan initializes the allocator prior to
CheckAndProtect, while MSan initializes the allocator afterwards;
this slightly simplifies the MSan patch. Nonetheless, we need to check
that the allocator layout is compatible with ASLR. This must be done silently ("dry run") because any warning messages may cause tests to fail.

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.

N.B. aggressive ASLR may also cause the app to overlap with the allocator region; this was fixed in af2bf86

@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

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

Author: Thurston Dang (thurstond)

Changes

This ports the change from TSan (0784b1e).

A key difference is that TSan initializes the allocator prior to
CheckAndProtect, while MSan initializes the allocator afterwards;
this slightly simplifies the MSan patch. Nonetheless, we need to check
that the allocator layout is compatible with ASLR. Since the information is
not readily available in msan.h, we duplicate the information from
msan_allocator.cpp, and create a new MappingDesc::ALLOCATOR type.

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.


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

4 Files Affected:

  • (modified) compiler-rt/lib/msan/msan.cpp (+1-1)
  • (modified) compiler-rt/lib/msan/msan.h (+18-11)
  • (modified) compiler-rt/lib/msan/msan_allocator.cpp (+3)
  • (modified) compiler-rt/lib/msan/msan_linux.cpp (+56-13)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 3cdf10c149902c..a2fc27de1901b4 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -467,7 +467,7 @@ void __msan_init() {
   __msan_clear_on_return();
   if (__msan_get_track_origins())
     VPrintf(1, "msan_track_origins\n");
-  if (!InitShadow(__msan_get_track_origins())) {
+  if (!InitShadowWithReExec(__msan_get_track_origins())) {
     Printf("FATAL: MemorySanitizer can not mmap the shadow memory.\n");
     Printf("FATAL: Make sure to compile with -fPIE and to link with -pie.\n");
     Printf("FATAL: Disabling ASLR is known to cause this error.\n");
diff --git a/compiler-rt/lib/msan/msan.h b/compiler-rt/lib/msan/msan.h
index 710447a3e1a357..f50358727448db 100644
--- a/compiler-rt/lib/msan/msan.h
+++ b/compiler-rt/lib/msan/msan.h
@@ -32,13 +32,13 @@
 struct MappingDesc {
   uptr start;
   uptr end;
-  enum Type {
-    INVALID, APP, SHADOW, ORIGIN
-  } type;
+  enum Type { INVALID, ALLOCATOR, APP, SHADOW, ORIGIN } type;
   const char *name;
 };
 
-
+// Note: MappingDesc::ALLOCATOR entries are only used to check for memory
+// layout compatibility. The actual allocation settings are in
+// msan_allocator.cpp, which need to be kept in sync.
 #if SANITIZER_LINUX && defined(__mips64)
 
 // MIPS64 maps:
@@ -84,7 +84,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)
@@ -106,7 +107,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(shadow) (((uptr)(shadow)) + 0x100000000000ULL)
 
@@ -118,7 +120,8 @@ const MappingDesc kMemoryLayout[] = {
     {0x180200000000ULL, 0x1C0000000000ULL, MappingDesc::INVALID, "invalid"},
     {0x1C0000000000ULL, 0x2C0200000000ULL, MappingDesc::ORIGIN, "origin"},
     {0x2C0200000000ULL, 0x300000000000ULL, MappingDesc::INVALID, "invalid"},
-    {0x300000000000ULL, 0x800000000000ULL, MappingDesc::APP, "high memory"}};
+    {0x300000000000ULL, 0x320000000000ULL, MappingDesc::ALLOCATOR, "allocator"},
+    {0x320000000000ULL, 0x800000000000ULL, MappingDesc::APP, "high memory"}};
 
 // Various kernels use different low end ranges but we can combine them into one
 // big range. They also use different high end ranges but we can map them all to
@@ -141,7 +144,8 @@ const MappingDesc kMemoryLayout[] = {
     {0x180000000000ULL, 0x1C0000000000ULL, MappingDesc::INVALID, "invalid"},
     {0x1C0000000000ULL, 0x2C0000000000ULL, MappingDesc::ORIGIN, "origin"},
     {0x2C0000000000ULL, 0x440000000000ULL, MappingDesc::INVALID, "invalid"},
-    {0x440000000000ULL, 0x500000000000ULL, MappingDesc::APP, "high memory"}};
+    {0x440000000000ULL, 0x460000000000ULL, MappingDesc::ALLOCATOR, "allocator"},
+    {0x460000000000ULL, 0x500000000000ULL, MappingDesc::APP, "high memory"}};
 
 #define MEM_TO_SHADOW(mem) \
   ((((uptr)(mem)) & ~0xC00000000000ULL) + 0x080000000000ULL)
@@ -208,7 +212,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)
 
@@ -236,7 +241,9 @@ inline bool addr_is_type(uptr addr, MappingDesc::Type mapping_type) {
   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) || \
+   addr_is_type((uptr)(mem), 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)
 
@@ -250,7 +257,7 @@ extern bool msan_init_is_running;
 extern int msan_report_count;
 
 bool ProtectRange(uptr beg, uptr end);
-bool InitShadow(bool init_origins);
+bool InitShadowWithReExec(bool init_origins);
 char *GetProcSelfMaps();
 void InitializeInterceptors();
 
diff --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp
index 0b2dd2b2f1883d..b1bc5b9390f75b 100644
--- a/compiler-rt/lib/msan/msan_allocator.cpp
+++ b/compiler-rt/lib/msan/msan_allocator.cpp
@@ -48,6 +48,9 @@ struct MsanMapUnmapCallback {
   }
 };
 
+// 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 msan.h.
 #if defined(__mips64)
 static const uptr kMaxAllowedMallocSize = 2UL << 30;
 
diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp
index c7ecb7cad56661..41fa29d4216c50 100644
--- a/compiler-rt/lib/msan/msan_linux.cpp
+++ b/compiler-rt/lib/msan/msan_linux.cpp
@@ -20,6 +20,9 @@
 #  include <signal.h>
 #  include <stdio.h>
 #  include <stdlib.h>
+#  if SANITIZER_LINUX
+#    include <sys/personality.h>
+#  endif
 #  include <sys/resource.h>
 #  include <sys/time.h>
 #  include <unistd.h>
@@ -43,11 +46,13 @@ void ReportMapRange(const char *descr, uptr beg, uptr size) {
   }
 }
 
-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 0x%zx - 0x%zx is not available.\n", beg, end);
+      if (verbose)
+        Printf("FATAL: Memory range 0x%zx - 0x%zx is not available.\n", beg,
+               end);
       return false;
     }
   }
@@ -86,7 +91,7 @@ static void CheckMemoryLayoutSanity() {
     CHECK(addr_is_type(start, type));
     CHECK(addr_is_type((start + end) / 2, type));
     CHECK(addr_is_type(end - 1, type));
-    if (type == MappingDesc::APP) {
+    if (type == MappingDesc::APP || type == MappingDesc::ALLOCATOR) {
       uptr addr = start;
       CHECK(MEM_IS_SHADOW(MEM_TO_SHADOW(addr)));
       CHECK(MEM_IS_ORIGIN(MEM_TO_ORIGIN(addr)));
@@ -106,7 +111,7 @@ static void CheckMemoryLayoutSanity() {
   }
 }
 
-bool InitShadow(bool init_origins) {
+static bool InitShadow(bool init_origins, bool dry_run) {
   // Let user know mapping parameters first.
   VPrintf(1, "__msan_init %p\n", reinterpret_cast<void *>(&__msan_init));
   for (unsigned i = 0; i < kMemoryLayoutSize; ++i)
@@ -116,8 +121,9 @@ bool InitShadow(bool init_origins) {
   CheckMemoryLayoutSanity();
 
   if (!MEM_IS_APP(&__msan_init)) {
-    Printf("FATAL: Code %p is out of application range. Non-PIE build?\n",
-           reinterpret_cast<void *>(&__msan_init));
+    if (!dry_run)
+      Printf("FATAL: Code %p is out of application range. Non-PIE build?\n",
+             reinterpret_cast<void *>(&__msan_init));
     return false;
   }
 
@@ -138,20 +144,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 (type == MappingDesc::ALLOCATOR &&
+          !CheckMemoryRangeAvailability(start, size, !dry_run))
+        return false;
+    }
     if (map) {
-      if (!CheckMemoryRangeAvailability(start, size))
+      if (!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 (!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;
     }
   }
@@ -159,6 +171,37 @@ 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(__msan_get_track_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: MemorySanitizer: 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.
+  success = InitShadow(__msan_get_track_origins(), false);
+
+  return success;
+}
+
 static void MsanAtExit(void) {
   if (flags()->print_stats && (flags()->atexit || msan_report_count > 0))
     ReportStats();

@@ -106,7 +107,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"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need another enum value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

InitShadow is called before the allocator is initialized, so we need a way to tell InitShadow that we want to check that the allocator memory range is available (but not map it, because the allocator will do that).

The existing enum values don't suffice:

  • MappingDesc::APP does not call CheckMemoryRangeAvailability()
  • MappingDesc::INVALID does call CheckMemoryRangeAvailability(), but it will also be protected, which we don't want
  • MappingDesc::SHADOW also calls CheckMemoryRangeAvailability(), but it will map the memory, which we also don't want
  • MappingDesc::ALLOCATOR will call CheckMemoryRangeAvailability() but will not map or protect the memory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the kernel might randomly map the app into the same region that the allocator expects (this isn't a problem with low-entropy ASLR, because it never maps into the bottom part of the app region, which is where the allocator typically lives). We therefore need to enforce that the APP and ALLOCATOR regions are disjoint.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it possible to introduce MappingDesc::ALLOCATOR in a separate patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean introduce MappingDesc::ALLOCATOR as an NFC patch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, and related conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #85153

thurstond added a commit to thurstond/llvm-project that referenced this pull request Mar 13, 2024
MSan divides the virtual address space into APP, INVALID, SHADOW and
ORIGIN memory. The allocator usually just steals a bit of the APP
address space: typically the bottom portion of the PIE binaries
section, which works because the Linux kernel maps from the top of
the PIE binaries section. However, if ASLR is very aggressive, the
binary may end up mapped in the same location where the allocator
wants to live; this results in a segfault.

This patch adds in a MappingDesc::ALLOCATOR type and enforces that
the memory range for the allocator is not occupied by anything else.

Since the allocator range information is not readily available in msan.h,
we duplicate the information from msan_allocator.cpp.

Note: aggressive ASLR can also lead to a different type of failure, where
the PIE binaries/libaries are mapped entirely outside of the APP/ALLOCATOR
sections; that will be addressed in a separate patch (llvm#85142).
thurstond added a commit that referenced this pull request Mar 14, 2024
…85153)

MSan divides the virtual address space into APP, INVALID, SHADOW and
ORIGIN memory. The allocator usually just steals a bit of the APP
address space: typically the bottom portion of the PIE binaries section,
which works because the Linux kernel maps from the top of the PIE
binaries section. However, if ASLR is very aggressive, the binary may
end up mapped in the same location where the allocator wants to live;
this results in a segfault.

This patch adds in a MappingDesc::ALLOCATOR type and enforces that the
memory range for the allocator is not occupied by anything else.

Since the allocator range information is not readily available in
msan.h, we duplicate the information from msan_allocator.cpp.

Note: aggressive ASLR can also lead to a different type of failure,
where the PIE binaries/libraries are mapped entirely outside of the
APP/ALLOCATOR sections; that will be addressed in a separate patch
(#85142).
This ports the change from TSan (llvm@0784b1e).

A key difference is that TSan initializes the allocator prior to
CheckAndProtect, while MSan initializes the allocator afterwards;
this slightly simplifies the MSan patch. Nonetheless, we need to check
that the allocator layout is compatible with ASLR. Since the information is
not readily available in msan.h, we duplicate the information from
msan_allocator.cpp, and create a new MappingDesc::ALLOCATOR type.

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.
… we must

still return false. (The second InitShadow call does not perform the
same checks that the first InitShadow call did.)
@@ -197,7 +197,7 @@ bool InitShadowWithReExec(bool init_origins) {

// The earlier dry run didn't actually map or protect anything. Run again in
// non-dry run mode.
success = InitShadow(__msan_get_track_origins(), false);
success = success && InitShadow(__msan_get_track_origins(), false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

return success && InitShadow(__msan_get_track_origins(), false);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@thurstond thurstond merged commit 58f7251 into llvm:main Mar 15, 2024
3 of 4 checks passed
Chilledheart added a commit to Chilledheart/yass that referenced this pull request Mar 18, 2024
Chilledheart added a commit to Chilledheart/yass that referenced this pull request Mar 18, 2024
Chilledheart added a commit to Chilledheart/yass that referenced this pull request Mar 18, 2024
@aeubanks
Copy link
Contributor

We're seeing similar issues with dfsan, e.g. https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview

 FAIL: DataFlowSanitizer-x86_64 :: lookup_table.c (22061 of 81767)
 ******************** TEST 'DataFlowSanitizer-x86_64 :: lookup_table.c' FAILED ********************
 Exit Code: 139
 
 Command Output (stderr):
 --
 RUN: at line 1: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./bin/clang  -fsanitize=dataflow   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/dfsan/lookup_table.c -mllvm -dfsan-combine-offset-labels-on-gep=false -mllvm -dfsan-combine-pointer-labels-on-load=false -mllvm -dfsan-combine-taint-lookup-table=remap_to_upper -DLOOKUP_TABLE -o /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp &&  /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 + /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./bin/clang -fsanitize=dataflow -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/dfsan/lookup_table.c -mllvm -dfsan-combine-offset-labels-on-gep=false -mllvm -dfsan-combine-pointer-labels-on-load=false -mllvm -dfsan-combine-taint-lookup-table=remap_to_upper -DLOOKUP_TABLE -o /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 + /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 FATAL: Code 0x618710e92e60 is out of application range. Non-PIE build?
 /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.script: line 3: 850288 Segmentation fault      (core dumped) /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp

thurstond added a commit to thurstond/llvm-project that referenced this pull request Mar 18, 2024
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
@thurstond
Copy link
Contributor Author

We're seeing similar issues with dfsan, e.g. https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/5066/overview

 FAIL: DataFlowSanitizer-x86_64 :: lookup_table.c (22061 of 81767)
 ******************** TEST 'DataFlowSanitizer-x86_64 :: lookup_table.c' FAILED ********************
 Exit Code: 139
 
 Command Output (stderr):
 --
 RUN: at line 1: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./bin/clang  -fsanitize=dataflow   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/dfsan/lookup_table.c -mllvm -dfsan-combine-offset-labels-on-gep=false -mllvm -dfsan-combine-pointer-labels-on-load=false -mllvm -dfsan-combine-taint-lookup-table=remap_to_upper -DLOOKUP_TABLE -o /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp &&  /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 + /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./bin/clang -fsanitize=dataflow -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/dfsan/lookup_table.c -mllvm -dfsan-combine-offset-labels-on-gep=false -mllvm -dfsan-combine-pointer-labels-on-load=false -mllvm -dfsan-combine-taint-lookup-table=remap_to_upper -DLOOKUP_TABLE -o /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 + /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp
 FATAL: Code 0x618710e92e60 is out of application range. Non-PIE build?
 /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.script: line 3: 850288 Segmentation fault      (core dumped) /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/dfsan/X86_64Config/Output/lookup_table.c.tmp

Thank you Arthur for the heads up! I've drafted a pull request to port this patch to DFSan (#85674)

thurstond added a commit that referenced this pull request Mar 20, 2024
#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"
(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
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 21, 2024
…lvm#85153)

MSan divides the virtual address space into APP, INVALID, SHADOW and
ORIGIN memory. The allocator usually just steals a bit of the APP
address space: typically the bottom portion of the PIE binaries section,
which works because the Linux kernel maps from the top of the PIE
binaries section. However, if ASLR is very aggressive, the binary may
end up mapped in the same location where the allocator wants to live;
this results in a segfault.

This patch adds in a MappingDesc::ALLOCATOR type and enforces that the
memory range for the allocator is not occupied by anything else.

Since the allocator range information is not readily available in
msan.h, we duplicate the information from msan_allocator.cpp.

Note: aggressive ASLR can also lead to a different type of failure,
where the PIE binaries/libraries are mapped entirely outside of the
APP/ALLOCATOR sections; that will be addressed in a separate patch
(llvm#85142).

(cherry picked from commit af2bf86)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 21, 2024
…llvm#85142)

This ports the change from TSan
(llvm@0784b1e).

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.

N.B. aggressive ASLR may also cause the app to overlap with the
allocator region; for MSan, this was fixed in
llvm@af2bf86

(cherry picked from commit 58f7251)
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
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 23, 2024
…lvm#85153)

MSan divides the virtual address space into APP, INVALID, SHADOW and
ORIGIN memory. The allocator usually just steals a bit of the APP
address space: typically the bottom portion of the PIE binaries section,
which works because the Linux kernel maps from the top of the PIE
binaries section. However, if ASLR is very aggressive, the binary may
end up mapped in the same location where the allocator wants to live;
this results in a segfault.

This patch adds in a MappingDesc::ALLOCATOR type and enforces that the
memory range for the allocator is not occupied by anything else.

Since the allocator range information is not readily available in
msan.h, we duplicate the information from msan_allocator.cpp.

Note: aggressive ASLR can also lead to a different type of failure,
where the PIE binaries/libraries are mapped entirely outside of the
APP/ALLOCATOR sections; that will be addressed in a separate patch
(llvm#85142).

(cherry picked from commit af2bf86)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 23, 2024
…llvm#85142)

This ports the change from TSan
(llvm@0784b1e).

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.

N.B. aggressive ASLR may also cause the app to overlap with the
allocator region; for MSan, this was fixed in
llvm@af2bf86

(cherry picked from commit 58f7251)
tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 27, 2024
…lvm#85153)

MSan divides the virtual address space into APP, INVALID, SHADOW and
ORIGIN memory. The allocator usually just steals a bit of the APP
address space: typically the bottom portion of the PIE binaries section,
which works because the Linux kernel maps from the top of the PIE
binaries section. However, if ASLR is very aggressive, the binary may
end up mapped in the same location where the allocator wants to live;
this results in a segfault.

This patch adds in a MappingDesc::ALLOCATOR type and enforces that the
memory range for the allocator is not occupied by anything else.

Since the allocator range information is not readily available in
msan.h, we duplicate the information from msan_allocator.cpp.

Note: aggressive ASLR can also lead to a different type of failure,
where the PIE binaries/libraries are mapped entirely outside of the
APP/ALLOCATOR sections; that will be addressed in a separate patch
(llvm#85142).

(cherry picked from commit af2bf86)
tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 27, 2024
…llvm#85142)

This ports the change from TSan
(llvm@0784b1e).

Testing notes: run 'sudo sysctl vm.mmap_rnd_bits=32; ninja check-msan'
before and after this patch.

N.B. aggressive ASLR may also cause the app to overlap with the
allocator region; for MSan, this was fixed in
llvm@af2bf86

(cherry picked from commit 58f7251)
thurstond added a commit to thurstond/llvm-project that referenced this pull request Mar 28, 2024
…igins() in InitShadowWithReExec

This fixes a nit I had accidentally introduced in llvm#85142

I don't think the value of __msan_get_track_origins() will change between the start and end of InitShadowWithReExec, but it's cleaner to use the parameter.
vitalybuka pushed a commit that referenced this pull request Apr 1, 2024
…igins() in InitShadowWithReExec (#86994)

This fixes a nit I had accidentally introduced in
#85142

I don't think the value of __msan_get_track_origins() will change
between the start and end of InitShadowWithReExec, but it's cleaner to
use the parameter.
@pointhex pointhex mentioned this pull request May 7, 2024
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.

4 participants