From 79ad6630d354233e6b2eefb7901c0d39a0476a12 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Fri, 15 May 2020 19:27:32 +0000 Subject: [PATCH 1/2] alloc: eliminate external_pointer/address distinction Just always work with pointers using the functions defined in ds/address.h. This more obviously preserves provenance through the chain of reasoning. Note that there is still risk of malloc() being used as an amplification oracle on CHERI, but there's no additional risk from this change. Rename the external_address into external_pointer. --- src/mem/alloc.h | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index 82005df..b008e61 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -394,7 +394,7 @@ namespace snmalloc } template - static address_t external_address(void* p) + static void* external_pointer(void* p) { #ifdef USE_MALLOC error("Unsupported"); @@ -423,12 +423,13 @@ namespace snmalloc return external_pointer(p, sc, slab_end); } - auto ss = address_cast(super); + auto ss = super; while (size > 64) { // This is a large alloc redirect. - ss = ss - (1ULL << (size - 64)); + ss = pointer_offset_signed( + ss, -(static_cast(1) << (size - 64))); size = ChunkMap::get(ss); } @@ -436,28 +437,22 @@ namespace snmalloc { if constexpr ((location == End) || (location == OnePastEnd)) // We don't know the End, so return MAX_PTR - return UINTPTR_MAX; + return pointer_offset(nullptr, UINTPTR_MAX); else // We don't know the Start, so return MIN_PTR - return 0; + return nullptr; } // This is a large alloc, mask off to the slab size. if constexpr (location == Start) return ss; else if constexpr (location == End) - return (ss + (1ULL << size) - 1ULL); + return pointer_offset(ss, (1ULL << size) - 1ULL); else - return (ss + (1ULL << size)); + return pointer_offset(ss, 1ULL << size); #endif } - template - static void* external_pointer(void* p) - { - return pointer_cast(external_address(p)); - } - static size_t alloc_size(const void* p) { // This must be called on an external pointer. @@ -824,24 +819,24 @@ namespace snmalloc } template - static uintptr_t + static void* external_pointer(void* p, sizeclass_t sizeclass, void* end_point) { size_t rsize = sizeclass_to_size(sizeclass); void* end_point_correction = location == End ? - (static_cast(end_point) - 1) : - (location == OnePastEnd ? end_point : - (static_cast(end_point) - rsize)); + pointer_offset_signed(end_point, -1) : + (location == OnePastEnd ? + end_point : + pointer_offset_signed(end_point, -static_cast(rsize))); - ptrdiff_t offset_from_end = - (static_cast(end_point) - 1) - static_cast(p); + size_t offset_from_end = + pointer_diff(p, pointer_offset_signed(end_point, -1)); - size_t end_to_end = - round_by_sizeclass(rsize, static_cast(offset_from_end)); + size_t end_to_end = round_by_sizeclass(rsize, offset_from_end); - return address_cast( - static_cast(end_point_correction) - end_to_end); + return pointer_offset_signed( + end_point_correction, -static_cast(end_to_end)); } void init_message_queue() From 3f32f37e60bf7cef75c0d650e0cab1f53df26ea2 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Fri, 15 May 2020 19:32:09 +0000 Subject: [PATCH 2/2] Eliminate pointer_cast() Since we anticipate address_t not carrying provenance on CHERI, but rather being vaddr_t there, it doesn't make sense to offer conversion back to a provenance-carrying pointer. Thankfully, there is not much to be done here: the uses were few and could be replaced with the vocabulary of other pointer operations in ds/address.h --- src/ds/address.h | 36 +++++++++++++++++++++++------------- src/mem/largealloc.h | 26 ++++++++++++-------------- src/mem/superslab.h | 4 ++-- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/ds/address.h b/src/ds/address.h index a92ef36..720f1af 100644 --- a/src/ds/address.h +++ b/src/ds/address.h @@ -42,16 +42,6 @@ namespace snmalloc return reinterpret_cast(ptr); } - /** - * Cast from an address back to a pointer of the specified type. All uses of - * this will eventually need auditing for CHERI compatibility. - */ - template - inline T* pointer_cast(address_t address) - { - return reinterpret_cast(address); - } - /** * Test if a pointer is aligned to a given size, which must be a power of * two. @@ -81,7 +71,8 @@ namespace snmalloc #if __has_builtin(__builtin_align_down) return static_cast(__builtin_align_down(p, alignment)); #else - return pointer_cast(bits::align_down(address_cast(p), alignment)); + return reinterpret_cast( + bits::align_down(reinterpret_cast(p), alignment)); #endif } } @@ -102,11 +93,29 @@ namespace snmalloc #if __has_builtin(__builtin_align_up) return static_cast(__builtin_align_up(p, alignment)); #else - return pointer_cast(bits::align_up(address_cast(p), alignment)); + return reinterpret_cast( + bits::align_up(reinterpret_cast(p), alignment)); #endif } } + /** + * Align a pointer down to a dynamically specified granularity, which must be + * a power of two. + */ + template + SNMALLOC_FAST_PATH T* pointer_align_down(void* p, size_t alignment) + { + SNMALLOC_ASSERT(alignment > 0); + SNMALLOC_ASSERT(bits::next_pow2(alignment) == alignment); +#if __has_builtin(__builtin_align_down) + return static_cast(__builtin_align_down(p, alignment)); +#else + return reinterpret_cast( + bits::align_down(reinterpret_cast(p), alignment)); +#endif + } + /** * Align a pointer up to a dynamically specified granularity, which must * be a power of two. @@ -119,7 +128,8 @@ namespace snmalloc #if __has_builtin(__builtin_align_up) return static_cast(__builtin_align_up(p, alignment)); #else - return pointer_cast(bits::align_up(address_cast(p), alignment)); + return reinterpret_cast( + bits::align_up(reinterpret_cast(p), alignment)); #endif } diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index cb347c0..c4683ab 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -183,10 +183,9 @@ namespace snmalloc lazy_decommit_guard.clear(); } - void push_space(address_t start, size_t large_class) + void push_space(void* p, size_t large_class) { // All fresh pages so can use "NoZero" - void* p = pointer_cast(start); if (large_class > 0) PAL::template notify_using(p, OS_PAGE_SIZE); else @@ -284,24 +283,24 @@ namespace snmalloc if (p == nullptr) return nullptr; - address_t p0 = address_cast(p); - address_t start = bits::align_up(p0, align); - address_t p1 = p0 + request; - address_t end = start + size; + void* start = pointer_align_up(p, align); + void* p1 = pointer_offset(p, request); + void* end = pointer_offset(start, size); - for (; end < bits::align_down(p1, align); end += size) + for (; end < pointer_align_down(p1, align); + end = pointer_offset(end, size)) { push_space(end, large_class); } // Put offcuts before alignment into the large stack - address_t offcut_end = start; - address_t offcut_start; + void* offcut_end = start; + void* offcut_start; for (size_t i = large_class; i > 0;) { i--; size_t offcut_align = bits::one_at_bit(SUPERSLAB_BITS) << i; - offcut_start = bits::align_up(p0, offcut_align); + offcut_start = pointer_align_up(p, offcut_align); if (offcut_start != offcut_end) { push_space(offcut_start, i); @@ -315,7 +314,7 @@ namespace snmalloc { i--; auto offcut_align = bits::one_at_bit(SUPERSLAB_BITS) << i; - offcut_end = bits::align_down(p1, offcut_align); + offcut_end = pointer_align_down(p1, offcut_align); if (offcut_start != offcut_end) { push_space(offcut_start, i); @@ -323,11 +322,10 @@ namespace snmalloc } } - void* result = pointer_cast(start); if (committed) - PAL::template notify_using(result, size); + PAL::template notify_using(start, size); - return result; + return start; } } }; diff --git a/src/mem/superslab.h b/src/mem/superslab.h index c528e65..50e6e16 100644 --- a/src/mem/superslab.h +++ b/src/mem/superslab.h @@ -181,8 +181,8 @@ namespace snmalloc Slab* alloc_slab(sizeclass_t sizeclass) { uint8_t h = head; - Slab* slab = pointer_cast( - address_cast(this) + (static_cast(h) << SLAB_BITS)); + Slab* slab = pointer_offset( + reinterpret_cast(this), (static_cast(h) << SLAB_BITS)); uint8_t n = meta[h].next;