Skip to content

Commit 9b87b25

Browse files
authored
Merge pull request #150 from apple/cherry-pick-50366151
Reland "[ASan] Do not misrepresent high value address dereferences as…
2 parents f1af536 + 8474856 commit 9b87b25

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
@@ -880,6 +880,11 @@ struct SignalContext {
880880
bool is_memory_access;
881881
enum WriteFlag { UNKNOWN, READ, WRITE } write_flag;
882882

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

@@ -913,6 +919,7 @@ struct SignalContext {
913919
uptr GetAddress() const;
914920
WriteFlag GetWriteFlag() const;
915921
bool IsMemoryAccess() const;
922+
bool IsTrueFaultingAddress() const;
916923
};
917924

918925
void InitializePlatformEarly();

compiler-rt/lib/sanitizer_common/sanitizer_linux.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,12 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
18451845
#endif
18461846
}
18471847

1848+
bool SignalContext::IsTrueFaultingAddress() const {
1849+
auto si = static_cast<const siginfo_t *>(siginfo);
1850+
// SIGSEGV signals without a true fault address have si_code set to 128.
1851+
return si->si_signo == SIGSEGV && si->si_code != 128;
1852+
}
1853+
18481854
void SignalContext::DumpAllRegisters(void *context) {
18491855
// FIXME: Implement this.
18501856
}

compiler-rt/lib/sanitizer_common/sanitizer_mac.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,12 @@ SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
676676
#endif
677677
}
678678

679+
bool SignalContext::IsTrueFaultingAddress() const {
680+
auto si = static_cast<const siginfo_t *>(siginfo);
681+
// "Real" SIGSEGV codes (e.g., SEGV_MAPERR, SEGV_MAPERR) are non-zero.
682+
return si->si_signo == SIGSEGV && si->si_code != 0;
683+
}
684+
679685
static void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp) {
680686
ucontext_t *ucontext = (ucontext_t*)context;
681687
# if defined(__aarch64__)

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cc

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

compiler-rt/lib/sanitizer_common/sanitizer_win.cc

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

944+
bool SignalContext::IsTrueFaultingAddress() const {
945+
// FIXME: Provide real implementation for this. See Linux and Mac variants.
946+
return IsMemoryAccess();
947+
}
948+
944949
SignalContext::WriteFlag SignalContext::GetWriteFlag() const {
945950
EXCEPTION_RECORD *exception_record = (EXCEPTION_RECORD *)siginfo;
946951
// 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)