[compiler-rt] [hwasan] Fix stack tag mismatch report (PR #81939)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 15 16:18:27 PST 2024
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/81939
>From dcf3d82da6cf56e21bdd17199fdff24888b7a849 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 15 Feb 2024 15:29:55 -0800
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
compiler-rt/lib/hwasan/hwasan_report.cpp | 86 +++++++++++++------
compiler-rt/test/hwasan/TestCases/stack-uas.c | 12 ++-
2 files changed, 71 insertions(+), 27 deletions(-)
diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 80763c2ada943c..f271b7db775573 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -230,33 +230,70 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
tag_t obj_tag = base_tag ^ local.tag_offset;
if (obj_tag != addr_tag)
continue;
- // Guess top bits of local variable from the faulting address, because
- // we only store bits 4-19 of FP (bits 0-3 are guaranteed to be zero).
- uptr local_beg = (fp + local.frame_offset) |
- (untagged_addr & ~(uptr(kRecordFPModulus) - 1));
- uptr local_end = local_beg + local.size;
+
+ // We only store bits 4-19 of FP (bits 0-3 are guaranteed to be zero).
+ // So we know only `FP % kRecordFPModulus`, and we can only calculate
+ // `local_beg % kRecordFPModulus`.
+ // Out of all possible `local_beg` we will only consider two candidates
+ // nearest to the `untagged_addr`.
+ uptr local_beg_mod = (fp + local.frame_offset) % kRecordFPModulus;
+ // Pick `local_beg` in the same 1 MiB block as `untagged_addr`.
+ uptr local_beg =
+ RoundDownTo(untagged_addr, kRecordFPModulus) + local_beg_mod;
+ // Now the largest `local_beg <= untagged_addr`. It's either the current
+ // one or the one before.
+ if (local_beg > untagged_addr)
+ local_beg -= kRecordFPModulus;
+
+ uptr offset = -1ull;
+ const char *whence;
+ const char *cause = nullptr;
+ uptr best_beg;
+
+ // Try two 1 MiB blocks options and pick nearest one.
+ for (uptr i = 0; i < 2; ++i, local_beg += kRecordFPModulus) {
+ uptr local_end = local_beg + local.size;
+ if (local_beg > local_end)
+ continue; // This is a wraparound.
+ if (local_beg <= untagged_addr && untagged_addr < local_end) {
+ offset = untagged_addr - local_beg;
+ whence = "inside";
+ cause = "use-after-scope";
+ best_beg = local_beg;
+ break; // This is as close at it can be.
+ }
+
+ if (untagged_addr >= local_end) {
+ uptr new_offset = untagged_addr - local_end;
+ if (new_offset < offset) {
+ offset = new_offset;
+ whence = "after";
+ cause = "stack-buffer-overflow";
+ best_beg = local_beg;
+ }
+ }
+
+ if (untagged_addr < local_beg) {
+ uptr new_offset = local_beg - untagged_addr;
+ if (new_offset < offset) {
+ offset = new_offset;
+ whence = "before";
+ cause = "stack-buffer-overflow";
+ best_beg = local_beg;
+ }
+ }
+ }
+
+ // To fail the `untagged_addr` must be near nullptr, which is impossible
+ // with Linux user space memory layout.
+ if (!cause)
+ continue;
if (!found_local) {
Printf("\nPotentially referenced stack objects:\n");
found_local = true;
}
- uptr offset;
- const char *whence;
- const char *cause;
- if (local_beg <= untagged_addr && untagged_addr < local_end) {
- offset = untagged_addr - local_beg;
- whence = "inside";
- cause = "use-after-scope";
- } else if (untagged_addr >= local_end) {
- offset = untagged_addr - local_end;
- whence = "after";
- cause = "stack-buffer-overflow";
- } else {
- offset = local_beg - untagged_addr;
- whence = "before";
- cause = "stack-buffer-overflow";
- }
Decorator d;
Printf("%s", d.Error());
Printf("Cause: %s\n", cause);
@@ -267,10 +304,11 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
common_flags()->symbolize_vs_style,
common_flags()->strip_path_prefix);
Printf(
- "%p is located %zd bytes %s a %zd-byte local variable %s [%p,%p) "
+ "%p is located %zd bytes %s a %zd-byte local variable %s "
+ "[%p,%p) "
"in %s %s\n",
- untagged_addr, offset, whence, local_end - local_beg, local.name,
- local_beg, local_end, local.function_name, location.data());
+ untagged_addr, offset, whence, local.size, local.name, best_beg,
+ best_beg + local.size, local.function_name, location.data());
location.clear();
Printf("%s\n", d.Default());
}
diff --git a/compiler-rt/test/hwasan/TestCases/stack-uas.c b/compiler-rt/test/hwasan/TestCases/stack-uas.c
index c8a8ee4f7e8092..1875b15df84050 100644
--- a/compiler-rt/test/hwasan/TestCases/stack-uas.c
+++ b/compiler-rt/test/hwasan/TestCases/stack-uas.c
@@ -1,6 +1,8 @@
// Tests use-after-scope detection and reporting.
// RUN: %clang_hwasan -O0 -g %s -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clang_hwasan -O2 -g %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang_hwasan -O2 -g %s -DBUFFER_SIZE=1000000 -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang_hwasan -O2 -g %s -DBUFFER_SIZE=2000000 -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clang_hwasan -g %s -o %t && not %env_hwasan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
// RUN: %clang_hwasan -mllvm -hwasan-use-after-scope=false -g %s -o %t && %run %t 2>&1
@@ -18,6 +20,10 @@
#include <sanitizer/hwasan_interface.h>
#include <stdio.h>
+#ifndef BUFFER_SIZE
+# define BUFFER_SIZE 0x800
+#endif
+
void USE(void *x) { // pretend_to_do_something(void *x)
__asm__ __volatile__(""
:
@@ -41,8 +47,8 @@ __attribute__((noinline)) void Unrelated3() {
__attribute__((noinline)) char buggy() {
char *volatile p;
{
- char zzz[0x800] = {};
- char yyy[0x800] = {};
+ char zzz[BUFFER_SIZE] = {};
+ char yyy[BUFFER_SIZE] = {};
// With -hwasan-generate-tags-with-calls=false, stack tags can occasionally
// be zero, leading to a false negative
// (https://github.com/llvm/llvm-project/issues/69221). Work around it by
@@ -71,7 +77,7 @@ int main() {
// CHECK: is located in stack of thread
// CHECK: Potentially referenced stack objects:
// CHECK: Cause: use-after-scope
- // CHECK-NEXT: 0x{{.*}} is located 0 bytes inside a 2048-byte local variable {{zzz|yyy}} [0x{{.*}}) in buggy {{.*}}stack-uas.c:
+ // CHECK-NEXT: 0x{{.*}} is located 0 bytes inside a {{.*}}-byte local variable {{zzz|yyy}} [0x{{.*}}) in buggy {{.*}}stack-uas.c:
// CHECK: Memory tags around the buggy address
// NOSYM: Previously allocated frames:
>From 2da6d679174d8ad9e71f432af7e3ed57df388bda Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 15 Feb 2024 15:33:52 -0800
Subject: [PATCH 2/3] comment
Created using spr 1.3.4
---
compiler-rt/lib/hwasan/hwasan_report.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index f271b7db775573..f6ef14133cb0cb 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -240,7 +240,7 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
// Pick `local_beg` in the same 1 MiB block as `untagged_addr`.
uptr local_beg =
RoundDownTo(untagged_addr, kRecordFPModulus) + local_beg_mod;
- // Now the largest `local_beg <= untagged_addr`. It's either the current
+ // Pick the largest `local_beg <= untagged_addr`. It's either the current
// one or the one before.
if (local_beg > untagged_addr)
local_beg -= kRecordFPModulus;
>From 52b097640ff37a42100f34af1bee7816a4e151de Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 15 Feb 2024 16:18:12 -0800
Subject: [PATCH 3/3] addressing reviewers comments
Created using spr 1.3.4
---
compiler-rt/lib/hwasan/hwasan_report.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index f6ef14133cb0cb..016ec182dea0fd 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -234,7 +234,7 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
// We only store bits 4-19 of FP (bits 0-3 are guaranteed to be zero).
// So we know only `FP % kRecordFPModulus`, and we can only calculate
// `local_beg % kRecordFPModulus`.
- // Out of all possible `local_beg` we will only consider two candidates
+ // Out of all possible `local_beg` we will only consider 2 candidates
// nearest to the `untagged_addr`.
uptr local_beg_mod = (fp + local.frame_offset) % kRecordFPModulus;
// Pick `local_beg` in the same 1 MiB block as `untagged_addr`.
@@ -271,9 +271,7 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
cause = "stack-buffer-overflow";
best_beg = local_beg;
}
- }
-
- if (untagged_addr < local_beg) {
+ } else {
uptr new_offset = local_beg - untagged_addr;
if (new_offset < offset) {
offset = new_offset;
More information about the llvm-commits
mailing list