From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Wed, 18 Aug 2021 16:36:48 +0000 (-0700) Subject: [hwasan] Don't report short-granule shadow as overwritten. X-Git-Tag: upstream/15.0.7~33584 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fd51ab634143e0c1be49a62e16616ba5ab89273e;p=platform%2Fupstream%2Fllvm.git [hwasan] Don't report short-granule shadow as overwritten. The shadow for a short granule is stored in the last byte of the granule. Currently, if there's a tail-overwrite report (a buffer-overflow-write in uninstrumented code), we report the shadow byte as a mismatch against the magic. Fix this bug by slapping the shadow into the expected value. This also makes sure that if the uninstrumented WRITE does clobber the shadow byte, it reports the shadow was actually clobbered as well. Reviewed By: eugenis, fmayer Differential Revision: https://reviews.llvm.org/D107938 --- diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp index 482eaa2..63d86cf 100644 --- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp +++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp @@ -219,9 +219,11 @@ static bool CheckInvalidFree(StackTrace *stack, void *untagged_ptr, static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { CHECK(tagged_ptr); HWASAN_FREE_HOOK(tagged_ptr); - void *untagged_ptr = InTaggableRegion(reinterpret_cast(tagged_ptr)) - ? UntagPtr(tagged_ptr) - : tagged_ptr; + + bool in_taggable_region = + InTaggableRegion(reinterpret_cast(tagged_ptr)); + void *untagged_ptr = in_taggable_region ? UntagPtr(tagged_ptr) : tagged_ptr; + if (CheckInvalidFree(stack, untagged_ptr, tagged_ptr)) return; @@ -246,7 +248,11 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { CHECK_LT(tail_size, kShadowAlignment); void *tail_beg = reinterpret_cast( reinterpret_cast(aligned_ptr) + orig_size); - if (tail_size && internal_memcmp(tail_beg, tail_magic, tail_size)) + tag_t short_granule_memtag = *(reinterpret_cast( + reinterpret_cast(tail_beg) + tail_size)); + if (tail_size && + (internal_memcmp(tail_beg, tail_magic, tail_size) || + (in_taggable_region && pointer_tag != short_granule_memtag))) ReportTailOverwritten(stack, reinterpret_cast(tagged_ptr), orig_size, tail_magic); } @@ -261,8 +267,7 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { Min(TaggedSize(orig_size), (uptr)flags()->max_free_fill_size); internal_memset(aligned_ptr, flags()->free_fill_byte, fill_size); } - if (InTaggableRegion(reinterpret_cast(tagged_ptr)) && - flags()->tag_in_free && malloc_bisect(stack, 0) && + if (in_taggable_region && flags()->tag_in_free && malloc_bisect(stack, 0) && atomic_load_relaxed(&hwasan_allocator_tagging_enabled)) { // Always store full 8-bit tags on free to maximize UAF detection. tag_t tag; diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp index 8e7f3bb..5beb25c 100644 --- a/compiler-rt/lib/hwasan/hwasan_report.cpp +++ b/compiler-rt/lib/hwasan/hwasan_report.cpp @@ -604,6 +604,15 @@ void ReportInvalidFree(StackTrace *stack, uptr tagged_addr) { void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size, const u8 *expected) { uptr tail_size = kShadowAlignment - (orig_size % kShadowAlignment); + u8 actual_expected[kShadowAlignment]; + internal_memcpy(actual_expected, expected, tail_size); + tag_t ptr_tag = GetTagFromPointer(tagged_addr); + // Short granule is stashed in the last byte of the magic string. To avoid + // confusion, make the expected magic string contain the short granule tag. + if (orig_size % kShadowAlignment != 0) { + actual_expected[tail_size - 1] = ptr_tag; + } + ScopedReport R(flags()->halt_on_error); Decorator d; uptr untagged_addr = UntagAddr(tagged_addr); @@ -640,14 +649,13 @@ void ReportTailOverwritten(StackTrace *stack, uptr tagged_addr, uptr orig_size, s.append("Expected: "); for (uptr i = 0; i < kShadowAlignment - tail_size; i++) s.append(".. "); - for (uptr i = 0; i < tail_size; i++) - s.append("%02x ", expected[i]); + for (uptr i = 0; i < tail_size; i++) s.append("%02x ", actual_expected[i]); s.append("\n"); s.append(" "); for (uptr i = 0; i < kShadowAlignment - tail_size; i++) s.append(" "); for (uptr i = 0; i < tail_size; i++) - s.append("%s ", expected[i] != tail[i] ? "^^" : " "); + s.append("%s ", actual_expected[i] != tail[i] ? "^^" : " "); s.append("\nThis error occurs when a buffer overflow overwrites memory\n" "to the right of a heap object, but within the %zd-byte granule, e.g.\n" diff --git a/compiler-rt/test/hwasan/TestCases/tail-magic.c b/compiler-rt/test/hwasan/TestCases/tail-magic.c index fcbc8f1..efece53 100644 --- a/compiler-rt/test/hwasan/TestCases/tail-magic.c +++ b/compiler-rt/test/hwasan/TestCases/tail-magic.c @@ -1,8 +1,13 @@ // Tests free_checks_tail_magic=1. -// RUN: %clang_hwasan %s -o %t +// RUN: %clang_hwasan %s -o %t // RUN: %env_hwasan_opts=free_checks_tail_magic=0 %run %t -// RUN: %env_hwasan_opts=free_checks_tail_magic=1 not %run %t 2>&1 | FileCheck %s -// RUN: not %run %t 2>&1 | FileCheck %s +// RUN: %env_hwasan_opts=free_checks_tail_magic=1 not %run %t 2>&1 | \ +// RUN: FileCheck --check-prefixes=CHECK,CHECK-NONLASTGRANULE --strict-whitespace %s +// RUN: not %run %t 2>&1 | \ +// RUN: FileCheck --check-prefixes=CHECK,CHECK-NONLASTGRANULE --strict-whitespace %s +// RUN: %clang_hwasan -DLAST_GRANULE %s -o %t +// RUN: not %run %t 2>&1 | \ +// RUN: FileCheck --check-prefixes=CHECK,CHECK-LASTGRANULE --strict-whitespace %s // REQUIRES: stable-runtime @@ -15,22 +20,33 @@ static volatile char *sink; // Overwrite the tail in a non-hwasan function so that we don't detect the // stores as OOB. __attribute__((no_sanitize("hwaddress"))) void overwrite_tail() { +#ifdef LAST_GRANULE + sink[31] = 0x71; +#else // LAST_GRANULE sink[20] = 0x42; sink[24] = 0x66; +#endif // LAST_GRANULE } int main(int argc, char **argv) { __hwasan_enable_allocator_tagging(); char *p = (char*)malloc(20); + __hwasan_print_shadow(p, 1); sink = p; overwrite_tail(); free(p); +// CHECK: HWASan shadow map for {{.*}} (pointer tag [[TAG:[a-f0-9]+]]) // CHECK: ERROR: HWAddressSanitizer: allocation-tail-overwritten; heap object [{{.*}}) of size 20 // CHECK: Stack of invalid access unknown. Issue detected at deallocation time. // CHECK: deallocated here: -// CHECK: in main {{.*}}tail-magic.c:[[@LINE-4]] +// CHECK: in main {{.*}}tail-magic.c:[[@LINE-5]] // CHECK: allocated here: -// CHECK: in main {{.*}}tail-magic.c:[[@LINE-9]] -// CHECK: Tail contains: .. .. .. .. 42 {{.. .. ..}} 66 +// CHECK: in main {{.*}}tail-magic.c:[[@LINE-11]] +// CHECK-NONLASTGRANULE: Tail contains: .. .. .. .. 42 {{(([a-f0-9]{2} ){3})}}66 +// CHECK-LASTGRANULE: Tail contains: .. .. .. .. {{(([a-f0-9]{2} ?)+)}}71{{ *$}} +// CHECK-NEXT: Expected: {{ +}} .. .. .. .. {{([a-f0-9]{2} )+0?}}[[TAG]]{{ *$}} +// CHECK-NONLASTGRANULE-NEXT: {{ +}}^^{{ +}}^^{{ *$}} +// CHECK-LASTGRANULE-NEXT: {{ +}}^^{{ *$}} + return 0; }