Convert alloc paths to capptr::Alloc<void>

The use of void* had let an overzealous unsafe_ptr() leak a pointer with address
space control to the client (in LocalAllocator::alloc_not_small, specifically).
Correct this to call capptr_chunk_is_alloc() (to capture our intent) and
capptr_to_user_address_control() (to do the bounding) and defer the conversion
to void* until the very periphery of the allocator, using capptr_reveal()
(again, to capture intent).
This commit is contained in:
Nathaniel Wesley Filardo
2021-10-20 02:49:32 +01:00
committed by Nathaniel Wesley Filardo
parent 31b8d99ca6
commit 5bb556cb68
3 changed files with 19 additions and 20 deletions

View File

@@ -176,7 +176,7 @@ namespace snmalloc
* - Allocating stub in the message queue * - Allocating stub in the message queue
* Note this is not performance critical as very infrequently called. * Note this is not performance critical as very infrequently called.
*/ */
void* small_alloc_one(size_t size) capptr::Alloc<void> small_alloc_one(size_t size)
{ {
SNMALLOC_ASSERT(attached_cache != nullptr); SNMALLOC_ASSERT(attached_cache != nullptr);
auto domesticate = auto domesticate =
@@ -285,7 +285,8 @@ namespace snmalloc
[local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA { [local_state](freelist::QueuePtr p) SNMALLOC_FAST_PATH_LAMBDA {
return capptr_domesticate<SharedStateHandle>(local_state, p); return capptr_domesticate<SharedStateHandle>(local_state, p);
}; };
void* p = finish_alloc_no_zero(fl.take(key, domesticate), sizeclass); capptr::Alloc<void> p =
finish_alloc_no_zero(fl.take(key, domesticate), sizeclass);
#ifdef SNMALLOC_CHECK_CLIENT #ifdef SNMALLOC_CHECK_CLIENT
// Check free list is well-formed on platforms with // Check free list is well-formed on platforms with
@@ -321,11 +322,11 @@ namespace snmalloc
auto start_of_slab = pointer_align_down<void>( auto start_of_slab = pointer_align_down<void>(
p, snmalloc::sizeclass_to_slab_size(sizeclass)); p, snmalloc::sizeclass_to_slab_size(sizeclass));
// TODO Add bounds correctly here // TODO Add bounds correctly here
chunk_record->chunk = capptr::Chunk<void>(start_of_slab); chunk_record->chunk = capptr::Chunk<void>(start_of_slab.unsafe_ptr());
#ifdef SNMALLOC_TRACING #ifdef SNMALLOC_TRACING
std::cout << "Slab " << start_of_slab << " is unused, Object sizeclass " std::cout << "Slab " << start_of_slab.unsafe_ptr()
<< sizeclass << std::endl; << " is unused, Object sizeclass " << sizeclass << std::endl;
#endif #endif
return chunk_record; return chunk_record;
} }
@@ -654,7 +655,7 @@ namespace snmalloc
} }
template<ZeroMem zero_mem> template<ZeroMem zero_mem>
SNMALLOC_SLOW_PATH void* SNMALLOC_SLOW_PATH capptr::Alloc<void>
small_alloc(sizeclass_t sizeclass, freelist::Iter<>& fast_free_list) small_alloc(sizeclass_t sizeclass, freelist::Iter<>& fast_free_list)
{ {
size_t rsize = sizeclass_to_size(sizeclass); size_t rsize = sizeclass_to_size(sizeclass);
@@ -716,7 +717,7 @@ namespace snmalloc
} }
template<ZeroMem zero_mem> template<ZeroMem zero_mem>
SNMALLOC_SLOW_PATH void* small_alloc_slow( SNMALLOC_SLOW_PATH capptr::Alloc<void> small_alloc_slow(
sizeclass_t sizeclass, freelist::Iter<>& fast_free_list, size_t rsize) sizeclass_t sizeclass, freelist::Iter<>& fast_free_list, size_t rsize)
{ {
// No existing free list get a new slab. // No existing free list get a new slab.

View File

@@ -167,7 +167,7 @@ namespace snmalloc
* passed to the core allocator. * passed to the core allocator.
*/ */
template<ZeroMem zero_mem> template<ZeroMem zero_mem>
SNMALLOC_SLOW_PATH void* alloc_not_small(size_t size) SNMALLOC_SLOW_PATH capptr::Alloc<void> alloc_not_small(size_t size)
{ {
if (size == 0) if (size == 0)
{ {
@@ -203,12 +203,12 @@ namespace snmalloc
chunk.unsafe_ptr(), size); chunk.unsafe_ptr(), size);
} }
return chunk.unsafe_ptr(); return capptr_chunk_is_alloc(capptr_to_user_address_control(chunk));
}); });
} }
template<ZeroMem zero_mem> template<ZeroMem zero_mem>
SNMALLOC_FAST_PATH void* small_alloc(size_t size) SNMALLOC_FAST_PATH capptr::Alloc<void> small_alloc(size_t size)
{ {
// SNMALLOC_ASSUME(size <= sizeclass_to_size(NUM_SIZECLASSES)); // SNMALLOC_ASSUME(size <= sizeclass_to_size(NUM_SIZECLASSES));
auto domesticate = [this](freelist::QueuePtr p) auto domesticate = [this](freelist::QueuePtr p)
@@ -430,11 +430,10 @@ namespace snmalloc
{ {
// Small allocations are more likely. Improve // Small allocations are more likely. Improve
// branch prediction by placing this case first. // branch prediction by placing this case first.
return small_alloc<zero_mem>(size); return capptr_reveal(small_alloc<zero_mem>(size));
} }
// TODO capptr_reveal? return capptr_reveal(alloc_not_small<zero_mem>(size));
return alloc_not_small<zero_mem>(size);
#endif #endif
} }

View File

@@ -12,25 +12,24 @@ namespace snmalloc
{ {
using Stats = AllocStats<NUM_SIZECLASSES, NUM_LARGE_CLASSES>; using Stats = AllocStats<NUM_SIZECLASSES, NUM_LARGE_CLASSES>;
inline static SNMALLOC_FAST_PATH void* inline static SNMALLOC_FAST_PATH capptr::Alloc<void>
finish_alloc_no_zero(freelist::HeadPtr p, sizeclass_t sizeclass) finish_alloc_no_zero(freelist::HeadPtr p, sizeclass_t sizeclass)
{ {
SNMALLOC_ASSERT(Metaslab::is_start_of_object(sizeclass, address_cast(p))); SNMALLOC_ASSERT(Metaslab::is_start_of_object(sizeclass, address_cast(p)));
UNUSED(sizeclass); UNUSED(sizeclass);
auto r = capptr_reveal(p.as_void()); return p.as_void();
return r;
} }
template<ZeroMem zero_mem, typename SharedStateHandle> template<ZeroMem zero_mem, typename SharedStateHandle>
inline static SNMALLOC_FAST_PATH void* inline static SNMALLOC_FAST_PATH capptr::Alloc<void>
finish_alloc(freelist::HeadPtr p, sizeclass_t sizeclass) finish_alloc(freelist::HeadPtr p, sizeclass_t sizeclass)
{ {
auto r = finish_alloc_no_zero(p, sizeclass); auto r = finish_alloc_no_zero(p, sizeclass);
if constexpr (zero_mem == YesZero) if constexpr (zero_mem == YesZero)
SharedStateHandle::Pal::zero(r, sizeclass_to_size(sizeclass)); SharedStateHandle::Pal::zero(
r.unsafe_ptr(), sizeclass_to_size(sizeclass));
// TODO: Should this be zeroing the free Object state, in the non-zeroing // TODO: Should this be zeroing the free Object state, in the non-zeroing
// case? // case?
@@ -105,7 +104,7 @@ namespace snmalloc
typename SharedStateHandle, typename SharedStateHandle,
typename Slowpath, typename Slowpath,
typename Domesticator> typename Domesticator>
SNMALLOC_FAST_PATH void* SNMALLOC_FAST_PATH capptr::Alloc<void>
alloc(Domesticator domesticate, size_t size, Slowpath slowpath) alloc(Domesticator domesticate, size_t size, Slowpath slowpath)
{ {
auto& key = entropy.get_free_list_key(); auto& key = entropy.get_free_list_key();