[hwasan] Do not use short granule tags as poison tags.

[fmayer]

Summary

Short granule tags as poison cause a UaF to read the referenced
memory to retrieve the tag, and means we do not detect the UaF
if the last granule's tag is still around.

This only increases the change of not catching a UaF from
0.39 % (1 / 256) to 0.42 % (1 / (256 - 17)).

[fmayer]

Static assert for fallback tag.

[fmayer]

Fix formatting.

[eugenis]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

240

I think it would be cleaner and faster to loop until GenerateRandomTag returns something >=kShadowAlignment - that would be just 1 unlikely branch compared to all the stuff GenerateRandomTag does on each call.

Or even pass a minimum tag value as an argument - there is a loop like that inside GenerateRandomTag already.

[fmayer]

Simplify logic.

[eugenis]

LGTM

compiler-rt/lib/hwasan/hwasan_allocator.cpp

if (flags()->max_free_fill_size > 0) {
     uptr fill_size =
         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) &&
       atomic_load_relaxed(&hwasan_allocator_tagging_enabled)) {
     // Always store full 8-bit tags on free to maximize UAF detection.
-    tag_t tag = t ? t->GenerateRandomTag(/*num_bits=*/8) : kFallbackFreeTag;
+    tag_t tag;
+    if (t) {
+      // Make sure we are not using a short granule tag as a poison tag. This
+      // would make us attempt to read the memory on a UaF.
+      // The tag can be zero if tagging is disabled on this thread.
+      do {
+        tag = t->GenerateRandomTag(/*num_bits=*/8);
+      } while (UNLIKELY(tag < kShadowAlignment && tag != 0));
+    } else {
+      static_assert(kFallbackFreeTag >= kShadowAlignment,
+                    "fallback tag must not be a short granule tag.");
+      tag = kFallbackFreeTag;
+    }
     TagMemoryAligned(reinterpret_cast<uptr>(aligned_ptr), TaggedSize(orig_size),
                      tag);
   }
   if (t) {
     allocator.Deallocate(t->allocator_cache(), aligned_ptr);
     if (auto *ha = t->heap_allocations())
       ha->push({reinterpret_cast<uptr>(tagged_ptr), alloc_context_id,
                 free_context_id, static_cast<u32>(orig_size)});