diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp --- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp +++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp @@ -219,9 +219,11 @@ 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 @@ 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 @@ 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 --- a/compiler-rt/lib/hwasan/hwasan_report.cpp +++ b/compiler-rt/lib/hwasan/hwasan_report.cpp @@ -604,6 +604,15 @@ 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 @@ 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 --- 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 @@ // 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; }