[sanitizer] Fix a crash in SizeClassAllocator32 with an out-of-range pointer

[kubamracek]

Summary

This only happens on a 64-bit platform that uses SizeClassAllocator32 (e.g. ASan on AArch64). When querying a large invalid pointer about its size, e.g. with:

__sanitizer_get_allocated_size(0xdeadbeefdeadbeef);

...an assertion will fail:

AddressSanitizer CHECK failed: .../sanitizer_allocator.h "((res)) < ((kNumPossibleRegions))"

This patch changes PointerIsMine to return false if the pointer is outside of [kSpaceBeg, kSpaceBeg + kSpaceSize).

[dvyukov]

lib/sanitizer_common/sanitizer_allocator.h

753 ↗

(On Diff #41217)

The condition in GetSizeClass->ComputeRegionId is different. It effectively checks that mem < kSpaceSize.
At least ComputeRegionId assumes that kSpaceBeg==0. We seem to be missing some tests.
Kostya?

[kubamracek]

lib/sanitizer_common/sanitizer_allocator.h

753 ↗

(On Diff #41217)

How should this be fixed? All uses of SizeClassAllocator32 currently use "0" as kSpaceBeg, so we could just assume that as a requirement.

[dvyukov]

I will ask Kostya.

[kcc]

Where is "__sanitizer_get_allocated_size(0xdeadbeefdeadbeef);" called?
I'd say this is a misuse of the function, you should not pass a pointer outside of the AS there.

[dvyukov]

Kostya, I think we should remove kSpaceBeg and replace it with zero. What do you think?

[kcc]

In D15008#304630, @dvyukov wrote:

Kostya, I think we should remove kSpaceBeg and replace it with zero. What do you think?

You mean, only SizeClassAllocator32::kSpaceBeg, right?
Why?

Theoretically, we may gain something by making kSpaceBeg non-zero.
Before we actually do it, SizeClassAllocator32::kSpaceBeg is kind of useless, but it also doesn't harm much.

[dvyukov]

It is broken. We never used it. We don't have tests for it.
See previous comments.

[kcc]

No strong opinion here. If you think we should remove it -- ok, let's do it.
We can reinstate it back if needed.

[kubamracek]

Ping. Adding a test case.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator.h

NOINLINE void DeallocateBatch(AllocatorStats *stat, uptr class_id, Batch *b) {
     CHECK_LT(class_id, kNumClasses);
     SizeClassInfo *sci = GetSizeClassInfo(class_id);
     SpinMutexLock l(&sci->mutex);
     CHECK_GT(b->count, 0);
     sci->free_list.push_front(b);
   }
 
   bool PointerIsMine(const void *p) {
+    uptr mem = reinterpret_cast<uptr>(p);
+    if (mem < kSpaceBeg || mem >= kSpaceBeg + kSpaceSize)
+      return false;
     return GetSizeClass(p) != 0;
   }
 
   uptr GetSizeClass(const void *p) {
     return possible_regions[ComputeRegionId(reinterpret_cast<uptr>(p))];
   }
 
   void *GetBlockBegin(const void *p) {

compiler-rt/trunk/test/asan/TestCases/Darwin/malloc_size_crash.mm

+// RUN: %clang_asan %s -o %t -framework Foundation
+// RUN: %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+#include <malloc/malloc.h>
+
+int main(int argc, char *argv[]) {
+  id obj = @0;
+  fprintf(stderr, "obj = %p\n", obj);
+  size_t size = malloc_size(obj);
+  fprintf(stderr, "size = 0x%zx\n", size);
+  fprintf(stderr, "Done.\n");
+  // CHECK: Done.
+  return 0;
+}