[compiler-rt] 82a6152 - [asan] Don't double poison secondary allocations

[Vitaly Buka via llvm-commits]

Author: Vitaly Buka
Date: 2023-06-22T12:35:21-07:00
New Revision: 82a61523443ccfeb4302556f58b0427afb7c474f

URL: https://github.com/llvm/llvm-project/commit/82a61523443ccfeb4302556f58b0427afb7c474f
DIFF: https://github.com/llvm/llvm-project/commit/82a61523443ccfeb4302556f58b0427afb7c474f.diff

LOG: [asan] Don't double poison secondary allocations

Sanitizers allocate shadow and memory as MAP_NORESERVE.

User memory can stay this way and do not increase RSS as long as we
don't store there.

The shadow unpoisoning also can avoid RSS increase for zeroed pages.
However as soon we poison the shadow, we need the page in RSS.

To avoid unnececary RSS increase we should not poison memory just before
unpoisoning them.

Depends on D153497.

Reviewed By: thurston

Differential Revision: https://reviews.llvm.org/D153500

Added: 
    

Modified: 
    compiler-rt/lib/asan/asan_allocator.cpp
    compiler-rt/test/sanitizer_common/TestCases/Posix/huge_malloc.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp
index 1e3c67b86f31c..fab58c12c7a6b 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -288,7 +288,13 @@ void AsanMapUnmapCallback::OnMap(uptr p, uptr size) const {
 
 void AsanMapUnmapCallback::OnMapSecondary(uptr p, uptr size, uptr user_begin,
                                           uptr user_size) const {
-  PoisonShadow(p, size, kAsanHeapLeftRedzoneMagic);
+  uptr user_end = RoundDownTo(user_begin + user_size, ASAN_SHADOW_GRANULARITY);
+  user_begin = RoundUpTo(user_begin, ASAN_SHADOW_GRANULARITY);
+  // The secondary mapping will be immediately returned to user, no value
+  // poisoning that with non-zero just before unpoisoning by Allocate(). So just
+  // poison head/tail invisible to Allocate().
+  PoisonShadow(p, user_begin - p, kAsanHeapLeftRedzoneMagic);
+  PoisonShadow(user_end, size - (user_end - p), kAsanHeapLeftRedzoneMagic);
   // Statistics.
   AsanStats &thread_stats = GetCurrentThreadStats();
   thread_stats.mmaps++;
@@ -550,9 +556,10 @@ struct Allocator {
     uptr needed_size = rounded_size + rz_size;
     if (alignment > min_alignment)
       needed_size += alignment;
+    bool from_primary = PrimaryAllocator::CanAllocate(needed_size, alignment);
     // If we are allocating from the secondary allocator, there will be no
     // automatic right redzone, so add the right redzone manually.
-    if (!PrimaryAllocator::CanAllocate(needed_size, alignment))
+    if (!from_primary)
       needed_size += rz_size;
     CHECK(IsAligned(needed_size, min_alignment));
     if (size > kMaxAllowedMallocSize || needed_size > kMaxAllowedMallocSize ||
@@ -584,15 +591,6 @@ struct Allocator {
       ReportOutOfMemory(size, stack);
     }
 
-    if (*(u8 *)MEM_TO_SHADOW((uptr)allocated) == 0 && CanPoisonMemory()) {
-      // Heap poisoning is enabled, but the allocator provides an unpoisoned
-      // chunk. This is possible if CanPoisonMemory() was false for some
-      // time, for example, due to flags()->start_disabled.
-      // Anyway, poison the block before using it for anything else.
-      uptr allocated_size = allocator.GetActuallyAllocatedSize(allocated);
-      PoisonShadow((uptr)allocated, allocated_size, kAsanHeapLeftRedzoneMagic);
-    }
-
     uptr alloc_beg = reinterpret_cast<uptr>(allocated);
     uptr alloc_end = alloc_beg + needed_size;
     uptr user_beg = alloc_beg + rz_size;
@@ -609,6 +607,17 @@ struct Allocator {
 
     m->SetAllocContext(t ? t->tid() : kMainTid, StackDepotPut(*stack));
 
+    if (!from_primary || *(u8 *)MEM_TO_SHADOW((uptr)allocated) == 0) {
+      // The allocator provides an unpoisoned chunk. This is possible for the
+      // secondary allocator, or if CanPoisonMemory() was false for some time,
+      // for example, due to flags()->start_disabled. Anyway, poison left and
+      // right of the block before using it for anything else.
+      uptr tail_beg = RoundUpTo(user_end, ASAN_SHADOW_GRANULARITY);
+      uptr tail_end = alloc_beg + allocator.GetActuallyAllocatedSize(allocated);
+      PoisonShadow(alloc_beg, user_beg - alloc_beg, kAsanHeapLeftRedzoneMagic);
+      PoisonShadow(tail_beg, tail_end - tail_beg, kAsanHeapLeftRedzoneMagic);
+    }
+
     uptr size_rounded_down_to_granularity =
         RoundDownTo(size, ASAN_SHADOW_GRANULARITY);
     // Unpoison the bulk of the memory region.

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/huge_malloc.c b/compiler-rt/test/sanitizer_common/TestCases/Posix/huge_malloc.c
index e22349f4d95b9..a9c04f353ab6a 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Posix/huge_malloc.c
+++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/huge_malloc.c
@@ -1,4 +1,4 @@
-// RUN: %clang -O0 %s -o %t && %env_tool_opts=allocator_may_return_null=1:hard_rss_limit_mb=50 %run %t
+// RUN: %clang -O0 %s -o %t && %env_tool_opts=allocator_may_return_null=1:hard_rss_limit_mb=50:quarantine_size_mb=1 %run %t
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -10,7 +10,7 @@
 // UNSUPPORTED: android-26
 
 // FIXME: Make it work. Don't xfail to avoid excessive memory usage.
-// UNSUPPORTED: asan, msan, hwasan
+// UNSUPPORTED: msan, hwasan
 
 void *p;