Handle libhwasan system allocator fallback during thread initialisation

[mmalcomson]

Summary

The system allocator fallback added in https://reviews.llvm.org/D55986
(llvm-svn: 350427) introduces the assumption that all allocations with a zero
tag have been allocated by the system allocator.

During thread initialisation this assumption no longer holds.
Thread::Init disables tagging using the ScopedTaggingDisabler, until its
internal data structures have been initialised so that
Thread::GenerateRandomTag can generate a random tag.

While libhwasan finds the stack bounds using pthread attributes, libc allocates
and frees an object at the time that stack tagging is disabled. Hence the
allocation is handled by libhwasan and the free is given to the system
allocator.

I have attached a patch here that makes the hwasan allocation routine pass
allocation off to the system allocator if tagging is disabled in the current
thread.

Another approach I considered was to make GenerateRandomTag return a known
non-zero tag when tagging is disabled for the current thread.
I decided against this since the tags of other pointers seem to be zero when
tagging is disabled (e.g. for flags->disable_allocator_tagging).

Testing done manually on an AArch64 VM using both GCC and clang.

This patch makes GenerateRandomTag return a known non-zero tag when tagging
is disabled, which essentially marks whatever pointer is being created as
coming from the internal allocator.

[mmalcomson]

Remove some superfluous semicolons that were causing warnings.

[eugenis]

Another approach I considered was to make GenerateRandomTag return a known
non-zero tag when tagging is disabled for the current thread.

I think this is not a bad idea. Could we use a fixed non-zero tag in the allocator under the same condition, i.e.

(t->TaggingIsDisabled() && !flags()->disable_allocator_tagging)

@pcc WDYT?

compiler-rt/lib/hwasan/hwasan_allocator.cpp

130 ↗

(On Diff #225457)

Wrong indentation here makes it look like REAL(malloc) is called outside of both if-s.
Please use clang-format, and also add {} braces for the outer if block.

[pcc]

In D69051#1713765, @eugenis wrote:

Another approach I considered was to make GenerateRandomTag return a known
non-zero tag when tagging is disabled for the current thread.

I think this is not a bad idea. Could we use a fixed non-zero tag in the allocator under the same condition, i.e.

(t->TaggingIsDisabled() && !flags()->disable_allocator_tagging)

@pcc WDYT?

SGTM.

[mmalcomson]

Now avoid returning a zero tag when tagging is disabled.

I've avoided that by returning a fixed non-zero tag from Thread::GenerateRandomTag.

NOTE: There's the same basic problem of tagging with zero going to the system allocator free when setting HWASAN_OPTIONS tag_in_malloc=false. I intend to fix that in a separate patch.

[eugenis]

In fact, we are not going to pursue this malloc fallback feature any more, and I doubt anyone else is interested.
Let's simply remove it: D69199.

[mmalcomson]

Closing given the allocator fallback feature is not being pursued any more.

compiler-rt/lib/hwasan/hwasan_thread.cpp

static u32 xorshift(u32 state) {
   state ^= state << 13;
   state ^= state >> 17;
   state ^= state << 5;
   return state;
 }
 
 // Generate a (pseudo-)random non-zero tag.
 tag_t Thread::GenerateRandomTag() {
-  if (tagging_disabled_) return 0;
+  // Return non-zero when tagging is not disabled so that the allocator
+  // fallback knows whatever pointer is being created has not come from the
+  // system allocator.
+  if (tagging_disabled_)
+#if HWASAN_WITH_INTERCEPTORS
+    return flags()->disable_allocator_tagging ? 0 : 0x10;
+#else
+    return 0;
+#endif
   tag_t tag;
   do {
     if (flags()->random_tags) {
       if (!random_buffer_)
         random_buffer_ = random_state_ = xorshift(random_state_);
       CHECK(random_buffer_);
       tag = random_buffer_ & 0xFF;
       random_buffer_ >>= 8;
     } else {
       tag = random_state_ = (random_state_ + 1) & 0xFF;
     }
   } while (!tag);
   return tag;
 }
 
 } // namespace __hwasan