[compiler-rt] r374384 - Reland "[ASan] Do not misrepresent high value address dereferences as null dereferences"
Julian Lettner via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 10:19:58 PDT 2019
Author: yln
Date: Thu Oct 10 10:19:58 2019
New Revision: 374384
URL: http://llvm.org/viewvc/llvm-project?rev=374384&view=rev
Log:
Reland "[ASan] Do not misrepresent high value address dereferences as null dereferences"
Updated: Removed offending TODO comment.
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
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D68676
llvm-svn: 374265
Added:
compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.c
Modified:
compiler-rt/trunk/lib/asan/asan_errors.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cpp
compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cpp
compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cpp
Modified: compiler-rt/trunk/lib/asan/asan_errors.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_errors.h?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_errors.h (original)
+++ compiler-rt/trunk/lib/asan/asan_errors.h Thu Oct 10 10:19:58 2019
@@ -48,7 +48,8 @@ struct ErrorDeadlySignal : ErrorBase {
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");
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Thu Oct 10 10:19:58 2019
@@ -881,6 +881,11 @@ struct SignalContext {
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 dereferences of null.
+ bool is_true_faulting_addr;
+
// VS2013 doesn't implement unrestricted unions, so we need a trivial default
// constructor
SignalContext() = default;
@@ -893,7 +898,8 @@ struct SignalContext {
context(context),
addr(GetAddress()),
is_memory_access(IsMemoryAccess()),
- write_flag(GetWriteFlag()) {
+ write_flag(GetWriteFlag()),
+ is_true_faulting_addr(IsTrueFaultingAddress()) {
InitPcSpBp();
}
@@ -914,6 +920,7 @@ struct SignalContext {
uptr GetAddress() const;
WriteFlag GetWriteFlag() const;
bool IsMemoryAccess() const;
+ bool IsTrueFaultingAddress() const;
};
void InitializePlatformEarly();
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cpp?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cpp (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cpp Thu Oct 10 10:19:58 2019
@@ -1849,6 +1849,12 @@ SignalContext::WriteFlag SignalContext::
#endif
}
+bool SignalContext::IsTrueFaultingAddress() const {
+ auto si = static_cast<const siginfo_t *>(siginfo);
+ // SIGSEGV signals without a true fault address have si_code set to 128.
+ return si->si_signo == SIGSEGV && si->si_code != 128;
+}
+
void SignalContext::DumpAllRegisters(void *context) {
// FIXME: Implement this.
}
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cpp?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cpp (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cpp Thu Oct 10 10:19:58 2019
@@ -754,6 +754,12 @@ SignalContext::WriteFlag SignalContext::
#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__)
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cpp?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cpp (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cpp Thu Oct 10 10:19:58 2019
@@ -191,9 +191,14 @@ static void ReportDeadlySignalImpl(const
SanitizerCommonDecorator d;
Printf("%s", d.Warning());
const char *description = sig.Describe();
- 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);
+ 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");
@@ -203,7 +208,11 @@ static void ReportDeadlySignalImpl(const
? "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);
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cpp?rev=374384&r1=374383&r2=374384&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cpp (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cpp Thu Oct 10 10:19:58 2019
@@ -945,6 +945,11 @@ bool SignalContext::IsMemoryAccess() con
return GetWriteFlag() != SignalContext::UNKNOWN;
}
+bool SignalContext::IsTrueFaultingAddress() const {
+ // FIXME: Provide real implementation for this. See Linux and Mac variants.
+ return IsMemoryAccess();
+}
+
SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
// The contents of this array are documented at
Added: compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.c
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.c?rev=374384&view=auto
==============================================================================
--- compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.c (added)
+++ compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.c Thu Oct 10 10:19:58 2019
@@ -0,0 +1,50 @@
+// On x86_64, the kernel 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 <stdint.h>
+#include <stdlib.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; // segmentation fault
+ return x;
+}
+
+// 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)
+
+// TODO: Currently, register values are only printed on Mac. Once this changes,
+// remove the 'TODO_' prefix in the following lines.
+// TODO_HIGH,TODO_MAX: Register values:
+// TODO_HIGH: = 0x4141414141414141
+// TODO_MAX: = 0xffffffffffffffff
More information about the llvm-commits
mailing list