[compiler-rt] [hwasan] Fixing false invalid-free with disabled tagging (PR #67169)

[Vitaly Buka via llvm-commits]

https://github.com/vitalybuka created https://github.com/llvm/llvm-project/pull/67169

This problem was accidentally discovered by the internal symbolizer, but
it's relevant for external one as well, see the test.

If we just disable tagging, there may still be tagged allocations that
have already been freed. After disabling tagging, these tagged
allocations can be released to the user as-is, which would later break
the "invalid-free" check.

We cannot just disable the "invalid-free" check with disabled tagging,
because if we re-enable tagging, the issue still applies to allocations
created when it was disabled.

The fix is to continue tagging with zero even if tagging is disabled.

This makes the "disabled" mode less efficient, but this is not the
primary use case.


>From 76a5c93f2d950b420b5bc8318712073ecbe4b679 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 22 Sep 2023 10:35:12 -0700
Subject: [PATCH] [hwasan] Fixing false invalid-free with disabled tagging

This problem was accidentally discovered by the internal symbolizer, but
it's relevant for external one as well, see the test.

If we just disable tagging, there may still be tagged allocations that
have already been freed. After disabling tagging, these tagged
allocations can be released to the user as-is, which would later break
the "invalid-free" check.

We cannot just disable the "invalid-free" check with disabled tagging,
because if we re-enable tagging, the issue still applies to allocations
created when it was disabled.

The fix is to continue tagging with zero even if tagging is disabled.

This makes the "disabled" mode less efficient, but this is not the
primary use case.
---
 compiler-rt/lib/hwasan/hwasan_allocator.cpp   | 29 +++++++++----------
 .../test/hwasan/TestCases/enable-disable.c    | 23 +++++++++++++++
 2 files changed, 36 insertions(+), 16 deletions(-)
 create mode 100644 compiler-rt/test/hwasan/TestCases/enable-disable.c

diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
index c99d730e059faab..7cebe2c4d8c6721 100644
--- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
@@ -239,23 +239,20 @@ static void *HwasanAllocate(StackTrace *stack, uptr orig_size, uptr alignment,
   // retag to 0.
   if (InTaggableRegion(reinterpret_cast<uptr>(user_ptr)) &&
       (flags()->tag_in_malloc || flags()->tag_in_free) &&
-      atomic_load_relaxed(&hwasan_allocator_tagging_enabled)) {
-    if (flags()->tag_in_malloc && malloc_bisect(stack, orig_size)) {
-      tag_t tag = t ? t->GenerateRandomTag() : kFallbackAllocTag;
-      uptr tag_size = orig_size ? orig_size : 1;
-      uptr full_granule_size = RoundDownTo(tag_size, kShadowAlignment);
-      user_ptr =
-          (void *)TagMemoryAligned((uptr)user_ptr, full_granule_size, tag);
-      if (full_granule_size != tag_size) {
-        u8 *short_granule =
-            reinterpret_cast<u8 *>(allocated) + full_granule_size;
-        TagMemoryAligned((uptr)short_granule, kShadowAlignment,
-                         tag_size % kShadowAlignment);
-        short_granule[kShadowAlignment - 1] = tag;
-      }
-    } else {
-      user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
+      atomic_load_relaxed(&hwasan_allocator_tagging_enabled) &&
+      flags()->tag_in_malloc && malloc_bisect(stack, orig_size)) {
+    tag_t tag = t ? t->GenerateRandomTag() : kFallbackAllocTag;
+    uptr tag_size = orig_size ? orig_size : 1;
+    uptr full_granule_size = RoundDownTo(tag_size, kShadowAlignment);
+    user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, full_granule_size, tag);
+    if (full_granule_size != tag_size) {
+      u8 *short_granule = reinterpret_cast<u8 *>(allocated) + full_granule_size;
+      TagMemoryAligned((uptr)short_granule, kShadowAlignment,
+                       tag_size % kShadowAlignment);
+      short_granule[kShadowAlignment - 1] = tag;
     }
+  } else {
+    user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
   }
 
   Metadata *meta =
diff --git a/compiler-rt/test/hwasan/TestCases/enable-disable.c b/compiler-rt/test/hwasan/TestCases/enable-disable.c
new file mode 100644
index 000000000000000..d686b6bdc62fd3c
--- /dev/null
+++ b/compiler-rt/test/hwasan/TestCases/enable-disable.c
@@ -0,0 +1,23 @@
+// RUN: %clang_hwasan -O1 %s -o %t && %run %t 2>&1
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <sanitizer/hwasan_interface.h>
+
+#define SZ 1000
+void * x[SZ];
+
+int main() {
+  __hwasan_enable_allocator_tagging();
+  for (unsigned i = 0; i < SZ; ++i)
+    x[i] = malloc(10);
+  for (unsigned i = 0; i < SZ; ++i) 
+    free(x[i]);
+  __hwasan_disable_allocator_tagging();
+  
+  for (unsigned i = 0; i < SZ; ++i)
+    x[i] = malloc(10);
+  for (unsigned i = 0; i < SZ; ++i)
+    free(x[i]);
+  return 0;
+}