[hwasan] Fixing false invalid-free with disabled tagging (#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.

Author: vitalybuka

compiler-rt/lib/hwasan/hwasan_allocator.cpp

@@ -234,28 +234,23 @@ static void *HwasanAllocate(StackTrace *stack, uptr orig_size, uptr alignment,
   }
 
   void *user_ptr = allocated;
-  // Tagging can only be skipped when both tag_in_malloc and tag_in_free are
-  // false. When tag_in_malloc = false and tag_in_free = true malloc needs to
-  // 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 {
+    // Tagging can not be completely skipped. If it's disabled, we need to tag
+    // with zeros.
+    user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
   }
 
   Metadata *meta =

compiler-rt/test/hwasan/TestCases/enable-disable.c

@@ -0,0 +1,42 @@
+// Test that disabling/enabling tagging does not trigger false reports on
+// allocations happened in a different state.
+
+// RUN: %clang_hwasan -O1 %s -o %t && %run %t 2>&1
+
+#include <assert.h>
+#include <sanitizer/hwasan_interface.h>
+#include <stdlib.h>
+
+enum {
+  COUNT = 5,
+  SZ = 10,
+};
+void *x[COUNT];
+
+int main() {
+  __hwasan_enable_allocator_tagging();
+  for (unsigned i = 0; i < COUNT; ++i) {
+    x[i] = malloc(SZ);
+    assert(__hwasan_test_shadow(x[i], SZ) == -1);
+  }
+  for (unsigned i = 0; i < COUNT; ++i)
+    free(x[i]);
+
+  __hwasan_disable_allocator_tagging();
+  for (unsigned i = 0; i < COUNT; ++i) {
+    x[i] = malloc(SZ);
+    assert(__hwasan_tag_pointer(x[i], 0) == x[i]);
+    assert(__hwasan_test_shadow(x[i], SZ) == -1);
+  }
+  for (unsigned i = 0; i < COUNT; ++i)
+    free(x[i]);
+
+  __hwasan_enable_allocator_tagging();
+  for (unsigned i = 0; i < COUNT; ++i) {
+    x[i] = malloc(SZ);
+    assert(__hwasan_test_shadow(x[i], SZ) == -1);
+  }
+  for (unsigned i = 0; i < COUNT; ++i)
+    free(x[i]);
+  return 0;
+}