From f694ab1f18df8beaec81fcabf7edf4818f7ffbac Mon Sep 17 00:00:00 2001 From: Kostya Serebryany Date: Mon, 14 Apr 2014 09:50:52 +0000 Subject: [PATCH] [asan] provide better reports for cases where memcpy/etc get negative size parameter. Also fix a typo found by Tetsuo Kiso llvm-svn: 206158 --- compiler-rt/lib/asan/asan_interceptors.cc | 4 ++++ compiler-rt/lib/asan/asan_poisoning.cc | 1 + compiler-rt/lib/asan/asan_report.cc | 13 +++++++++++++ compiler-rt/lib/asan/asan_report.h | 2 ++ compiler-rt/lib/sanitizer_common/sanitizer_posix.cc | 2 +- compiler-rt/test/asan/TestCases/memset_test.cc | 9 +++++++++ 6 files changed, 30 insertions(+), 1 deletion(-) diff --git a/compiler-rt/lib/asan/asan_interceptors.cc b/compiler-rt/lib/asan/asan_interceptors.cc index ddd92c7306b8..22fc31565805 100644 --- a/compiler-rt/lib/asan/asan_interceptors.cc +++ b/compiler-rt/lib/asan/asan_interceptors.cc @@ -43,6 +43,10 @@ static inline bool QuickCheckForUnpoisonedRegion(uptr beg, uptr size) { uptr __offset = (uptr)(offset); \ uptr __size = (uptr)(size); \ uptr __bad = 0; \ + if (__offset > __offset + __size) { \ + GET_STACK_TRACE_FATAL_HERE; \ + ReportStringFunctionSizeOverflow(__offset, __size, &stack); \ + } \ if (!QuickCheckForUnpoisonedRegion(__offset, __size) && \ (__bad = __asan_region_is_poisoned(__offset, __size))) { \ GET_CURRENT_PC_BP_SP; \ diff --git a/compiler-rt/lib/asan/asan_poisoning.cc b/compiler-rt/lib/asan/asan_poisoning.cc index f4792a85e88b..0e1d2baf6ae5 100644 --- a/compiler-rt/lib/asan/asan_poisoning.cc +++ b/compiler-rt/lib/asan/asan_poisoning.cc @@ -155,6 +155,7 @@ uptr __asan_region_is_poisoned(uptr beg, uptr size) { uptr end = beg + size; if (!AddrIsInMem(beg)) return beg; if (!AddrIsInMem(end)) return end; + CHECK_LT(beg, end); uptr aligned_b = RoundUpTo(beg, SHADOW_GRANULARITY); uptr aligned_e = RoundDownTo(end, SHADOW_GRANULARITY); uptr shadow_beg = MemToShadow(aligned_b); diff --git a/compiler-rt/lib/asan/asan_report.cc b/compiler-rt/lib/asan/asan_report.cc index 2a6de8ecfd23..c50df27cdb0f 100644 --- a/compiler-rt/lib/asan/asan_report.cc +++ b/compiler-rt/lib/asan/asan_report.cc @@ -701,6 +701,19 @@ void ReportStringFunctionMemoryRangesOverlap( ReportErrorSummary(bug_type, stack); } +void ReportStringFunctionSizeOverflow(uptr offset, uptr size, + StackTrace *stack) { + ScopedInErrorReport in_report; + Decorator d; + const char *bug_type = "negative-size-param"; + Printf("%s", d.Warning()); + Report("ERROR: AddressSanitizer: %s: (size=%zd)\n", bug_type, size); + Printf("%s", d.EndWarning()); + stack->Print(); + DescribeAddress(offset, size); + ReportErrorSummary(bug_type, stack); +} + void ReportBadParamsToAnnotateContiguousContainer(uptr beg, uptr end, uptr old_mid, uptr new_mid, StackTrace *stack) { diff --git a/compiler-rt/lib/asan/asan_report.h b/compiler-rt/lib/asan/asan_report.h index 84111b163ad7..3843a2ea2cd3 100644 --- a/compiler-rt/lib/asan/asan_report.h +++ b/compiler-rt/lib/asan/asan_report.h @@ -49,6 +49,8 @@ void NORETURN ReportStringFunctionMemoryRangesOverlap( const char *function, const char *offset1, uptr length1, const char *offset2, uptr length2, StackTrace *stack); void NORETURN +ReportStringFunctionSizeOverflow(uptr offset, uptr size, StackTrace *stack); +void NORETURN ReportBadParamsToAnnotateContiguousContainer(uptr beg, uptr end, uptr old_mid, uptr new_mid, StackTrace *stack); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc index fd1ee6ea2146..f3e8f3855398 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc @@ -52,7 +52,7 @@ uptr GetMaxVirtualAddress() { #if SANITIZER_WORDSIZE == 64 # if defined(__powerpc64__) // On PowerPC64 we have two different address space layouts: 44- and 46-bit. - // We somehow need to figure our which one we are using now and choose + // We somehow need to figure out which one we are using now and choose // one of 0x00000fffffffffffUL and 0x00003fffffffffffUL. // Note that with 'ulimit -s unlimited' the stack is moved away from the top // of the address space, so simply checking the stack address is not enough. diff --git a/compiler-rt/test/asan/TestCases/memset_test.cc b/compiler-rt/test/asan/TestCases/memset_test.cc index ea7702566fcb..fc796f5bc9c9 100644 --- a/compiler-rt/test/asan/TestCases/memset_test.cc +++ b/compiler-rt/test/asan/TestCases/memset_test.cc @@ -27,6 +27,9 @@ // RUN: %clangxx_asan -O3 -DTEST_MEMMOVE %s -o %t && not %t 2>&1 | \ // RUN: FileCheck %s --check-prefix=CHECK-MEMMOVE +// RUN: %clangxx_asan -O2 -DTEST_MEMCPY_SIZE_OVERFLOW %s -o %t && not %t 2>&1 | \ +// RUN: FileCheck %s --check-prefix=CHECK-MEMCPY_SIZE_OVERFLOW + #include #include #include @@ -34,6 +37,8 @@ #include +typedef void *(*memcpy_t)(void *, const void *, size_t); + int main(int argc, char **argv) { char * volatile p = (char *)malloc(3000); __asan_poison_memory_region(p + 512, 16); @@ -53,6 +58,10 @@ int main(int argc, char **argv) { memmove(q, p, 3000); // CHECK-MEMMOVE: AddressSanitizer: use-after-poison on address // CHECK-MEMMOVE: in {{.*(memmove|memcpy)}} +#elif defined(TEST_MEMCPY_SIZE_OVERFLOW) + volatile memcpy_t my_memcpy = &memcpy; + my_memcpy(p, q, -argc); + // CHECK-MEMCPY_SIZE_OVERFLOW: AddressSanitizer: negative-size-param: (size=-1) #endif assert(q[1] == 0); free(q);