[ASan] Do not misrepresent high value address dereferences as null dereferences

[yln]

Summary

Dereferences with addresses above the 48-bit hardware addressable range
produce "invalid instruction" (instead of "invalid access") hardware
exceptions (there is no hardware address decoding logic for those bits),
and the address provided by this exception is the address of the
instruction (not the faulting address). The kernel maps the "invalid
instruction" to SEGV, but fails to provide the real fault address.

Because of this ASan lies and says that those cases are null
dereferences. This downgrades the severity of a found bug in terms of
security. In the ASan signal handler, we can not provide the real
faulting address, but at least we can try not to lie.

rdar://50366151

So far, I tested this on macOS and putting it up for feedback.
Before landing this, I will move the test to asan/Posix and run ninja check-asan on Linux.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_posix.cpp

302 ↗

(On Diff #223966)

can you move it into darwin
on linux si_code is SI_KERNEL in similar cases so we will need different implementations

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp

194–201

can you change it to

if (trueaddress)
  Print("Full error with address")
else
  Print("Full error without address")

I guess more readable and less problems when multiple processes write into same terminal

[yln]

Address Vitaly's comments.

[yln]

Move test to 'asan/Posix' and provide Linux implementation for 'SignalContext::IsTrueFaultingAddress'.

[thakis]

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

949

This breaks a bunch of bots with /home/buildbots/ppc64be-clang-lnt-test/clang-ppc64be-lnt/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp:949: Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2]

[russell.gallop]

I've reverted this to get the bots working again.

[yln]

Apologies for the failing bots.

compiler-rt/lib/asan/asan_errors.h

struct ErrorDeadlySignal : ErrorBase {
   ErrorDeadlySignal(u32 tid, const SignalContext &sig)
       : ErrorBase(tid),
         signal(sig) {
     scariness.Clear();
     if (signal.IsStackOverflow()) {
       scariness.Scare(10, "stack-overflow");
     } else if (!signal.is_memory_access) {
       scariness.Scare(10, "signal");
-    } else if (signal.addr < GetPageSizeCached()) {
+    } else if (signal.is_true_faulting_addr &&
+               signal.addr < GetPageSizeCached()) {
       scariness.Scare(10, "null-deref");
     } else if (signal.addr == signal.pc) {
       scariness.Scare(60, "wild-jump");
     } else if (signal.write_flag == SignalContext::WRITE) {
       scariness.Scare(30, "wild-addr-write");
     } else if (signal.write_flag == SignalContext::READ) {
       scariness.Scare(20, "wild-addr-read");
     } else {

compiler-rt/lib/sanitizer_common/sanitizer_common.h

struct SignalContext {
   void *context;
   uptr addr;
   uptr pc;
   uptr sp;
   uptr bp;
   bool is_memory_access;
   enum WriteFlag { UNKNOWN, READ, WRITE } write_flag;
 
+  // In some cases the kernel cannot provide the true faulting address; `addr`
+  // will be zero then.  This field allows to distinguish between these cases
+  // and "null dereferences".
+  bool is_true_faulting_addr;
+
   // VS2013 doesn't implement unrestricted unions, so we need a trivial default
   // constructor
   SignalContext() = default;
 
   // Creates signal context in a platform-specific manner.
   // SignalContext is going to keep pointers to siginfo and context without
   // owning them.
   SignalContext(void *siginfo, void *context)
       : siginfo(siginfo),
         context(context),
         addr(GetAddress()),
         is_memory_access(IsMemoryAccess()),
-        write_flag(GetWriteFlag()) {
+        write_flag(GetWriteFlag()),
+        is_true_faulting_addr(IsTrueFaultingAddress()) {
     InitPcSpBp();
   }
 
   static void DumpAllRegisters(void *context);
 
   // Type of signal e.g. SIGSEGV or EXCEPTION_ACCESS_VIOLATION.
   int GetType() const;
 
   // String description of the signal.
   const char *Describe() const;
 
   // Returns true if signal is stack overflow.
   bool IsStackOverflow() const;
 
  private:
   // Platform specific initialization.
   void InitPcSpBp();
   uptr GetAddress() const;
   WriteFlag GetWriteFlag() const;
   bool IsMemoryAccess() const;
+  bool IsTrueFaultingAddress() const;
 };
 
 void InitializePlatformEarly();
 void MaybeReexec();
 
 template <typename Fn>
 class RunOnDestruction {
  public:

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

#endif
   u32 instr = *(u32 *)pc;
   return (instr >> 21) & 1 ? WRITE: READ;
 #else
   (void)ucontext;
   return UNKNOWN;  // FIXME: Implement.
 #endif
 }
 
+bool SignalContext::IsTrueFaultingAddress() const {
+  // TODO: Implement this properly.
+  return IsMemoryAccess();
+}
+
 void SignalContext::DumpAllRegisters(void *context) {
   // FIXME: Implement this.
 }
 
 static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
 #if SANITIZER_NETBSD
   // This covers all NetBSD architectures
   ucontext_t *ucontext = (ucontext_t *)context;

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

 #if defined(__x86_64__) || defined(__i386__)
   ucontext_t *ucontext = static_cast<ucontext_t*>(context);
   return ucontext->uc_mcontext->__es.__err & 2 /*T_PF_WRITE*/ ? WRITE : READ;
 #else
   return UNKNOWN;
 #endif
 }
 
+bool SignalContext::IsTrueFaultingAddress() const {
+  auto si = static_cast<const siginfo_t *>(siginfo);
+  // "Real" SIGSEGV codes (e.g., SEGV_MAPERR, SEGV_MAPERR) are non-zero
+  return si->si_signo == SIGSEGV && si->si_code != 0;
+}
+
 static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
   ucontext_t *ucontext = (ucontext_t*)context;
 # if defined(__aarch64__)
   *pc = ucontext->uc_mcontext->__ss.__pc;
 #   if defined(__IPHONE_8_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_8_0
   *bp = ucontext->uc_mcontext->__ss.__fp;
 #   else
   *bp = ucontext->uc_mcontext->__ss.__lr;

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp

 }
 
 static void ReportDeadlySignalImpl(const SignalContext &sig, u32 tid,
                                    UnwindSignalStackCallbackType unwind,
                                    const void *unwind_context) {
   SanitizerCommonDecorator d;
   Printf("%s", d.Warning());
   const char *description = sig.Describe();
+  if (sig.is_memory_access && !sig.is_true_faulting_addr)
+    Report("ERROR: %s: %s on unknown address (pc %p bp %p sp %p T%d)\n",
+           SanitizerToolName, description, (void *)sig.pc, (void *)sig.bp,
+           (void *)sig.sp, tid);
+  else
     Report("ERROR: %s: %s on unknown address %p (pc %p bp %p sp %p T%d)\n",
            SanitizerToolName, description, (void *)sig.addr, (void *)sig.pc,
            (void *)sig.bp, (void *)sig.sp, tid);
   Printf("%s", d.Default());
   if (sig.pc < GetPageSizeCached())
     Report("Hint: pc points to the zero page.\n");
   if (sig.is_memory_access) {
     const char *access_type =
         sig.write_flag == SignalContext::WRITE
             ? "WRITE"
             : (sig.write_flag == SignalContext::READ ? "READ" : "UNKNOWN");
     Report("The signal is caused by a %s memory access.\n", access_type);
-    if (sig.addr < GetPageSizeCached())
+    if (!sig.is_true_faulting_addr)
+      Report("Hint: this fault was caused by a dereference of a high value "
+             "address (see registers below).  Dissassemble the provided pc "
+             "to learn which register value was used.\n");
+    else if (sig.addr < GetPageSizeCached())
       Report("Hint: address points to the zero page.\n");
   }
   MaybeReportNonExecRegion(sig.pc);
   InternalMmapVector<BufferedStackTrace> stack_buffer(1);
   BufferedStackTrace *stack = stack_buffer.data();
   stack->Reset();
   unwind(sig, unwind_context, stack);
   stack->Print();

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

uptr SignalContext::GetAddress() const {
   EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
   return exception_record->ExceptionInformation[1];
 }
 
 bool SignalContext::IsMemoryAccess() const {
   return GetWriteFlag() != SignalContext::UNKNOWN;
 }
 
+bool SignalContext::IsTrueFaultingAddress() const {
+  return IsMemoryAccess();
+}
+
 SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
   EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
   // The contents of this array are documented at
   // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363082(v=vs.85).aspx
   // The first element indicates read as 0, write as 1, or execute as 8.  The
   // second element is the faulting address.
   switch (exception_record->ExceptionInformation[0]) {
     case 0:

compiler-rt/test/asan/TestCases/Darwin/high-address-dereference.c

+// On x86_64, XNU does not provide the faulting address for dereferences of
+// addresses greater than the 48-bit hardware addressable range, i.e.,
+// `siginfo.si_addr` is zero in ASan's SEGV signal handler. This test checks
+// that ASan does not misrepresent such cases as "NULL dereferences".
+
+// REQUIRES: x86_64-target-arch
+// RUN: %clang_asan %s -o %t
+// RUN: export %env_asan_opts=print_scariness=1
+// RUN: not %run %t 0x0000000000000000 2>&1 | FileCheck %s --check-prefixes=ZERO,HINT-PAGE0
+// RUN: not %run %t 0x0000000000000FFF 2>&1 | FileCheck %s --check-prefixes=LOW1,HINT-PAGE0
+// RUN: not %run %t 0x0000000000001000 2>&1 | FileCheck %s --check-prefixes=LOW2,HINT-NONE
+// RUN: not %run %t 0x4141414141414141 2>&1 | FileCheck %s --check-prefixes=HIGH,HINT-HIGHADDR
+// RUN: not %run %t 0xFFFFFFFFFFFFFFFF 2>&1 | FileCheck %s --check-prefixes=MAX,HINT-HIGHADDR
+
+#include <stdlib.h>
+#include <stdio.h>
+
+int main(int argc, const char *argv[]) {
+  const char *hex = argv[1];
+  uint64_t *addr = (uint64_t *)strtoull(hex, NULL, 16);
+  uint64_t x = *addr;
+  printf("%llu\n", x);
+  return 0;
+}
+
+// ZERO:  SEGV on unknown address 0x000000000000 (pc
+// LOW1:  SEGV on unknown address 0x000000000fff (pc
+// LOW2:  SEGV on unknown address 0x000000001000 (pc
+// HIGH:  SEGV on unknown address (pc
+// MAX:   SEGV on unknown address (pc
+
+// HINT-PAGE0-NOT: Hint: this fault was caused by a dereference of a high value address
+// HINT-PAGE0:     Hint: address points to the zero page.
+
+// HINT-NONE-NOT:  Hint: this fault was caused by a dereference of a high value address
+// HINT-NONE-NOT:  Hint: address points to the zero page.
+
+// HINT-HIGHADDR:     Hint: this fault was caused by a dereference of a high value address
+// HINT-HIGHADDR-NOT: Hint: address points to the zero page.
+
+// ZERO:  SCARINESS: 10 (null-deref)
+// LOW1:  SCARINESS: 10 (null-deref)
+// LOW2:  SCARINESS: 20 (wild-addr-read)
+// HIGH:  SCARINESS: 20 (wild-addr-read)
+// MAX:   SCARINESS: 20 (wild-addr-read)
+
+// HIGH,MAX: Register values:
+// HIGH: = 0x4141414141414141
+// MAX:  = 0xffffffffffffffff