From dc19b5ace2414bc45fe23dd3a5c83180fadf4bce Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 18 Jan 2019 21:07:29 +0000 Subject: [PATCH 1/7] Test to check external pointer on deallocated objects This fails an assertion as it reads from a "head" and assumes it is a sizeclass. This leads to accessing memory out of bounds. --- src/test/func/memory/memory.cc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/func/memory/memory.cc b/src/test/func/memory/memory.cc index 69b8326..5ae4759 100644 --- a/src/test/func/memory/memory.cc +++ b/src/test/func/memory/memory.cc @@ -242,6 +242,30 @@ void test_external_pointer_large() } } +void test_external_pointer_dealloc_bug() +{ + auto* alloc = ThreadAlloc::get(); + constexpr size_t count = (SUPERSLAB_SIZE / SLAB_SIZE) * 2; + void* allocs[count]; + + for (size_t i = 0; i < count; i++) + { + allocs[i] = alloc->alloc(SLAB_SIZE / 2); + } + + for (size_t i = 1; i < count; i++) + { + alloc->dealloc(allocs[i]); + } + + for (size_t i = 0; i < count; i++) + { + Alloc::external_pointer(allocs[i]); + } + + alloc->dealloc(allocs[0]); +} + void test_alloc_16M() { auto* alloc = ThreadAlloc::get(); @@ -264,6 +288,7 @@ int main(int argc, char** argv) UNUSED(argv); #endif + test_external_pointer_dealloc_bug(); test_external_pointer_large(); test_alloc_dealloc_64k(); test_random_allocation(); From 94f8b886a0a3ae6a32e3bbbf88a0e970a273f3d6 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 18 Jan 2019 21:09:55 +0000 Subject: [PATCH 2/7] Remove union in metaslab The union in Metaslab provides no benefit in size, as the single byte it effectively saves will be removed due to padding. By removing the union, we get stronger properties over sizeclass, and remove an out-of-bounds access. --- src/mem/metaslab.h | 7 ++----- src/mem/superslab.h | 1 + 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index 1d24fae..c94de3e 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -57,11 +57,8 @@ namespace snmalloc // doubly linked node into that size class's free list. uint16_t link; - union - { - uint8_t sizeclass; - uint8_t next; - }; + uint8_t sizeclass; + uint8_t next; void add_use() { diff --git a/src/mem/superslab.h b/src/mem/superslab.h index 5a1c901..fc2e4e8 100644 --- a/src/mem/superslab.h +++ b/src/mem/superslab.h @@ -194,6 +194,7 @@ namespace snmalloc uint8_t index = (uint8_t)slab_to_index(slab); uint8_t n = head - index - 1; + meta[index].sizeclass = 0; meta[index].next = n; head = index; bool was_almost_full = is_almost_full(); From bb5027b45428ad04849dac422eb3ba69da5092e9 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 21 Jan 2019 17:44:09 +0000 Subject: [PATCH 3/7] Harden sizeclass table If a sizeclass in the metadata is corrupted, then this can be used to force an index beyond the end of these tables. This extends the tables to the next power of two, and uses a mask on the index, so they are always either a valid piece of data, or zero. --- src/ds/helpers.h | 19 +++++++++++++++++++ src/mem/sizeclasstable.h | 14 ++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index fc4461c..44d61f3 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -1,5 +1,6 @@ #pragma once +#include "bits.h" #include "flaglock.h" namespace snmalloc @@ -30,4 +31,22 @@ namespace snmalloc return obj; } }; + + template + class ModArray + { + static constexpr size_t rlength = bits::next_pow2_const(length); + T array[rlength]; + + public: + constexpr const T &operator[] (const size_t i) const + { + return array[i & (rlength - 1)]; + } + + constexpr T &operator[] (const size_t i) + { + return array[i & (rlength - 1)]; + } + }; } diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h index 099b465..21aa913 100644 --- a/src/mem/sizeclasstable.h +++ b/src/mem/sizeclasstable.h @@ -1,16 +1,17 @@ #pragma once #include "superslab.h" +#include "../ds/helpers.h" namespace snmalloc { struct SizeClassTable { - size_t size[NUM_SIZECLASSES]; - uint16_t bump_ptr_start[NUM_SMALL_CLASSES]; - uint16_t short_bump_ptr_start[NUM_SMALL_CLASSES]; - uint16_t count_per_slab[NUM_SMALL_CLASSES]; - uint16_t medium_slab_slots[NUM_MEDIUM_CLASSES]; + ModArray size; + ModArray bump_ptr_start; + ModArray short_bump_ptr_start; + ModArray count_per_slab; + ModArray medium_slab_slots; constexpr SizeClassTable() : size(), @@ -66,6 +67,7 @@ namespace snmalloc constexpr static inline uint16_t medium_slab_free(uint8_t sizeclass) { - return sizeclass_metadata.medium_slab_slots[sizeclass - NUM_SMALL_CLASSES]; + return sizeclass_metadata.medium_slab_slots + [(sizeclass - NUM_SMALL_CLASSES)]; } } From 18dd15c2c0397463ead15f14a5cb9809d0aca46d Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 21 Jan 2019 17:53:30 +0000 Subject: [PATCH 4/7] Minor restructuring to move static_assert --- src/mem/allocconfig.h | 2 ++ src/mem/sizeclass.h | 10 +--------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/mem/allocconfig.h b/src/mem/allocconfig.h index fd7fb53..01bc1d0 100644 --- a/src/mem/allocconfig.h +++ b/src/mem/allocconfig.h @@ -110,6 +110,8 @@ namespace snmalloc static constexpr size_t SUPERSLAB_BITS = SLAB_BITS + SLAB_COUNT_BITS; static constexpr size_t RESERVE_SIZE = SUPERSLAB_SIZE * RESERVE_MULTIPLE; + static_assert((1ULL << SUPERSLAB_BITS) == SUPERSLAB_SIZE, "Sanity check"); + // Number of slots for remote deallocation. static constexpr size_t REMOTE_SLOT_BITS = 6; static constexpr size_t REMOTE_SLOTS = 1 << REMOTE_SLOT_BITS; diff --git a/src/mem/sizeclass.h b/src/mem/sizeclass.h index 1561df2..36136f8 100644 --- a/src/mem/sizeclass.h +++ b/src/mem/sizeclass.h @@ -35,7 +35,7 @@ namespace snmalloc size_to_sizeclass_const((size_t)1 << SLAB_BITS) + 1; static constexpr size_t NUM_SIZECLASSES = - size_to_sizeclass_const((size_t)1 << SUPERSLAB_BITS); + size_to_sizeclass_const(SUPERSLAB_SIZE); // Medium classes range from (SLAB, SUPERSLAB), i.e. non-inclusive. static constexpr size_t NUM_MEDIUM_CLASSES = @@ -45,14 +45,6 @@ namespace snmalloc static constexpr size_t NUM_LARGE_CLASSES = bits::ADDRESS_BITS - SUPERSLAB_BITS; - template - constexpr void check_same() - { - static_assert(X == Y, "Values must be the same"); - } - - static_assert(size_to_sizeclass_const(SUPERSLAB_SIZE) == NUM_SIZECLASSES); - inline static size_t round_by_sizeclass(size_t rsize, size_t offset) { // check_same(); From 1d5df3b7a9df0d1aea4d693ff82d8f994ec61c03 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 21 Jan 2019 17:53:52 +0000 Subject: [PATCH 5/7] Build both 1MiB and 16MiB tests --- CMakeLists.txt | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e3842ee..68114d9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,20 +64,24 @@ enable_testing() set(TESTDIR ${CMAKE_CURRENT_SOURCE_DIR}/src/test) subdirlist(TEST_CATEGORIES ${TESTDIR}) -foreach(TEST_CATEGORY ${TEST_CATEGORIES}) - subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) - foreach(TEST ${TESTS}) - unset(SRC) - aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) - set(TESTNAME "${TEST_CATEGORY}-${TEST}") - add_executable(${TESTNAME} ${SRC} src/override/new.cc) - target_include_directories(${TESTNAME} PRIVATE src) - linklibs(${TESTNAME}) - add_test(${TESTNAME} ${TESTNAME}) +foreach(SUPER_SLAB_SIZE 1;16) + foreach(TEST_CATEGORY ${TEST_CATEGORIES}) + subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) + foreach(TEST ${TESTS}) + unset(SRC) + aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) + set(TESTNAME "${TEST_CATEGORY}-${TEST}-${SUPER_SLAB_SIZE}") + add_executable(${TESTNAME} ${SRC} src/override/new.cc) + if (${SUPER_SLAB_SIZE} EQUAL 1) + target_compile_definitions(${TESTNAME} PRIVATE IS_ADDRESS_SPACE_CONSTRAINED) + endif() + target_include_directories(${TESTNAME} PRIVATE src) + linklibs(${TESTNAME}) + add_test(${TESTNAME} ${TESTNAME}) + endforeach() endforeach() endforeach() - # The clang-format tool is installed under a variety of different names. Try # to find a sensible one. Only look for 6.0 and 7.0 versions explicitly - we # don't know whether our clang-format file will work with newer versions of the From cb70aa3f7f142205d835e2a9455ec5f2a66c5b04 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 22 Jan 2019 10:03:29 +0000 Subject: [PATCH 6/7] Restricted another array to bounds. --- src/mem/largealloc.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index 4745471..6c26ab7 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -2,6 +2,7 @@ #include "../ds/flaglock.h" #include "../ds/mpmcstack.h" +#include "../ds/helpers.h" #include "../pal/pal.h" #include "allocstats.h" #include "baseslab.h" @@ -62,7 +63,7 @@ namespace snmalloc /** * Stack of large allocations that have been returned for reuse. */ - MPMCStack large_stack[NUM_LARGE_CLASSES]; + ModArray> large_stack; /** * Primitive allocator for structure that are required before From 745a2a53d166cf77ed521aff9de1b527e3dd7f2e Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 22 Jan 2019 17:18:54 +0000 Subject: [PATCH 7/7] Clang format --- src/ds/helpers.h | 18 +++++++++--------- src/mem/largealloc.h | 2 +- src/mem/sizeclasstable.h | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index 44d61f3..57a7fef 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -32,21 +32,21 @@ namespace snmalloc } }; - template + template class ModArray { static constexpr size_t rlength = bits::next_pow2_const(length); T array[rlength]; - - public: - constexpr const T &operator[] (const size_t i) const - { - return array[i & (rlength - 1)]; - } - constexpr T &operator[] (const size_t i) + public: + constexpr const T& operator[](const size_t i) const { return array[i & (rlength - 1)]; - } + } + + constexpr T& operator[](const size_t i) + { + return array[i & (rlength - 1)]; + } }; } diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index 6c26ab7..b161f51 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -1,8 +1,8 @@ #pragma once #include "../ds/flaglock.h" -#include "../ds/mpmcstack.h" #include "../ds/helpers.h" +#include "../ds/mpmcstack.h" #include "../pal/pal.h" #include "allocstats.h" #include "baseslab.h" diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h index 21aa913..80009c5 100644 --- a/src/mem/sizeclasstable.h +++ b/src/mem/sizeclasstable.h @@ -1,7 +1,7 @@ #pragma once -#include "superslab.h" #include "../ds/helpers.h" +#include "superslab.h" namespace snmalloc { @@ -67,7 +67,7 @@ namespace snmalloc constexpr static inline uint16_t medium_slab_free(uint8_t sizeclass) { - return sizeclass_metadata.medium_slab_slots - [(sizeclass - NUM_SMALL_CLASSES)]; + return sizeclass_metadata + .medium_slab_slots[(sizeclass - NUM_SMALL_CLASSES)]; } }