Skip to content

Commit b577efe

Browse files
committedOct 10, 2019
[ASan] Do not misrepresent high value address dereferences as null dereferences
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
1 parent d6e9e99 commit b577efe

File tree

7 files changed

+90
-6
lines changed

7 files changed

+90
-6
lines changed
 

‎compiler-rt/lib/asan/asan_errors.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ struct ErrorDeadlySignal : ErrorBase {
4848
scariness.Scare(10, "stack-overflow");
4949
} else if (!signal.is_memory_access) {
5050
scariness.Scare(10, "signal");
51-
} else if (signal.addr < GetPageSizeCached()) {
51+
} else if (signal.is_true_faulting_addr &&
52+
signal.addr < GetPageSizeCached()) {
5253
scariness.Scare(10, "null-deref");
5354
} else if (signal.addr == signal.pc) {
5455
scariness.Scare(60, "wild-jump");

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,11 @@ struct SignalContext {
881881
bool is_memory_access;
882882
enum WriteFlag { UNKNOWN, READ, WRITE } write_flag;
883883

884+
// In some cases the kernel cannot provide the true faulting address; `addr`
885+
// will be zero then. This field allows to distinguish between these cases
886+
// and dereferences of null.
887+
bool is_true_faulting_addr;
888+
884889
// VS2013 doesn't implement unrestricted unions, so we need a trivial default
885890
// constructor
886891
SignalContext() = default;
@@ -893,7 +898,8 @@ struct SignalContext {
893898
context(context),
894899
addr(GetAddress()),
895900
is_memory_access(IsMemoryAccess()),
896-
write_flag(GetWriteFlag()) {
901+
write_flag(GetWriteFlag()),
902+
is_true_faulting_addr(IsTrueFaultingAddress()) {
897903
InitPcSpBp();
898904
}
899905

@@ -914,6 +920,7 @@ struct SignalContext {
914920
uptr GetAddress() const;
915921
WriteFlag GetWriteFlag() const;
916922
bool IsMemoryAccess() const;
923+
bool IsTrueFaultingAddress() const;
917924
};
918925

919926
void InitializePlatformEarly();

‎compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,12 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
18491849
#endif
18501850
}
18511851

1852+
bool SignalContext::IsTrueFaultingAddress() const {
1853+
auto si = static_cast<const siginfo_t *>(siginfo);
1854+
// SIGSEGV signals without a true fault address have si_code set to 128.
1855+
return si->si_signo == SIGSEGV && si->si_code != 128;
1856+
}
1857+
18521858
void SignalContext::DumpAllRegisters(void *context) {
18531859
// FIXME: Implement this.
18541860
}

‎compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,12 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
754754
#endif
755755
}
756756

757+
bool SignalContext::IsTrueFaultingAddress() const {
758+
auto si = static_cast<const siginfo_t *>(siginfo);
759+
// "Real" SIGSEGV codes (e.g., SEGV_MAPERR, SEGV_MAPERR) are non-zero.
760+
return si->si_signo == SIGSEGV && si->si_code != 0;
761+
}
762+
757763
static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
758764
ucontext_t *ucontext = (ucontext_t*)context;
759765
# if defined(__aarch64__)

‎compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,14 @@ static void ReportDeadlySignalImpl(const SignalContext &sig, u32 tid,
191191
SanitizerCommonDecorator d;
192192
Printf("%s", d.Warning());
193193
const char *description = sig.Describe();
194-
Report("ERROR: %s: %s on unknown address %p (pc %p bp %p sp %p T%d)\n",
195-
SanitizerToolName, description, (void *)sig.addr, (void *)sig.pc,
196-
(void *)sig.bp, (void *)sig.sp, tid);
194+
if (sig.is_memory_access && !sig.is_true_faulting_addr)
195+
Report("ERROR: %s: %s on unknown address (pc %p bp %p sp %p T%d)\n",
196+
SanitizerToolName, description, (void *)sig.pc, (void *)sig.bp,
197+
(void *)sig.sp, tid);
198+
else
199+
Report("ERROR: %s: %s on unknown address %p (pc %p bp %p sp %p T%d)\n",
200+
SanitizerToolName, description, (void *)sig.addr, (void *)sig.pc,
201+
(void *)sig.bp, (void *)sig.sp, tid);
197202
Printf("%s", d.Default());
198203
if (sig.pc < GetPageSizeCached())
199204
Report("Hint: pc points to the zero page.\n");
@@ -203,7 +208,11 @@ static void ReportDeadlySignalImpl(const SignalContext &sig, u32 tid,
203208
? "WRITE"
204209
: (sig.write_flag == SignalContext::READ ? "READ" : "UNKNOWN");
205210
Report("The signal is caused by a %s memory access.\n", access_type);
206-
if (sig.addr < GetPageSizeCached())
211+
if (!sig.is_true_faulting_addr)
212+
Report("Hint: this fault was caused by a dereference of a high value "
213+
"address (see registers below). Dissassemble the provided pc "
214+
"to learn which register value was used.\n");
215+
else if (sig.addr < GetPageSizeCached())
207216
Report("Hint: address points to the zero page.\n");
208217
}
209218
MaybeReportNonExecRegion(sig.pc);

‎compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,11 @@ bool SignalContext::IsMemoryAccess() const {
945945
return GetWriteFlag() != SignalContext::UNKNOWN;
946946
}
947947

948+
bool SignalContext::IsTrueFaultingAddress() const {
949+
// TODO: Provide real implementation for this. See Linux and Mac variants.
950+
return IsMemoryAccess();
951+
}
952+
948953
SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
949954
EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
950955
// The contents of this array are documented at
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// On x86_64, the kernel does not provide the faulting address for dereferences
2+
// of addresses greater than the 48-bit hardware addressable range, i.e.,
3+
// `siginfo.si_addr` is zero in ASan's SEGV signal handler. This test checks
4+
// that ASan does not misrepresent such cases as "NULL dereferences".
5+
6+
// REQUIRES: x86_64-target-arch
7+
// RUN: %clang_asan %s -o %t
8+
// RUN: export %env_asan_opts=print_scariness=1
9+
// RUN: not %run %t 0x0000000000000000 2>&1 | FileCheck %s --check-prefixes=ZERO,HINT-PAGE0
10+
// RUN: not %run %t 0x0000000000000FFF 2>&1 | FileCheck %s --check-prefixes=LOW1,HINT-PAGE0
11+
// RUN: not %run %t 0x0000000000001000 2>&1 | FileCheck %s --check-prefixes=LOW2,HINT-NONE
12+
// RUN: not %run %t 0x4141414141414141 2>&1 | FileCheck %s --check-prefixes=HIGH,HINT-HIGHADDR
13+
// RUN: not %run %t 0xFFFFFFFFFFFFFFFF 2>&1 | FileCheck %s --check-prefixes=MAX,HINT-HIGHADDR
14+
15+
#include <stdint.h>
16+
#include <stdlib.h>
17+
18+
int main(int argc, const char *argv[]) {
19+
const char *hex = argv[1];
20+
uint64_t *addr = (uint64_t *)strtoull(hex, NULL, 16);
21+
uint64_t x = *addr; // segmentation fault
22+
return x;
23+
}
24+
25+
// ZERO: SEGV on unknown address 0x000000000000 (pc
26+
// LOW1: SEGV on unknown address 0x000000000fff (pc
27+
// LOW2: SEGV on unknown address 0x000000001000 (pc
28+
// HIGH: SEGV on unknown address (pc
29+
// MAX: SEGV on unknown address (pc
30+
31+
// HINT-PAGE0-NOT: Hint: this fault was caused by a dereference of a high value address
32+
// HINT-PAGE0: Hint: address points to the zero page.
33+
34+
// HINT-NONE-NOT: Hint: this fault was caused by a dereference of a high value address
35+
// HINT-NONE-NOT: Hint: address points to the zero page.
36+
37+
// HINT-HIGHADDR: Hint: this fault was caused by a dereference of a high value address
38+
// HINT-HIGHADDR-NOT: Hint: address points to the zero page.
39+
40+
// ZERO: SCARINESS: 10 (null-deref)
41+
// LOW1: SCARINESS: 10 (null-deref)
42+
// LOW2: SCARINESS: 20 (wild-addr-read)
43+
// HIGH: SCARINESS: 20 (wild-addr-read)
44+
// MAX: SCARINESS: 20 (wild-addr-read)
45+
46+
// TODO: Currently, register values are only printed on Mac. Once this changes,
47+
// remove the 'TODO_' prefix in the following lines.
48+
// TODO_HIGH,TODO_MAX: Register values:
49+
// TODO_HIGH: = 0x4141414141414141
50+
// TODO_MAX: = 0xffffffffffffffff

0 commit comments

Comments
 (0)
Please sign in to comment.