[scudo] Fix bad request handling when allocator has not been initialized

[cryptoad]

Summary

In a few functions (scudoMemalign and the like), we would call
ScudoAllocator::FailureHandler::OnBadRequest if the parameters didn't check
out. The issue is that if the allocator had not been initialized (eg: if this
is the first heap related function called), we would use variables like
allocator_may_return_null and exitcode that still had their default value
(as opposed to the one set by the user or the initialization path).

To solve this, we introduce handleBadRequest that will call initThreadMaybe,
allowing the options to be correctly initialized.

Unfortunately, the tests were passing because exitcode was still 0, so the
results looked like success. Change those tests to do what they were supposed
to.

[alekseyshl]

Would moving SetAllocatorMayReturnNull call earler in the init() function help to resolve the problem?

[cryptoad]

In D37853#870912, @alekseyshl wrote:

Would moving SetAllocatorMayReturnNull call earler in the init() function help to resolve the problem?

Unfortunately, prior to this change, init might not be called at all. This would be the case for the following pseudo-example:

int main(void) {
  void *p = memalign(3, 3);
  return 0;
}

We would go directly to ScudoAllocator::FailureHandler::OnBadRequest without going through initThread -> initScudo -> initFlags.

[cryptoad]

I should add for completeness sake that to be fair, it's not a "real life" issue, as it would require the first heap operation ever called to be wrong.
But it's not a proper behavior.

[alekseyshl]

Ah, right, makes sense.

lib/scudo/scudo_allocator.cpp

struct ScudoAllocator {
   }
 
   uptr getStats(AllocatorStat StatType) {
     initThreadMaybe();
     uptr stats[AllocatorStatCount];
     BackendAllocator.getStats(stats);
     return stats[StatType];
   }
+
+  void *handleBadRequest() {
+    initThreadMaybe();
+    return FailureHandler::OnBadRequest();
+  }
 };
 
 static ScudoAllocator Instance(LINKER_INITIALIZED);
 
 static ScudoBackendAllocator &getBackendAllocator() {
   return Instance.BackendAllocator;
 }
 
void *scudoValloc(uptr Size) {
   return SetErrnoOnNull(
       Instance.allocate(Size, GetPageSizeCached(), FromMemalign));
 }
 
 void *scudoPvalloc(uptr Size) {
   uptr PageSize = GetPageSizeCached();
   if (UNLIKELY(CheckForPvallocOverflow(Size, PageSize))) {
     errno = errno_ENOMEM;
-    return ScudoAllocator::FailureHandler::OnBadRequest();
+    return Instance.handleBadRequest();
   }
   // pvalloc(0) should allocate one page.
   Size = Size ? RoundUpTo(Size, PageSize) : PageSize;
   return SetErrnoOnNull(Instance.allocate(Size, PageSize, FromMemalign));
 }
 
 void *scudoMemalign(uptr Alignment, uptr Size) {
   if (UNLIKELY(!IsPowerOfTwo(Alignment))) {
     errno = errno_EINVAL;
-    return ScudoAllocator::FailureHandler::OnBadRequest();
+    return Instance.handleBadRequest();
   }
   return SetErrnoOnNull(Instance.allocate(Size, Alignment, FromMemalign));
 }
 
 int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size) {
   if (UNLIKELY(!CheckPosixMemalignAlignment(Alignment))) {
-    ScudoAllocator::FailureHandler::OnBadRequest();
+    Instance.handleBadRequest();
     return errno_EINVAL;
   }
   void *Ptr = Instance.allocate(Size, Alignment, FromMemalign);
   if (UNLIKELY(!Ptr))
     return errno_ENOMEM;
   *MemPtr = Ptr;
   return 0;
 }
 
 void *scudoAlignedAlloc(uptr Alignment, uptr Size) {
   if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(Alignment, Size))) {
     errno = errno_EINVAL;
-    return ScudoAllocator::FailureHandler::OnBadRequest();
+    return Instance.handleBadRequest();
   }
   return SetErrnoOnNull(Instance.allocate(Size, Alignment, FromMalloc));
 }
 
 uptr scudoMallocUsableSize(void *Ptr) {
   return Instance.getUsableSize(Ptr);
 }
 

test/scudo/memalign.cpp

 // RUN: %clang_scudo %s -o %t
-// RUN: %run %t valid   2>&1
-// RUN: %run %t invalid 2>&1
+// RUN:                                               %run %t valid   2>&1
+// RUN:                                           not %run %t invalid 2>&1
+// RUN: SCUDO_OPTIONS=allocator_may_return_null=1     %run %t invalid 2>&1
 
 // Tests that the various aligned allocation functions work as intended. Also
 // tests for the condition where the alignment is not a power of 2.
 
 #include <assert.h>
 #include <errno.h>
 #include <malloc.h>
 #include <stdint.h>

test/scudo/valloc.cpp

 // RUN: %clang_scudo %s -o %t
-// RUN: %run %t valid   2>&1
-// RUN: %run %t invalid 2>&1
+// RUN:                                               %run %t valid   2>&1
+// RUN:                                           not %run %t invalid 2>&1
+// RUN: SCUDO_OPTIONS=allocator_may_return_null=1     %run %t invalid 2>&1
 
 // Tests that valloc and pvalloc work as intended.
 
 #include <assert.h>
 #include <errno.h>
 #include <malloc.h>
 #include <stdint.h>
 #include <string.h>