Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hwasan] Fix stack tag mismatch report #81939

Merged

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Feb 15, 2024

Existing code worked only for local, recorder FP, and the faulty address are the same 1 MiB page.

Now, instead of guessing FP, we guess variable address.
We need to try just two cases of addresses around of faulty one.

Fixes google/sanitizers#1723

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

Existing code worked only for local and the faulty address are the same 1 MiB page.

Now we try the cases when local starts in nebouring blocks.

Fixes #1723.


Full diff: https://github.com/llvm/llvm-project/pull/81939.diff

2 Files Affected:

  • (modified) compiler-rt/lib/hwasan/hwasan_report.cpp (+62-24)
  • (modified) compiler-rt/test/hwasan/TestCases/stack-uas.c (+9-3)
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:

Created using spr 1.3.4
compiler-rt/lib/hwasan/hwasan_report.cpp Outdated Show resolved Hide resolved
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 39e32b4 into main Feb 16, 2024
3 of 4 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/hwasan-fix-stack-tag-mismatch-report branch February 16, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestCases/stack-uas.c occasionally misclassified as stack-buffer-overflow
4 participants