[compiler-rt] fd51ab6 - [hwasan] Don't report short-granule shadow as overwritten.
Mitch Phillips via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 18 11:26:11 PDT 2021
Author: Mitch Phillips
Date: 2021-08-18T11:25:57-07:00
New Revision: fd51ab634143e0c1be49a62e16616ba5ab89273e
URL: https://github.com/llvm/llvm-project/commit/fd51ab634143e0c1be49a62e16616ba5ab89273e
DIFF: https://github.com/llvm/llvm-project/commit/fd51ab634143e0c1be49a62e16616ba5ab89273e.diff
LOG: [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
Added:
Modified:
compiler-rt/lib/hwasan/hwasan_allocator.cpp
compiler-rt/lib/hwasan/hwasan_report.cpp
compiler-rt/test/hwasan/TestCases/tail-magic.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
index 482eaa2e0ff76..63d86cf99e582 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<uptr>(tagged_ptr))
- ? UntagPtr(tagged_ptr)
- : tagged_ptr;
+
+ bool in_taggable_region =
+ InTaggableRegion(reinterpret_cast<uptr>(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<void *>(
reinterpret_cast<uptr>(aligned_ptr) + orig_size);
- if (tail_size && internal_memcmp(tail_beg, tail_magic, tail_size))
+ tag_t short_granule_memtag = *(reinterpret_cast<tag_t *>(
+ reinterpret_cast<uptr>(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<uptr>(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<uptr>(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 8e7f3bbf26bfb..5beb25cd512fb 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 fcbc8f115d73c..efece53aa699c 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;
}
More information about the llvm-commits
mailing list