PR20721: Don't let UBSan print inaccessible memory

[samsonov]

Summary

UBSan needs to check if memory snippet it's going to print resides
in addressable memory. Similar check might be helpful in ASan with
dump_instruction_bytes option (see http://reviews.llvm.org/D5167).

Instead of scanning /proc/self/maps manually, delegate this check to
the OS kernel: try to write this memory in a syscall and assume that
memory is inaccessible if the syscall failed (e.g. with EFAULT).

Fixes PR20721.

[ygribov]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

177

Perhaps assert that write_errno == EFAULT in case of error?

[glider]

I've checked that IsAccessibleMemoryRange works on Linux and OSX on the following examples:

char *mem = (char*)mmap(0, 4096 * 2, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
mprotect(mem + 4096, 4096, PROT_NONE);
printf("mem: %p\n", mem);
IsAccessibleMemoryRange(mem, 4095);  // 1
IsAccessibleMemoryRange(mem, 4096);  // 1
IsAccessibleMemoryRange(mem, 4097);  // 0
IsAccessibleMemoryRange(mem + 4095, 1);  // 1
IsAccessibleMemoryRange(mem + 4095, 2);  // 0
IsAccessibleMemoryRange(0, 2);  // 0

Care to add a unittest?

lib/sanitizer_common/sanitizer_posix_libcdep.cc

169

Any limits on |size|?
I think it must be an sptr greater than -1 (you're going to compare bytes_written to it) and less than kPageSize (otherwise it'll take too much time to check chunks of too big size)

173

internal_write() returns sptr, not uptr (at least it must).
Also, when bytes_written == size (assuming size != -1), internal_iserror() is always false.

[eugenis]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

171

Pipes have capacity, and large writes can block.
Maybe write to /dev/null instead?

[glider]

A too restrictive seccomp policy may prohibit opening /dev/null

[samsonov]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

171

See http://stackoverflow.com/questions/4611776/isbadreadptr-analogue-on-unix#comment23029943_4611930 - writing to /dev/null may always succeed.

171

If handling EINTR is not enough, I'd rather assert that "size" argument is reasonable small.

173

I believe we have a custom calling convention for returning errno from syscalls on Linux...

[samsonov]

Address reviewers' comments. Add a unit test for IsAccessibleMemoryRange.

[glider]

LGTM

lib/sanitizer_common/sanitizer_win.cc

526

I suggest making this function always return true and adding a FIXME

[samsonov]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

169

I pass beg/end instead and check that beg <= end and end - beg < 10*pagesize

177

Done

lib/sanitizer_common/sanitizer_win.cc

526

Done.

lib/sanitizer_common/sanitizer_common.h

 uptr ReadFileToBuffer(const char *file_name, char **buff,
                       uptr *buff_size, uptr max_len);
 // Maps given file to virtual memory, and returns pointer to it
 // (or NULL if the mapping failes). Stores the size of mmaped region
 // in '*buff_size'.
 void *MapFileToMemory(const char *file_name, uptr *buff_size);
 void *MapWritableFileToMemory(void *addr, uptr size, uptr fd, uptr offset);
 
+bool IsAccessibleMemoryRange(uptr beg, uptr size);
+
 // Error report formatting.
 const char *StripPathPrefix(const char *filepath,
                             const char *strip_file_prefix);
 void PrintSourceLocation(InternalScopedString *buffer, const char *file,
                          int line, int column);
 void PrintModuleAndOffset(InternalScopedString *buffer,
                           const char *module, uptr offset);
 

lib/sanitizer_common/sanitizer_posix_libcdep.cc

void InstallDeadlySignalHandlers(SignalHandlerType handler) {
   // This will cause SetAlternateSignalStack to be called twice, but the stack
   // will be actually set only once.
   if (common_flags()->use_sigaltstack) SetAlternateSignalStack();
   MaybeInstallSigaction(SIGSEGV, handler);
   MaybeInstallSigaction(SIGBUS, handler);
 }
 #endif  // SANITIZER_GO
 
+bool IsAccessibleMemoryRange(uptr beg, uptr size) {
+  int sock_pair[2];
+  if (pipe(sock_pair))
+    return false;
+  uptr bytes_written =
+      internal_write(sock_pair[1], reinterpret_cast<void *>(beg), size);
+  int write_errno;
+  bool res =
+      !internal_iserror(bytes_written, &write_errno) && bytes_written == size;
+  internal_close(sock_pair[0]);
+  internal_close(sock_pair[1]);
+  return res;
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_POSIX

lib/sanitizer_common/sanitizer_win.cc

void InstallDeadlySignalHandlers(SignalHandlerType handler) {
   // FIXME: Decide what to do on Windows.
 }
 
 bool IsDeadlySignal(int signum) {
   // FIXME: Decide what to do on Windows.
   return false;
 }
 
+bool IsAccessibleMemoryRange(uptr beg, uptr size) {
+  UNIMPLEMENTED();
+}
+
 }  // namespace __sanitizer
 
 #endif  // _WIN32

lib/ubsan/ubsan_diag.cc

for (unsigned I = 0; I != NumRanges; ++I)
     if (Ranges[I].getEnd().getMemoryLocation() > Loc &&
         (!Best ||
          Best->getStart().getMemoryLocation() >
          Ranges[I].getStart().getMemoryLocation()))
       Best = &Ranges[I];
   return Best;
 }
 
+static inline MemoryLocation subtractNoOverflow(uptr LHS, uptr RHS) {
+  return (LHS < RHS) ? 0 : LHS - RHS;
+}
+
+static inline MemoryLocation addNoOverflow(uptr LHS, uptr RHS) {
+  const uptr Limit = (uptr)-1;
+  return (LHS > Limit - RHS) ? Limit : LHS + RHS;
+}
+
 /// Render a snippet of the address space near a location.
 static void renderMemorySnippet(const Decorator &Decor, MemoryLocation Loc,
                                 Range *Ranges, unsigned NumRanges,
                                 const Diag::Arg *Args) {
-  const unsigned BytesToShow = 32;
   const unsigned MinBytesNearLoc = 4;
 
   // Show at least the 8 bytes surrounding Loc.
-  MemoryLocation Min = Loc - MinBytesNearLoc, Max = Loc + MinBytesNearLoc;
+  MemoryLocation Min = subtractNoOverflow(Loc, MinBytesNearLoc);
+  MemoryLocation Max = addNoOverflow(Loc, MinBytesNearLoc);
+  MemoryLocation OrigMin = Min;
   for (unsigned I = 0; I < NumRanges; ++I) {
     Min = __sanitizer::Min(Ranges[I].getStart().getMemoryLocation(), Min);
     Max = __sanitizer::Max(Ranges[I].getEnd().getMemoryLocation(), Max);
   }
 
   // If we have too many interesting bytes, prefer to show bytes after Loc.
+  const unsigned BytesToShow = 32;
   if (Max - Min > BytesToShow)
-    Min = __sanitizer::Min(Max - BytesToShow, Loc - MinBytesNearLoc);
-  Max = Min + BytesToShow;
+    Min = __sanitizer::Min(Max - BytesToShow, OrigMin);
+  Max = addNoOverflow(Min, BytesToShow);
+
+  // Don't print the contents of inaccessible memory.
+  if (!IsAccessibleMemoryRange(Min, Max - Min)) {
+    Printf("<memory cannot be printed>\n");
+    return;
+  }
 
   // Emit data.
   for (uptr P = Min; P != Max; ++P) {
-    // FIXME: Check that the address is readable before printing it.
     unsigned char C = *reinterpret_cast<const unsigned char*>(P);
     Printf("%s%02x", (P % 8 == 0) ? "  " : " ", C);
   }
   Printf("\n");
 
   // Emit highlights.
   Printf(Decor.Highlight());
   Range *InRange = upperBound(Min, Ranges, NumRanges);

test/ubsan/TestCases/TypeCheck/misaligned.cpp

 // RUN: %clangxx -fsanitize=alignment -g %s -O3 -o %t
 // RUN: %run %t l0 && %run %t s0 && %run %t r0 && %run %t m0 && %run %t f0 && %run %t n0
 // RUN: %run %t l1 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD --strict-whitespace
 // RUN: %run %t s1 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
 // RUN: %run %t r1 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
 // RUN: %run %t m1 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
 // RUN: %run %t f1 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
 // RUN: %run %t n1 2>&1 | FileCheck %s --check-prefix=CHECK-NEW
 // RUN: UBSAN_OPTIONS=print_stacktrace=1 %run %t l1 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD --check-prefix=CHECK-%os-STACK-LOAD
 
+// RUN: %clangxx -fsanitize=alignment -fno-sanitize-recover %s -O3 -o %t
+// RUN: not %run %t w1 2>&1 | FileCheck %s --check-prefix=CHECK-WILD
+
 #include <new>
 
 struct S {
   S() {}
   int f() { return 0; }
   int k;
 };
 
 int main(int, char **argv) {
   char c[] __attribute__((aligned(8))) = { 0, 0, 0, 0, 1, 2, 3, 4, 5 };
 
   // Pointer value may be unspecified here, but behavior is not undefined.
   int *p = (int*)&c[4 + argv[1][1] - '0'];
   S *s = (S*)p;
 
+  void *wild = reinterpret_cast<void *>(0x123L);
+
   (void)*p; // ok!
 
   switch (argv[1][0]) {
   case 'l':
     // CHECK-LOAD: misaligned.cpp:[[@LINE+4]]:12: runtime error: load of misaligned address [[PTR:0x[0-9a-f]*]] for type 'int', which requires 4 byte alignment
     // CHECK-LOAD-NEXT: [[PTR]]: note: pointer points here
     // CHECK-LOAD-NEXT: {{^ 00 00 00 01 02 03 04  05}}
     // CHECK-LOAD-NEXT: {{^             \^}}
case 'f':
     return s->f() && 0;
 
   case 'n':
     // CHECK-NEW: misaligned.cpp:[[@LINE+4]]:5: runtime error: constructor call on misaligned address [[PTR:0x[0-9a-f]*]] for type 'S', which requires 4 byte alignment
     // CHECK-NEW-NEXT: [[PTR]]: note: pointer points here
     // CHECK-NEW-NEXT: {{^ 00 00 00 01 02 03 04  05}}
     // CHECK-NEW-NEXT: {{^             \^}}
     return (new (s) S)->k && 0;
+
+  case 'w':
+    // CHECK-WILD: misaligned.cpp:[[@LINE+3]]:35: runtime error: member access within misaligned address 0x000000000123 for type 'S', which requires 4 byte alignment
+    // CHECK-WILD-NEXT: 0x000000000123: note: pointer points here
+    // CHECK-WILD-NEXT: <memory cannot be printed>
+    return static_cast<S*>(wild)->k;
   }
 }