Mmap interceptor providing mprotect support

[devnexen]

Summary

• Intercepting mprotect calls.
• Fixing forgotten flag check.

Patch by David CARLIER

[devnexen]

After that, next step would be trying to fix Android support. sorry for all the work I give you @vitalybuka

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

280 ↗

(On Diff #139432)

Just put this into INTERCEPTOR
No need to have COMMON_INTERCEPTOR_MPROTECT_IMPL before we have overrides

6925 ↗

(On Diff #139432)

Oh, it didn't actually worked.

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

15 ↗

(On Diff #139432)

Please extend tests with:

1. env_tool_opts=detect_write_exec=0 and // CHECK-DISABLED-NOT: writable-executable
2. correct call to mprotect without warning.
3. add //CHECK for [[@LINE
4. Move all correct mprotec and mpap to the end and CHECK-NOT: writable-executable to make sure there is no 3rd report

[devnexen]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

15 ↗

(On Diff #139432)

When I set to respectively LINE-3, LINE-2 ... does not work ... any clue eventually ?

[vitalybuka]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

21 ↗

(On Diff #139539)

CHECK-DISABLED-NOT: #0 is unnecacary
you still need to keep // CHECK: for reports

15 ↗

(On Diff #139432)

you are matching sanitizer_common_interceptors.in file.
you should match mmap_write_exec.cpp:[[@LINE....

[devnexen]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

15 ↗

(On Diff #139432)

Oh right I missed this ...

[vitalybuka]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

15 ↗

(On Diff #139546)

For check-not I'd use simple rule, e.g. just "-NOT: Sanitizer"
With complex rule there is a check that it can get always true if we change format.

20 ↗

(On Diff #139546)

there is no point in checking "-NOT:" twice here
one above covers everything

[devnexen]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

20 ↗

(On Diff #139546)

Oh ok good to know.

[vitalybuka]

test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

2 ↗

(On Diff #139561)

"FileCheck %s" was lost

[kubamracek]

Hi, this probably broke on Darwin, https://smooshbase.apple.com/ci/view/BuildCzar/job/apple-clang-master-RA-stage1-cmake-incremental/44333/console:

duplicate symbol _wrap_mprotect in:
    projects/compiler-rt/lib/asan/CMakeFiles/RTAsan_dynamic.watchossim.dir/asan_interceptors.cc.o
    projects/compiler-rt/lib/asan/CMakeFiles/RTAsan_dynamic.watchossim.dir/asan_malloc_mac.cc.o
ld: 1 duplicate symbol for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Can you take a look?

[devnexen]

Seems sanitizer_malloc_mac.inc already intercepts mprotect.

[devnexen]

Would need to protect the common interception and still catching the flags in the mac part maybe.

[vitalybuka]

reverted r328375

[kubamracek]

Thanks. If you don't have a macOS machine, you can email me a patch and I can test it out for you.

[devnexen]

Fix Apple build, as mprotect is already intercepted. We check the flags in apple case too.

[devnexen]

In D44777#1047175, @kubamracek wrote:

Thanks. If you don't have a macOS machine, you can email me a patch and I can test it out for you.

I tested with an weak apple machine I reproduced your linkage issues and do not get them after the changes.

[vitalybuka]

lib/sanitizer_common/sanitizer_malloc_mac.inc

76 ↗

(On Diff #139659)

It does not look right.
We have now two conditional interceptors.

I think better approach to instert some mmaloc hook into the common interceptor
and make it empty on all platforms and put "if (addr == malloc_zones && prot == PROT_READ) {" into the mac one.

[devnexen]

lib/sanitizer_common/sanitizer_malloc_mac.inc

76 ↗

(On Diff #139659)

Sure fair point I tried to avoid too much changes. Will update.

[vitalybuka]

lib/sanitizer_common/sanitizer_malloc_mac.inc

309 ↗

(On Diff #139678)

please move this into some cc file. e.g sanitizer_mac.cc

[devnexen]

Refactoring MmapMallocZones

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 // Dies on all but out of memory errors, in the latter case returns nullptr.
 void *MmapAlignedOrDieOnFatalError(uptr size, uptr alignment,
                                    const char *mem_type);
 // Disallow access to a memory range.  Use MmapFixedNoAccess to allocate an
 // unaccessible memory.
 bool MprotectNoAccess(uptr addr, uptr size);
 bool MprotectReadOnly(uptr addr, uptr size);
 
+void MprotectMallocZones(void *addr, int prot);
+
 // Find an available address space.
 uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding,
                               uptr *largest_gap_found, uptr *max_occupied_addr);
 
 // Used to check if we can map shadow memory to a fixed location.
 bool MemoryRangeIsAvailable(uptr range_start, uptr range_end);
 // Releases memory pages entirely within the [beg, end] address range. Noop if
 // the provided range does not contain at least one entire page.

 // Same as above, but construct error_message as:
 //   error_type file:line[:column][ function]
 void ReportErrorSummary(const char *error_type, const AddressInfo &info,
                         const char *alt_tool_name = nullptr);
 // Same as above, but obtains AddressInfo by symbolizing top stack trace frame.
 void ReportErrorSummary(const char *error_type, const StackTrace *trace,
                         const char *alt_tool_name = nullptr);
 
-void ReportMmapWriteExec();
+void ReportMmapWriteExec(int prot);
 
 // Math
 #if SANITIZER_WINDOWS && !defined(__clang__) && !defined(__GNUC__)
 extern "C" {
 unsigned char _BitScanForward(unsigned long *index, unsigned long mask);  // NOLINT
 unsigned char _BitScanReverse(unsigned long *index, unsigned long mask);  // NOLINT
 #if defined(_WIN64)
 unsigned char _BitScanForward64(unsigned long *index, unsigned __int64 mask);  // NOLINT

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

 #define INIT_STRLCPY
 #endif
 
 #if SANITIZER_INTERCEPT_MMAP
 INTERCEPTOR(void *, mmap, void *addr, SIZE_T sz, int prot, int flags, int fd,
             OFF_T off) {
   void *ctx;
   if (common_flags()->detect_write_exec)
-    ReportMmapWriteExec();
+    ReportMmapWriteExec(prot);
   if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
     return (void *)internal_mmap(addr, sz, prot, flags, fd, off);
   COMMON_INTERCEPTOR_ENTER(ctx, mmap, addr, sz, prot, flags, fd, off);
   COMMON_INTERCEPTOR_MMAP_IMPL(ctx, mmap, addr, sz, prot, flags, fd, off);
 }
-#define INIT_MMAP COMMON_INTERCEPT_FUNCTION(mmap);
+
+INTERCEPTOR(int, mprotect, void *addr, SIZE_T sz, int prot) {
+  void *ctx;
+  if (common_flags()->detect_write_exec)
+    ReportMmapWriteExec(prot);
+  if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
+    return (int)internal_mprotect(addr, sz, prot);
+  COMMON_INTERCEPTOR_ENTER(ctx, mprotect, addr, sz, prot);
+  MprotectMallocZones(addr, prot);
+  return REAL(mprotect)(addr, sz, prot);
+}
+#define INIT_MMAP                                                              \
+  COMMON_INTERCEPT_FUNCTION(mmap);                                             \
+  COMMON_INTERCEPT_FUNCTION(mprotect);
 #else
 #define INIT_MMAP
 #endif
 
 #if SANITIZER_INTERCEPT_MMAP64
 INTERCEPTOR(void *, mmap64, void *addr, SIZE_T sz, int prot, int flags, int fd,
             OFF64_T off) {
   void *ctx;
   if (common_flags()->detect_write_exec)
-    ReportMmapWriteExec();
+    ReportMmapWriteExec(prot);
   if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
     return (void *)internal_mmap(addr, sz, prot, flags, fd, off);
   COMMON_INTERCEPTOR_ENTER(ctx, mmap64, addr, sz, prot, flags, fd, off);
   COMMON_INTERCEPTOR_MMAP_IMPL(ctx, mmap64, addr, sz, prot, flags, fd, off);
 }
 #define INIT_MMAP64 COMMON_INTERCEPT_FUNCTION(mmap64);
 #else
 #define INIT_MMAP64

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc

 #include "sanitizer_procmaps.h"
 #include "sanitizer_report_decorator.h"
 #include "sanitizer_stackdepot.h"
 #include "sanitizer_stacktrace.h"
 #include "sanitizer_symbolizer.h"
 
 #if SANITIZER_POSIX
 #include "sanitizer_posix.h"
+#include <sys/mman.h>
 #endif
 
 namespace __sanitizer {
 
 #if !SANITIZER_FUCHSIA
 
 bool ReportFile::SupportsColors() {
   SpinMutexLock l(mu);
#if !SANITIZER_GO
   // Maybe sometimes we need to choose another frame (e.g. skip memcpy/etc).
   uptr pc = StackTrace::GetPreviousInstructionPc(stack->trace[0]);
   SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
   ReportErrorSummary(error_type, frame->info, alt_tool_name);
   frame->ClearAll();
 #endif
 }
 
-void ReportMmapWriteExec() {
-#if !SANITIZER_GO && !SANITIZER_ANDROID
+void ReportMmapWriteExec(int prot) {
+#if SANITIZER_POSIX && (!SANITIZER_GO && !SANITIZER_ANDROID)
+  if ((prot & (PROT_WRITE | PROT_EXEC)) != (PROT_WRITE | PROT_EXEC))
+    return;
+
   ScopedErrorReportLock l;
   SanitizerCommonDecorator d;
 
   InternalScopedBuffer<BufferedStackTrace> stack_buffer(1);
   BufferedStackTrace *stack = stack_buffer.data();
   stack->Reset();
   uptr top = 0;
   uptr bottom = 0;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc

 #include <dlfcn.h>  // for dladdr()
 #include <errno.h>
 #include <fcntl.h>
 #include <libkern/OSAtomic.h>
 #include <mach-o/dyld.h>
 #include <mach/mach.h>
 #include <mach/mach_time.h>
 #include <mach/vm_statistics.h>
+#include <malloc/malloc.h>
 #include <pthread.h>
 #include <sched.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>

 void ReExec() {
   UNIMPLEMENTED();
 }
 
 uptr GetPageSize() {
   return sysconf(_SC_PAGESIZE);
 }
 
+extern unsigned malloc_num_zones;
+extern malloc_zone_t **malloc_zones;
+static malloc_zone_t sanitizer_zone;
+
+// We need to make sure that sanitizer_zone is registered as malloc_zones[0]. If
+// libmalloc tries to set up a different zone as malloc_zones[0], it will call
+// mprotect(malloc_zones, ..., PROT_READ).  This interceptor will catch that and
+// make sure we are still the first (default) zone.
+void MprotectMallocZones(void *addr, int prot) {
+  if (addr == malloc_zones && prot == PROT_READ) {
+    if (malloc_num_zones > 1 && malloc_zones[0] != &sanitizer_zone) {
+      for (unsigned i = 1; i < malloc_num_zones; i++) {
+        if (malloc_zones[i] == &sanitizer_zone) {
+          // Swap malloc_zones[0] and malloc_zones[i].
+          malloc_zones[i] = malloc_zones[0];
+          malloc_zones[0] = &sanitizer_zone;
+          break;
+        }
+      }
+    }
+  }
+}
+
 BlockingMutex::BlockingMutex() {
   internal_memset(this, 0, sizeof(*this));
 }
 
 void BlockingMutex::Lock() {
   CHECK(sizeof(OSSpinLock) <= sizeof(opaque_storage_));
   CHECK_EQ(OS_SPINLOCK_INIT, 0);
   CHECK_EQ(owner_, 0);

compiler-rt/trunk/lib/sanitizer_common/sanitizer_malloc_mac.inc

 #include <sys/mman.h>
 
 #include "interception/interception.h"
 #include "sanitizer_common/sanitizer_mac.h"
 
 // Similar code is used in Google Perftools,
 // https://github.com/gperftools/gperftools.
 
-static malloc_zone_t sanitizer_zone;
-
 INTERCEPTOR(malloc_zone_t *, malloc_create_zone,
                              vm_size_t start_size, unsigned zone_flags) {
   COMMON_MALLOC_ENTER();
   uptr page_size = GetPageSizeCached();
   uptr allocated_size = RoundUpTo(sizeof(sanitizer_zone), page_size);
   COMMON_MALLOC_MEMALIGN(page_size, allocated_size);
   malloc_zone_t *new_zone = (malloc_zone_t *)p;
   internal_memcpy(new_zone, &sanitizer_zone, sizeof(sanitizer_zone));
if (GetMacosVersion() >= MACOS_VERSION_LION) {
     mprotect(zone, allocated_size, PROT_READ | PROT_WRITE);
   }
   if (zone->zone_name) {
     COMMON_MALLOC_FREE((void *)zone->zone_name);
   }
   COMMON_MALLOC_FREE(zone);
 }
 
-extern unsigned malloc_num_zones;
-extern malloc_zone_t **malloc_zones;
-
-// We need to make sure that sanitizer_zone is registered as malloc_zones[0]. If
-// libmalloc tries to set up a different zone as malloc_zones[0], it will call
-// mprotect(malloc_zones, ..., PROT_READ).  This interceptor will catch that and
-// make sure we are still the first (default) zone.
-INTERCEPTOR(int, mprotect, void *addr, size_t len, int prot) {
-  if (addr == malloc_zones && prot == PROT_READ) {
-    if (malloc_num_zones > 1 && malloc_zones[0] != &sanitizer_zone) {
-      for (unsigned i = 1; i < malloc_num_zones; i++) {
-        if (malloc_zones[i] == &sanitizer_zone) {
-          // Swap malloc_zones[0] and malloc_zones[i].
-          malloc_zones[i] = malloc_zones[0];
-          malloc_zones[0] = &sanitizer_zone;
-          break;
-        }
-      }
-    }
-  }
-  return REAL(mprotect)(addr, len, prot);
-}
-
 INTERCEPTOR(malloc_zone_t *, malloc_default_zone, void) {
   COMMON_MALLOC_ENTER();
   return &sanitizer_zone;
 }
 
 INTERCEPTOR(malloc_zone_t *, malloc_default_purgeable_zone, void) {
   // FIXME: ASan should support purgeable allocations.
   // https://github.com/google/sanitizers/issues/139

compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc

 bool MprotectNoAccess(uptr addr, uptr size) {
   return 0 == internal_mprotect((void*)addr, size, PROT_NONE);
 }
 
 bool MprotectReadOnly(uptr addr, uptr size) {
   return 0 == internal_mprotect((void *)addr, size, PROT_READ);
 }
 
+#if !SANITIZER_MAC
+void MprotectMallocZones(void *addr, int prot) {}
+#endif
+
 fd_t OpenFile(const char *filename, FileAccessMode mode, error_t *errno_p) {
   int flags;
   switch (mode) {
     case RdOnly: flags = O_RDONLY; break;
     case WrOnly: flags = O_WRONLY | O_CREAT; break;
     case RdWr: flags = O_RDWR | O_CREAT; break;
   }
   fd_t res = internal_open(filename, flags, 0660);

compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/mmap_write_exec.cpp

 // RUN: %clangxx %s -o %t
 // RUN: %env_tool_opts=detect_write_exec=1 %run %t 2>&1 | FileCheck %s
-// ubsan and lsan do not install mmap interceptors
-// UNSUPPORTED: ubsan, lsan
+// RUN: %env_tool_opts=detect_write_exec=0 %run %t 2>&1 | FileCheck %s \
+// RUN:   --check-prefix=CHECK-DISABLED
+// ubsan and lsan do not install mmap interceptors UNSUPPORTED: ubsan, lsan
 
 // TODO: Fix option on Android, it hangs there for unknown reasons.
 // XFAIL: android
 
+#include <stdio.h>
 #include <sys/mman.h>
 
 int main(int argc, char **argv) {
   char *p = (char *)mmap(0, 1024, PROT_READ | PROT_WRITE | PROT_EXEC,
                          MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   // CHECK: WARNING: {{.*}}Sanitizer: writable-executable page usage
+  // CHECK: #{{[0-9]+.*}}main{{.*}}mmap_write_exec.cpp:[[@LINE-3]]
+  // CHECK: SUMMARY: {{.*}}Sanitizer: w-and-x-usage
+
+  char *q = (char *)mmap(p, 64, PROT_READ | PROT_WRITE,
+                         MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+  (void)mprotect(q, 64, PROT_WRITE | PROT_EXEC);
+  // CHECK: WARNING: {{.*}}Sanitizer: writable-executable page usage
+  // CHECK: #{{[0-9]+.*}}main{{.*}}mmap_write_exec.cpp:[[@LINE-2]]
+  // CHECK: SUMMARY: {{.*}}Sanitizer: w-and-x-usage
+
+  char *a = (char *)mmap(0, 1024, PROT_READ | PROT_WRITE,
+                         MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  char *b = (char *)mmap(a, 64, PROT_READ | PROT_WRITE,
+                         MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+  (void)mprotect(q, 64, PROT_READ | PROT_EXEC);
+  // CHECK-NOT: Sanitizer
+
+  printf("done\n");
+  // CHECK-DISABLED-NOT: Sanitizer
+  // CHECK-DISABLED: done
 }