bionic_allocator: more detailed and consistent error reporting.

I only came to improve the signature mismatch error, but I was then annoyed by the copy & paste of the other checks.

get_chunk_size() seems to be deliberately avoiding any checks, though I think that might be a bug, and there should be a get_chunk_size() that _does_ check for most callers, and a get_chunk_size_unchecked() for the <sys/thread_properties.h> stuff that seems to want to only be "best effort" (but does still have _some_ possibility of aborting, in addition to the possibility of segfaulting).

Also a bit of "include what you use" after cider complained about all the unused includes in bionic_allocator.h.

Bug: https://issuetracker.google.com/341850283
Change-Id: I278b495601353733af516a2d60ed10feb9cef36b
This commit is contained in:
Elliott Hughes 2024-05-29 22:25:37 +00:00
parent d475ee45aa
commit 8f653f8ad9
5 changed files with 22 additions and 24 deletions

View file

@ -299,7 +299,7 @@ inline void* BionicAllocator::alloc_impl(size_t align, size_t size) {
log2_size = kSmallObjectMinSizeLog2; log2_size = kSmallObjectMinSizeLog2;
} }
return get_small_object_allocator(log2_size)->alloc(); return get_small_object_allocator_unchecked(log2_size)->alloc();
} }
void* BionicAllocator::alloc(size_t size) { void* BionicAllocator::alloc(size_t size) {
@ -330,9 +330,10 @@ inline page_info* BionicAllocator::get_page_info_unchecked(void* ptr) {
inline page_info* BionicAllocator::get_page_info(void* ptr) { inline page_info* BionicAllocator::get_page_info(void* ptr) {
page_info* info = get_page_info_unchecked(ptr); page_info* info = get_page_info_unchecked(ptr);
if (memcmp(info->signature, kSignature, sizeof(kSignature)) != 0) { if (memcmp(info->signature, kSignature, sizeof(kSignature)) != 0) {
async_safe_fatal("invalid pointer %p (page signature mismatch)", ptr); async_safe_fatal("invalid pointer %p (page signature %04x instead of %04x)", ptr,
*reinterpret_cast<const unsigned*>(info->signature),
*reinterpret_cast<const unsigned*>(kSignature));
} }
return info; return info;
} }
@ -353,12 +354,7 @@ void* BionicAllocator::realloc(void* ptr, size_t size) {
if (info->type == kLargeObject) { if (info->type == kLargeObject) {
old_size = info->allocated_size - (static_cast<char*>(ptr) - reinterpret_cast<char*>(info)); old_size = info->allocated_size - (static_cast<char*>(ptr) - reinterpret_cast<char*>(info));
} else { } else {
BionicSmallObjectAllocator* allocator = get_small_object_allocator(info->type); old_size = get_small_object_allocator(info, ptr)->get_block_size();
if (allocator != info->allocator_addr) {
async_safe_fatal("invalid pointer %p (page signature mismatch)", ptr);
}
old_size = allocator->get_block_size();
} }
if (old_size < size) { if (old_size < size) {
@ -377,16 +373,10 @@ void BionicAllocator::free(void* ptr) {
} }
page_info* info = get_page_info(ptr); page_info* info = get_page_info(ptr);
if (info->type == kLargeObject) { if (info->type == kLargeObject) {
munmap(info, info->allocated_size); munmap(info, info->allocated_size);
} else { } else {
BionicSmallObjectAllocator* allocator = get_small_object_allocator(info->type); get_small_object_allocator(info, ptr)->free(ptr);
if (allocator != info->allocator_addr) {
async_safe_fatal("invalid pointer %p (invalid allocator address for the page)", ptr);
}
allocator->free(ptr);
} }
} }
@ -402,7 +392,7 @@ size_t BionicAllocator::get_chunk_size(void* ptr) {
return info->allocated_size - (static_cast<char*>(ptr) - reinterpret_cast<char*>(info)); return info->allocated_size - (static_cast<char*>(ptr) - reinterpret_cast<char*>(info));
} }
BionicSmallObjectAllocator* allocator = get_small_object_allocator(info->type); BionicSmallObjectAllocator* allocator = get_small_object_allocator_unchecked(info->type);
if (allocator != info->allocator_addr) { if (allocator != info->allocator_addr) {
// Invalid pointer. // Invalid pointer.
return 0; return 0;
@ -410,7 +400,7 @@ size_t BionicAllocator::get_chunk_size(void* ptr) {
return allocator->get_block_size(); return allocator->get_block_size();
} }
BionicSmallObjectAllocator* BionicAllocator::get_small_object_allocator(uint32_t type) { BionicSmallObjectAllocator* BionicAllocator::get_small_object_allocator_unchecked(uint32_t type) {
if (type < kSmallObjectMinSizeLog2 || type > kSmallObjectMaxSizeLog2) { if (type < kSmallObjectMinSizeLog2 || type > kSmallObjectMaxSizeLog2) {
async_safe_fatal("invalid type: %u", type); async_safe_fatal("invalid type: %u", type);
} }
@ -418,3 +408,11 @@ BionicSmallObjectAllocator* BionicAllocator::get_small_object_allocator(uint32_t
initialize_allocators(); initialize_allocators();
return &allocators_[type - kSmallObjectMinSizeLog2]; return &allocators_[type - kSmallObjectMinSizeLog2];
} }
BionicSmallObjectAllocator* BionicAllocator::get_small_object_allocator(page_info* pi, void* ptr) {
BionicSmallObjectAllocator* result = get_small_object_allocator_unchecked(pi->type);
if (result != pi->allocator_addr) {
async_safe_fatal("invalid pointer %p (invalid allocator address for the page)", ptr);
}
return result;
}

View file

@ -34,6 +34,7 @@
#include <platform/bionic/malloc.h> #include <platform/bionic/malloc.h>
#include <sanitizer/hwasan_interface.h> #include <sanitizer/hwasan_interface.h>
#include <sys/auxv.h> #include <sys/auxv.h>
#include <sys/prctl.h>
extern "C" void scudo_malloc_disable_memory_tagging(); extern "C" void scudo_malloc_disable_memory_tagging();
extern "C" void scudo_malloc_set_track_allocation_stacks(int); extern "C" void scudo_malloc_set_track_allocation_stacks(int);

View file

@ -31,6 +31,7 @@
#include <stdatomic.h> #include <stdatomic.h>
#include <stdint.h> #include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <unistd.h>
#include <private/bionic_malloc_dispatch.h> #include <private/bionic_malloc_dispatch.h>

View file

@ -28,13 +28,9 @@
#pragma once #pragma once
#include <errno.h>
#include <stdlib.h>
#include <sys/cdefs.h> #include <sys/cdefs.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <stddef.h> #include <stddef.h>
#include <unistd.h> #include <stdint.h>
const uint32_t kSmallObjectMaxSizeLog2 = 10; const uint32_t kSmallObjectMaxSizeLog2 = 10;
const uint32_t kSmallObjectMinSizeLog2 = 4; const uint32_t kSmallObjectMinSizeLog2 = 4;
@ -120,7 +116,8 @@ class BionicAllocator {
inline void* alloc_impl(size_t align, size_t size); inline void* alloc_impl(size_t align, size_t size);
inline page_info* get_page_info_unchecked(void* ptr); inline page_info* get_page_info_unchecked(void* ptr);
inline page_info* get_page_info(void* ptr); inline page_info* get_page_info(void* ptr);
BionicSmallObjectAllocator* get_small_object_allocator(uint32_t type); BionicSmallObjectAllocator* get_small_object_allocator_unchecked(uint32_t type);
BionicSmallObjectAllocator* get_small_object_allocator(page_info* pi, void* ptr);
void initialize_allocators(); void initialize_allocators();
BionicSmallObjectAllocator* allocators_; BionicSmallObjectAllocator* allocators_;

View file

@ -31,6 +31,7 @@
#include <link.h> #include <link.h>
#include <stdlib.h> #include <stdlib.h>
#include <sys/auxv.h> #include <sys/auxv.h>
#include <sys/prctl.h>
#include "linker.h" #include "linker.h"
#include "linker_auxv.h" #include "linker_auxv.h"