[asan] implement more strict checking for memset/etc parameters. Instead of checking the first and the last byte, we check the entire shadow region. This costs ~10 slowdown for the instrumented functions. Motivated by a nasty memset-buffer-overflow-by-140-bytes in chrome which was reported as a use-after-free or not at all

llvm-svn: 171198

compiler-rt/lib/asan/asan_interceptors.cc

@@ -27,38 +27,20 @@
 
 namespace __asan {
 
-// Instruments read/write access to a single byte in memory.
-// On error calls __asan_report_error, which aborts the program.
-#define ACCESS_ADDRESS(address, isWrite)   do {         \
-  if (!AddrIsInMem(address) || AddressIsPoisoned(address)) {                \
-    GET_CURRENT_PC_BP_SP;                               \
-    __asan_report_error(pc, bp, sp, address, isWrite, /* access_size */ 1); \
-  } \
-} while (0)
-
 // We implement ACCESS_MEMORY_RANGE, ASAN_READ_RANGE,
 // and ASAN_WRITE_RANGE as macro instead of function so
 // that no extra frames are created, and stack trace contains
 // relevant information only.
-
-// Instruments read/write access to a memory range.
-// More complex implementation is possible, for now just
-// checking the first and the last byte of a range.
+// We check all shadow bytes.
 #define ACCESS_MEMORY_RANGE(offset, size, isWrite) do {                  \
-  if (size > 0) { \
-    uptr _ptr = (uptr)(offset); \
-    ACCESS_ADDRESS(_ptr, isWrite); \
-    ACCESS_ADDRESS(_ptr + (size) - 1, isWrite); \
+  if (uptr __ptr = __asan_region_is_poisoned((uptr)(offset), size)) {    \
+    GET_CURRENT_PC_BP_SP;                                                \
+    __asan_report_error(pc, bp, sp, __ptr, isWrite, /* access_size */1); \
   }                                                                      \
 } while (0)
 
-#define ASAN_READ_RANGE(offset, size) do { \
-  ACCESS_MEMORY_RANGE(offset, size, false); \
-} while (0)
-
-#define ASAN_WRITE_RANGE(offset, size) do { \
-  ACCESS_MEMORY_RANGE(offset, size, true); \
-} while (0)
+#define ASAN_READ_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size, false)
+#define ASAN_WRITE_RANGE(offset, size) ACCESS_MEMORY_RANGE(offset, size, true);
 
 // Behavior of functions like "memcpy" or "strcpy" is undefined
 // if memory intervals overlap. We report error in this case.

compiler-rt/lib/asan/asan_poisoning.cc

@@ -16,6 +16,7 @@
 #include "asan_internal.h"
 #include "asan_mapping.h"
 #include "sanitizer/asan_interface.h"
+#include "sanitizer_common/sanitizer_libc.h"
 
 namespace __asan {
 
@@ -154,6 +155,33 @@ bool __asan_address_is_poisoned(void const volatile *addr) {
   return __asan::AddressIsPoisoned((uptr)addr);
 }
 
+uptr __asan_region_is_poisoned(uptr beg, uptr size) {
+  if (!size) return 0;
+  uptr end = beg + size;
+  if (!AddrIsInMem(beg)) return beg;
+  if (!AddrIsInMem(end)) return end;
+  uptr aligned_b = RoundUpTo(beg, SHADOW_GRANULARITY);
+  uptr aligned_e = RoundDownTo(end, SHADOW_GRANULARITY);
+  uptr shadow_beg = MemToShadow(aligned_b);
+  uptr shadow_end = MemToShadow(aligned_e);
+  // First check the first and the last application bytes,
+  // then check the SHADOW_GRANULARITY-aligned region by calling
+  // mem_is_zero on the corresponding shadow.
+  if (!__asan::AddressIsPoisoned(beg) &&
+      !__asan::AddressIsPoisoned(end - 1) &&
+      (shadow_end <= shadow_beg ||
+       __sanitizer::mem_is_zero((const char *)shadow_beg,
+                                shadow_end - shadow_beg)))
+    return 0;
+  // The fast check failed, so we have a poisoned byte somewhere.
+  // Find it slowly.
+  for (; beg < end; beg++)
+    if (__asan::AddressIsPoisoned(beg))
+      return beg;
+  UNREACHABLE("mem_is_zero returned false, but poisoned byte was not found");
+  return 0;
+}
+
 // This is a simplified version of __asan_(un)poison_memory_region, which
 // assumes that left border of region to be poisoned is properly aligned.
 static void PoisonAlignedStackMemory(uptr addr, uptr size, bool do_poison) {

compiler-rt/lib/asan/asan_rtl.cc

@@ -253,6 +253,7 @@ static NOINLINE void force_interface_symbols() {
     case 31: __asan_after_dynamic_init(); break;
     case 32: __asan_poison_stack_memory(0, 0); break;
     case 33: __asan_unpoison_stack_memory(0, 0); break;
+    case 34: __asan_region_is_poisoned(0, 0); break;
   }
 }
 

compiler-rt/lib/asan/tests/asan_noinst_test.cc

@@ -683,6 +683,45 @@ TEST(AddressSanitizerInterface, PoisoningStressTest) {
   }
 }
 
+TEST(AddressSanitizerInterface, PoisonedRegion) {
+  size_t rz = 16;
+  for (size_t size = 1; size <= 64; size++) {
+    char *p = new char[size];
+    uptr x = reinterpret_cast<uptr>(p);
+    for (size_t beg = 0; beg < size + rz; beg++) {
+      for (size_t end = beg; end < size + rz; end++) {
+        uptr first_poisoned = __asan_region_is_poisoned(x + beg, end - beg);
+        if (beg == end) {
+          EXPECT_FALSE(first_poisoned);
+        } else if (beg < size && end <= size) {
+          EXPECT_FALSE(first_poisoned);
+        } else if (beg >= size) {
+          EXPECT_EQ(x + beg, first_poisoned);
+        } else {
+          EXPECT_GT(end, size);
+          EXPECT_EQ(x + size, first_poisoned);
+        }
+      }
+    }
+    delete [] p;
+  }
+}
+
+// This is a performance benchmark for manual runs.
+// asan's memset interceptor calls mem_is_zero for the entire shadow region.
+// the profile should look like this:
+//     89.10%   [.] __memset_sse2
+//     10.50%   [.] __sanitizer::mem_is_zero
+// I.e. mem_is_zero should consume ~ SHADOW_GRANULARITY less CPU cycles
+// than memset itself.
+TEST(AddressSanitizerInterface, DISABLED_Stress_memset) {
+  size_t size = 1 << 20;
+  char *x = new char[size];
+  for (int i = 0; i < 100000; i++)
+    Ident(memset)(x, 0, size);
+  delete [] x;
+}
+
 static const char *kInvalidPoisonMessage = "invalid-poison-memory-range";
 static const char *kInvalidUnpoisonMessage = "invalid-unpoison-memory-range";
 

compiler-rt/lib/asan/tests/asan_test.cc

@@ -19,6 +19,7 @@
 #include <stdint.h>
 #include <setjmp.h>
 #include <assert.h>
+#include <algorithm>
 
 #ifdef __linux__
 # include <sys/prctl.h>
@@ -921,6 +922,50 @@ TEST(AddressSanitizer, MemSetOOBTest) {
   // We can test arrays of structres/classes here, but what for?
 }
 
+// Try to allocate two arrays of 'size' bytes that are near each other.
+// Strictly speaking we are not guaranteed to find such two pointers,
+// but given the structure of asan's allocator we will.
+static bool AllocateTwoAjacentArrays(char **x1, char **x2, size_t size) {
+  vector<char *> v;
+  bool res = false;
+  for (size_t i = 0; i < 1000U && !res; i++) {
+    v.push_back(new char[size]);
+    if (i == 0) continue;
+    sort(v.begin(), v.end());
+    for (size_t j = 1; j < v.size(); j++) {
+      assert(v[j] > v[j-1]);
+      if (v[j] - v[j-1] < size * 2) {
+        *x2 = v[j];
+        *x1 = v[j-1];
+        res = true;
+        break;
+      }
+    }
+  }
+
+  for (size_t i = 0; i < v.size(); i++) {
+    if (res && v[i] == *x1) continue;
+    if (res && v[i] == *x2) continue;
+    delete [] v[i];
+  }
+  return res;
+}
+
+TEST(AddressSanitizer, LargeOOBInMemset) {
+  for (size_t size = 200; size < 100000; size += size / 2) {
+    char *x1, *x2;
+    if (!AllocateTwoAjacentArrays(&x1, &x2, size))
+      continue;
+    // fprintf(stderr, "  large oob memset: %p %p %zd\n", x1, x2, size);
+    // Do a memset on x1 with huge out-of-bound access that will end up in x2.
+    EXPECT_DEATH(memset(x1, 0, size * 2), "is located 0 bytes to the right");
+    delete [] x1;
+    delete [] x2;
+    return;
+  }
+  assert(0 && "Did not find two adjacent malloc-ed pointers");
+}
+
 // Same test for memcpy and memmove functions
 template <typename T, class M>
 void MemTransferOOBTestTemplate(size_t length) {
@@ -1634,7 +1679,7 @@ TEST(AddressSanitizer, pread) {
   EXPECT_DEATH(pread(fd, x, 15, 0),
                ASAN_PCRE_DOTALL
                "AddressSanitizer: heap-buffer-overflow"
-               ".* is located 4 bytes to the right of 10-byte region");
+               ".* is located 0 bytes to the right of 10-byte region");
   close(fd);
   delete [] x;
 }
@@ -1646,7 +1691,7 @@ TEST(AddressSanitizer, pread64) {
   EXPECT_DEATH(pread64(fd, x, 15, 0),
                ASAN_PCRE_DOTALL
                "AddressSanitizer: heap-buffer-overflow"
-               ".* is located 4 bytes to the right of 10-byte region");
+               ".* is located 0 bytes to the right of 10-byte region");
   close(fd);
   delete [] x;
 }
@@ -1658,7 +1703,7 @@ TEST(AddressSanitizer, read) {
   EXPECT_DEATH(read(fd, x, 15),
                ASAN_PCRE_DOTALL
                "AddressSanitizer: heap-buffer-overflow"
-               ".* is located 4 bytes to the right of 10-byte region");
+               ".* is located 0 bytes to the right of 10-byte region");
   close(fd);
   delete [] x;
 }

compiler-rt/lib/sanitizer_common/sanitizer_libc.cc

@@ -205,4 +205,23 @@ s64 internal_simple_strtoll(const char *nptr, char **endptr, int base) {
   }
 }
 
+bool mem_is_zero(const char *beg, uptr size) {
+  CHECK_LE(size, 1UL << FIRST_32_SECOND_64(30, 40));  // Sanity check.
+  const char *end = beg + size;
+  uptr *aligned_beg = (uptr *)RoundUpTo((uptr)beg, sizeof(uptr));
+  uptr *aligned_end = (uptr *)RoundDownTo((uptr)end, sizeof(uptr));
+  uptr all = 0;
+  // Prologue.
+  for (const char *mem = beg; mem < (char*)aligned_beg && mem < end; mem++)
+    all |= *mem;
+  // Aligned loop.
+  for (; aligned_beg < aligned_end; aligned_beg++)
+    all |= *aligned_beg;
+  // Epilogue.
+  if ((char*)aligned_end >= beg)
+    for (const char *mem = (char*)aligned_end; mem < end; mem++)
+      all |= *mem;
+  return all == 0;
+}
+
 }  // namespace __sanitizer

compiler-rt/lib/sanitizer_common/sanitizer_libc.h

@@ -47,6 +47,11 @@ char *internal_strstr(const char *haystack, const char *needle);
 // Works only for base=10 and doesn't set errno.
 s64 internal_simple_strtoll(const char *nptr, char **endptr, int base);
 
+// Return true if all bytes in [mem, mem+size) are zero.
+// Optimized for the case when the result is true.
+bool mem_is_zero(const char *mem, uptr size);
+
+
 // Memory
 void *internal_mmap(void *addr, uptr length, int prot, int flags,
                     int fd, u64 offset);

compiler-rt/lib/sanitizer_common/tests/sanitizer_libc_test.cc

@@ -20,3 +20,23 @@ TEST(SanitizerCommon, InternalMemmoveRegression) {
   EXPECT_EQ(dest[0], src[0]);
   EXPECT_EQ(dest[4], src[4]);
 }
+
+TEST(SanitizerCommon, mem_is_zero) {
+  size_t size = 128;
+  char *x = new char[size];
+  memset(x, 0, size);
+  for (size_t pos = 0; pos < size; pos++) {
+    x[pos] = 1;
+    for (size_t beg = 0; beg < size; beg++) {
+      for (size_t end = beg; end < size; end++) {
+        // fprintf(stderr, "pos %zd beg %zd end %zd \n", pos, beg, end);
+        if (beg <= pos && pos < end)
+          EXPECT_FALSE(__sanitizer::mem_is_zero(x + beg, end - beg));
+        else
+          EXPECT_TRUE(__sanitizer::mem_is_zero(x + beg, end - beg));
+      }
+    }
+    x[pos] = 0;
+  }
+  delete [] x;
+}