From 0fc5bbde7596a2fc6dc03fe00f46dc5dfcf8b37f Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Thu, 2 Feb 2023 16:53:58 +0000 Subject: [PATCH] Factor checks under separate feature flags. All the checks and mitigations have been placed under feature flags. These can be controlled by defining SNMALLOC_CHECK_CLIENT_MITIGATIONS This can take a term that represents the mitigations that should be enabled. E.g. -DSNMALLOC_CHECK_CLIENT_MITIGATIONS=nochecks+random_pagemap The CMake uses this to build numerous versions of the LD_PRELOAD library and tests to allow individual features to be benchmarked. Co-authored-by: Nathaniel Wesley Filardo --- .github/workflows/main.yml | 18 +- CMakeLists.txt | 214 +++++++++------ docs/PORTING.md | 2 +- src/snmalloc/backend/globalconfig.h | 35 +-- src/snmalloc/ds/allocconfig.h | 7 +- src/snmalloc/ds/pagemap.h | 4 + src/snmalloc/ds_core/defines.h | 49 +--- src/snmalloc/ds_core/ds_core.h | 1 + src/snmalloc/ds_core/helpers.h | 2 +- src/snmalloc/ds_core/mitigations.h | 263 +++++++++++++++++++ src/snmalloc/mem/corealloc.h | 213 ++++++++------- src/snmalloc/mem/freelist.h | 234 ++++++++++------- src/snmalloc/mem/localalloc.h | 61 +++-- src/snmalloc/mem/metadata.h | 16 +- src/snmalloc/mem/remotecache.h | 1 + src/snmalloc/mem/sizeclasstable.h | 7 +- src/snmalloc/pal/pal_apple.h | 6 +- src/snmalloc/pal/pal_bsd.h | 2 +- src/snmalloc/pal/pal_bsd_aligned.h | 5 +- src/snmalloc/pal/pal_consts.h | 21 -- src/snmalloc/pal/pal_freebsd.h | 2 +- src/snmalloc/pal/pal_linux.h | 2 +- src/snmalloc/pal/pal_posix.h | 9 +- src/test/func/domestication/domestication.cc | 7 +- src/test/func/pagemap/pagemap.cc | 7 +- 25 files changed, 753 insertions(+), 435 deletions(-) create mode 100644 src/snmalloc/ds_core/mitigations.h diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e5758e2a5..e264e3289 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -58,6 +58,13 @@ jobs: build-type: Debug self-host: true extra-cmake-flags: "-DSNMALLOC_USE_PTHREAD_DESTRUCTORS=On" + # Extra build to check using individual mitigations works. + - os: "ubuntu-latest" + variant: "individual mitigations" + dependencies: "sudo apt install ninja-build" + build-type: Release + self-host: true + extra-cmake-flags: "-DSNMALLOC_BENCHMARK_INDIVIDUAL_MITIGATIONS=On -DBUILD_TESTING=Off" # Check that we can build specifically with libstdc++ - os: "ubuntu-latest" variant: "libstdc++ (Build only)" @@ -92,7 +99,7 @@ jobs: run: NINJA_STATUS="%p [%f:%s/%t] %o/s, %es" ninja - name: Test file size of binaries is sane working-directory: ${{github.workspace}}/build - run: "ls -l func-first_operation-fast ; [ $(ls -l func-first_operation-fast | awk '{ print $5}') -lt 10000000 ]" + run: "ls -l libsnmallocshim.* ; [ $(ls -l libsnmallocshim.* | awk '{ print $5}') -lt 10000000 ]" # If the tests are enabled for this job, run them - name: Test if: ${{ matrix.build-only != 'yes' }} @@ -102,12 +109,9 @@ jobs: if: ${{ matrix.self-host }} working-directory: ${{github.workspace}}/build run: | - sudo cp libsnmallocshim.so libsnmallocshim-checks.so /usr/local/lib/ - ninja clean - LD_PRELOAD=/usr/local/lib/libsnmallocshim.so ninja - ninja clean - LD_PRELOAD=/usr/local/lib/libsnmallocshim-checks.so ninja - + mkdir libs + cp libsnmallocshim*.so libs + for lib in `ls libs`; do echo; echo Testing $lib; ninja clean; LD_PRELOAD=libs/$lib ninja libsnmallocshim.so; done # GitHub doesn't natively support *BSD, but we can run them in VMs on Mac / # Linux runners freebsd: diff --git a/CMakeLists.txt b/CMakeLists.txt index 57de8cf04..b1358d1cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,6 +25,7 @@ option(SNMALLOC_NO_REALLOCARRAY "Build without reallocarray exported" ON) option(SNMALLOC_NO_REALLOCARR "Build without reallocarr exported" ON) option(SNMALLOC_LINK_ICF "Link with Identical Code Folding" ON) option(SNMALLOC_IPO "Link with IPO/LTO support" OFF) +option(SNMALLOC_BENCHMARK_INDIVIDUAL_MITIGATIONS "Build tests and ld_preload for individual mitigations" OFF) # Options that apply only if we're not building the header-only library cmake_dependent_option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) cmake_dependent_option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) @@ -271,7 +272,6 @@ function(add_warning_flags name) $<$:$<${ci_or_debug}:/DEBUG>>) endfunction() - # To build with just the header library target define SNMALLOC_HEADER_ONLY_LIBRARY if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) @@ -286,6 +286,78 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) set(${result} ${dirlist} PARENT_SCOPE) endfunction() + set(TESTDIR ${CMAKE_CURRENT_SOURCE_DIR}/src/test) + + if(BUILD_TESTING) + enable_testing() + subdirlist(TEST_CATEGORIES ${TESTDIR}) + else() + set(TEST_CATEGORIES "") + endif() + list(REVERSE TEST_CATEGORIES) + + if (${SNMALLOC_CLEANUP} STREQUAL THREAD_CLEANUP) + set(TEST_CLEANUP PTHREAD_DESTRUCTORS) + else () + set(TEST_CLEANUP ${SNMALLOC_CLEANUP}) + endif() + + function(make_tests TAG DEFINES) + foreach(TEST_CATEGORY ${TEST_CATEGORIES}) + message(STATUS "Adding ${TAG}/${TEST_CATEGORY} tests") + subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) + foreach(TEST ${TESTS}) + unset(SRC) + aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) + set(TESTNAME "${TEST_CATEGORY}-${TEST}-${TAG}") + + add_executable(${TESTNAME} ${SRC}) + + if(SNMALLOC_SANITIZER) + target_compile_options(${TESTNAME} PRIVATE -g -fsanitize=${SNMALLOC_SANITIZER} -fno-omit-frame-pointer) + target_link_libraries(${TESTNAME} -fsanitize=${SNMALLOC_SANITIZER}) + endif() + + add_warning_flags(${TESTNAME}) + + target_link_libraries(${TESTNAME} snmalloc) + target_compile_definitions(${TESTNAME} PRIVATE "SNMALLOC_USE_${TEST_CLEANUP}") + + if (NOT DEFINES STREQUAL " ") + target_compile_definitions(${TESTNAME} PRIVATE ${DEFINES}) + endif() + + if (${TEST} MATCHES "release-.*") + message(VERBOSE "Adding test: ${TESTNAME} only for release configs") + add_test(NAME ${TESTNAME} COMMAND ${TESTNAME} CONFIGURATIONS "Release") + else() + message(VERBOSE "Adding test: ${TESTNAME}") + add_test(${TESTNAME} ${TESTNAME}) + endif() + if (${TEST_CATEGORY} MATCHES "perf") + message(VERBOSE "Single threaded test: ${TESTNAME}") + set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) + endif() + if(WIN32) + # On Windows these tests use a lot of memory as it doesn't support + # lazy commit. + if (${TEST} MATCHES "two_alloc_types") + message(VERBOSE "Single threaded test: ${TESTNAME}") + set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) + endif() + if (${TEST} MATCHES "fixed_region") + message(VERBOSE "Single threaded test: ${TESTNAME}") + set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) + endif() + if (${TEST} MATCHES "memory") + message(VERBOSE "Single threaded test: ${TESTNAME}") + set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) + endif() + endif() + endforeach() + endforeach() + endfunction() + if(NOT (DEFINED SNMALLOC_LINKER_FLAVOUR) OR ("${SNMALLOC_LINKER_FLAVOUR}" MATCHES "^$")) # Linker not specified externally; probe to see if we can make lld work set(CMAKE_REQUIRED_LINK_OPTIONS -fuse-ld=lld -Wl,--icf=all) @@ -376,89 +448,75 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) target_compile_definitions(snmallocshim-checks-rust PRIVATE SNMALLOC_CHECK_CLIENT) endif() - set(TESTDIR ${CMAKE_CURRENT_SOURCE_DIR}/src/test) - - if(BUILD_TESTING) - enable_testing() - subdirlist(TEST_CATEGORIES ${TESTDIR}) - else() - set(TEST_CATEGORIES "") - endif() + if (BUILD_TESTING) + if (WIN32 + OR (CMAKE_SYSTEM_NAME STREQUAL NetBSD) + OR (CMAKE_SYSTEM_NAME STREQUAL OpenBSD) + OR (CMAKE_SYSTEM_NAME STREQUAL DragonFly) + OR (CMAKE_SYSTEM_NAME STREQUAL SunOS)) + # Windows does not support aligned allocation well enough + # for pass through. + # NetBSD, OpenBSD and DragonFlyBSD do not support malloc*size calls. + set(FLAVOURS fast;check) + else() + set(FLAVOURS fast;check;malloc) + endif() - list(REVERSE TEST_CATEGORIES) - if (${SNMALLOC_CLEANUP} STREQUAL THREAD_CLEANUP) - set(TEST_CLEANUP PTHREAD_DESTRUCTORS) - else () - set(TEST_CLEANUP ${SNMALLOC_CLEANUP}) - endif() - foreach(TEST_CATEGORY ${TEST_CATEGORIES}) - message(STATUS "Adding ${TEST_CATEGORY} tests") - subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) - foreach(TEST ${TESTS}) - if (WIN32 - OR (CMAKE_SYSTEM_NAME STREQUAL NetBSD) - OR (CMAKE_SYSTEM_NAME STREQUAL OpenBSD) - OR (CMAKE_SYSTEM_NAME STREQUAL DragonFly) - OR (CMAKE_SYSTEM_NAME STREQUAL SunOS)) - # Windows does not support aligned allocation well enough - # for pass through. - # NetBSD, OpenBSD and DragonFlyBSD do not support malloc*size calls. - set(FLAVOURS fast;check) - else() - set(FLAVOURS fast;check;malloc) + foreach(FLAVOUR ${FLAVOURS}) + if (${FLAVOUR} STREQUAL "malloc") + set(DEFINES SNMALLOC_PASS_THROUGH) + endif() + if (${FLAVOUR} STREQUAL "check") + set(DEFINES SNMALLOC_CHECK_CLIENT) + endif() + if (${FLAVOUR} STREQUAL "fast") + set(DEFINES " ") endif() - foreach(FLAVOUR ${FLAVOURS}) - unset(SRC) - aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC) - set(TESTNAME "${TEST_CATEGORY}-${TEST}-${FLAVOUR}") - - add_executable(${TESTNAME} ${SRC}) - if(SNMALLOC_SANITIZER) - target_compile_options(${TESTNAME} PRIVATE -g -fsanitize=${SNMALLOC_SANITIZER} -fno-omit-frame-pointer) - target_link_libraries(${TESTNAME} -fsanitize=${SNMALLOC_SANITIZER}) - endif() + make_tests(${FLAVOUR} ${DEFINES}) + endforeach() + endif() - add_warning_flags(${TESTNAME}) + if (SNMALLOC_BENCHMARK_INDIVIDUAL_MITIGATIONS) + set (MITIGATIONS + metadata_protection; + pal_enforce_access; + random_pagemap; + sanity_checks; + freelist_forward_edge; + freelist_backward_edge; + freelist_teardown_validate; + reuse_LIFO; + random_larger_thresholds; + random_initial; + random_preserve; + random_extra_slab) + + + foreach (MITIGATION ${MITIGATIONS}) + set(DEFINES "SNMALLOC_CHECK_CLIENT_MITIGATIONS=${MITIGATION}") + add_shim(snmallocshim-${MITIGATION} SHARED ${SHIM_FILES}) + target_compile_definitions(snmallocshim-${MITIGATION} PRIVATE ${DEFINES}) + if (BUILD_TESTING) + make_tests(${MITIGATION} ${DEFINES}) + endif() + endforeach() - if (${FLAVOUR} STREQUAL "malloc") - target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_PASS_THROUGH) - endif() - if (${FLAVOUR} STREQUAL "check") - target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_CHECK_CLIENT) - endif() - target_link_libraries(${TESTNAME} snmalloc) - target_compile_definitions(${TESTNAME} PRIVATE "SNMALLOC_USE_${TEST_CLEANUP}") - if (${TEST} MATCHES "release-.*") - message(VERBOSE "Adding test: ${TESTNAME} only for release configs") - add_test(NAME ${TESTNAME} COMMAND ${TESTNAME} CONFIGURATIONS "Release") - else() - message(VERBOSE "Adding test: ${TESTNAME}") - add_test(${TESTNAME} ${TESTNAME}) - endif() - if (${TEST_CATEGORY} MATCHES "perf") - message(VERBOSE "Single threaded test: ${TESTNAME}") - set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) - endif() - if(WIN32) - # On Windows these tests use a lot of memory as it doesn't support - # lazy commit. - if (${TEST} MATCHES "two_alloc_types") - message(VERBOSE "Single threaded test: ${TESTNAME}") - set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) - endif() - if (${TEST} MATCHES "fixed_region") - message(VERBOSE "Single threaded test: ${TESTNAME}") - set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) - endif() - if (${TEST} MATCHES "memory") - message(VERBOSE "Single threaded test: ${TESTNAME}") - set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) - endif() - endif() - endforeach() + set(MITIGATIONSET "no_checks") + set(COUNT 0) + foreach (MITIGATION ${MITIGATIONS}) + MATH(EXPR COUNT "${COUNT} + 1") + set(MITIGATIONNAME "mitigations-${COUNT}") + set(MITIGATIONSET "${MITIGATIONSET}+${MITIGATION}") + message(STATUS "MITIGATIONSET: ${COUNT} -> ${MITIGATIONSET}") + set(DEFINES "-DSNMALLOC_CHECK_CLIENT_MITIGATIONS=${MITIGATIONSET}") + add_shim(snmallocshim-${MITIGATIONNAME} SHARED ${SHIM_FILES}) + target_compile_definitions(snmallocshim-${MITIGATIONNAME} PRIVATE ${DEFINES}) + if (BUILD_TESTING) + make_tests(${MITIGATIONNAME} ${DEFINES}) + endif() endforeach() - endforeach() + endif() clangformat_targets() endif() diff --git a/docs/PORTING.md b/docs/PORTING.md index 0f1f2f0a6..2bfb46c2c 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -30,7 +30,7 @@ If memory is not required any more, then `snmalloc` will change the state to `not using`, and will ensure that it notifies the `Pal` again before it every accesses that memory again. The `not using` state allows the `Pal` to recycle the memory for other purposes. -If `PalEnforceAccess` is set to true, then accessing that has not been notified +If `pal_enforce_access` is set as a mitigation, then accessing memory that has not been notified correctly should trigger an exception/segfault. The state for a particular region of memory is set with diff --git a/src/snmalloc/backend/globalconfig.h b/src/snmalloc/backend/globalconfig.h index 0a69c860c..d1f6bf02c 100644 --- a/src/snmalloc/backend/globalconfig.h +++ b/src/snmalloc/backend/globalconfig.h @@ -9,19 +9,6 @@ # include "meta_protected_range.h" # include "standard_range.h" -# if defined(SNMALLOC_CHECK_CLIENT) && !defined(OPEN_ENCLAVE) -/** - * Protect meta data blocks by allocating separate from chunks for - * user allocations. This involves leaving gaps in address space. - * This is less efficient, so should only be applied for the checked - * build. - * - * On Open Enclave the address space is limited, so we disable this - * feature. - */ -# define SNMALLOC_META_PROTECTED -# endif - namespace snmalloc { // Forward reference to thread local cleanup. @@ -79,11 +66,10 @@ namespace snmalloc /** * Use one of the default range configurations */ -# ifdef SNMALLOC_META_PROTECTED - using LocalState = MetaProtectedRangeLocalState; -# else - using LocalState = StandardLocalState; -# endif + using LocalState = std::conditional_t< + mitigations(metadata_protection), + MetaProtectedRangeLocalState, + StandardLocalState>; /** * Use the default backend. @@ -136,14 +122,11 @@ namespace snmalloc // Initialise key for remote deallocation lists key_global = FreeListKey(entropy.get_free_list_key()); - // Need to initialise pagemap. If SNMALLOC_CHECK_CLIENT is set and this - // isn't a StrictProvenance architecture, randomize its table's location - // within a significantly larger address space allocation. -# if defined(SNMALLOC_CHECK_CLIENT) - static constexpr bool pagemap_randomize = !aal_supports; -# else - static constexpr bool pagemap_randomize = false; -# endif + // Need to randomise pagemap location. If requested and not a + // StrictProvenance architecture, randomize its table's location within a + // significantly larger address space allocation. + static constexpr bool pagemap_randomize = + mitigations(random_pagemap) && !aal_supports; Pagemap::concretePagemap.template init(); diff --git a/src/snmalloc/ds/allocconfig.h b/src/snmalloc/ds/allocconfig.h index 1089ccf7a..7f1e55e97 100644 --- a/src/snmalloc/ds/allocconfig.h +++ b/src/snmalloc/ds/allocconfig.h @@ -49,11 +49,8 @@ namespace snmalloc static constexpr size_t MIN_CHUNK_SIZE = bits::one_at_bit(MIN_CHUNK_BITS); // Minimum number of objects on a slab -#ifdef SNMALLOC_CHECK_CLIENT - static constexpr size_t MIN_OBJECT_COUNT = 13; -#else - static constexpr size_t MIN_OBJECT_COUNT = 4; -#endif + static constexpr size_t MIN_OBJECT_COUNT = + mitigations(random_larger_thresholds) ? 13 : 4; // Maximum size of an object that uses sizeclasses. #if defined(SNMALLOC_QEMU_WORKAROUND) && defined(SNMALLOC_VA_BITS_64) diff --git a/src/snmalloc/ds/pagemap.h b/src/snmalloc/ds/pagemap.h index aa6117b04..267fe9a0b 100644 --- a/src/snmalloc/ds/pagemap.h +++ b/src/snmalloc/ds/pagemap.h @@ -213,6 +213,10 @@ namespace snmalloc } else { + if constexpr (pal_supports) + { + PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE); + } new_body = static_cast(new_body_untyped); } // Ensure bottom page is committed diff --git a/src/snmalloc/ds_core/defines.h b/src/snmalloc/ds_core/defines.h index 54a4d7710..2de53be03 100644 --- a/src/snmalloc/ds_core/defines.h +++ b/src/snmalloc/ds_core/defines.h @@ -27,6 +27,11 @@ # define SNMALLOC_REQUIRE_CONSTINIT # define SNMALLOC_UNUSED_FUNCTION # define SNMALLOC_USED_FUNCTION +# ifdef SNMALLOC_USE_CXX17 +# define SNMALLOC_NO_UNIQUE_ADDRESS +# else +# define SNMALLOC_NO_UNIQUE_ADDRESS [[msvc::no_unique_address]] +# endif #else # define SNMALLOC_FAST_FAIL() __builtin_trap() # define SNMALLOC_LIKELY(x) __builtin_expect(!!(x), 1) @@ -50,6 +55,11 @@ # define SNMALLOC_COLD __attribute__((cold)) # define SNMALLOC_UNUSED_FUNCTION __attribute((unused)) # define SNMALLOC_USED_FUNCTION __attribute((used)) +# ifdef SNMALLOC_USE_CXX17 +# define SNMALLOC_NO_UNIQUE_ADDRESS +# else +# define SNMALLOC_NO_UNIQUE_ADDRESS [[no_unique_address]] +# endif # ifdef __clang__ # define SNMALLOC_REQUIRE_CONSTINIT \ [[clang::require_constant_initialization]] @@ -192,43 +202,4 @@ namespace snmalloc template SNMALLOC_FAST_PATH_INLINE void UNUSED(Args&&...) {} - - template - inline SNMALLOC_FAST_PATH void - check_client_error(const char* const str, Args... args) - { - //[[clang::musttail]] - return snmalloc::report_fatal_error(str, args...); - } - - template - inline SNMALLOC_FAST_PATH void - check_client_impl(bool test, const char* const str, Args... args) - { - if (SNMALLOC_UNLIKELY(!test)) - { - if constexpr (!DEBUG) - { - UNUSED(str, args...); - SNMALLOC_FAST_FAIL(); - } - else - { - check_client_error(str, args...); - } - } - } - -#ifdef SNMALLOC_CHECK_CLIENT - static constexpr bool CHECK_CLIENT = true; -#else - static constexpr bool CHECK_CLIENT = false; -#endif } // namespace snmalloc - -#ifdef SNMALLOC_CHECK_CLIENT -# define snmalloc_check_client(test, str, ...) \ - snmalloc::check_client_impl(test, str, ##__VA_ARGS__) -#else -# define snmalloc_check_client(test, str, ...) -#endif diff --git a/src/snmalloc/ds_core/ds_core.h b/src/snmalloc/ds_core/ds_core.h index 672f7d1b0..2083190bc 100644 --- a/src/snmalloc/ds_core/ds_core.h +++ b/src/snmalloc/ds_core/ds_core.h @@ -11,6 +11,7 @@ #include "concept.h" #include "defines.h" #include "helpers.h" +#include "mitigations.h" #include "ptrwrap.h" #include "redblacktree.h" #include "seqset.h" diff --git a/src/snmalloc/ds_core/helpers.h b/src/snmalloc/ds_core/helpers.h index e1e41d373..61fcee954 100644 --- a/src/snmalloc/ds_core/helpers.h +++ b/src/snmalloc/ds_core/helpers.h @@ -37,7 +37,7 @@ namespace snmalloc } }; -#ifdef SNMALLOC_CHECK_CLIENT +#ifdef SNMALLOC_CHECK_CLIENT // TODO is this used/helpful? template class ModArray { diff --git a/src/snmalloc/ds_core/mitigations.h b/src/snmalloc/ds_core/mitigations.h new file mode 100644 index 000000000..88547dcc7 --- /dev/null +++ b/src/snmalloc/ds_core/mitigations.h @@ -0,0 +1,263 @@ +#pragma once +#include "defines.h" + +#include + +namespace snmalloc +{ + template + inline SNMALLOC_FAST_PATH void + check_client_error(const char* const str, Args... args) + { + //[[clang::musttail]] + return snmalloc::report_fatal_error(str, args...); + } + + template + inline SNMALLOC_FAST_PATH void + check_client_impl(bool test, const char* const str, Args... args) + { + if (SNMALLOC_UNLIKELY(!test)) + { + if constexpr (!DEBUG) + { + UNUSED(str, args...); + SNMALLOC_FAST_FAIL(); + } + else + { + check_client_error(str, args...); + } + } + } + +#ifdef SNMALLOC_CHECK_CLIENT + static constexpr bool CHECK_CLIENT = true; +#else + static constexpr bool CHECK_CLIENT = false; +#endif + + namespace mitigation + { + class type + { + size_t mask; + + public: + constexpr type(size_t f) : mask(f){}; + constexpr type(const type& t) = default; + + constexpr type operator+(const type b) const + { + return {mask | b.mask}; + } + + constexpr type operator-(const type b) const + { + return {mask & ~(b.mask)}; + } + + constexpr bool operator()(const type a) const + { + return (mask & a.mask) != 0; + } + }; + } // namespace mitigation + + /** + * Randomize the location of the pagemap within a larger address space + * allocation. The other pages in that allocation may fault if accessed, on + * platforms that can efficiently express such configurations. + * + * This guards against adversarial attempts to access the pagemap. + * + * This is unnecessary on StrictProvenance architectures. + */ + constexpr mitigation::type random_pagemap{1 << 0}; + /** + * Ensure that every slab (especially slabs used for larger "small" size + * classes) has a larger minimum number of objects and that a larger + * percentage of objects in a slab must be free to awaken the slab. + * + * This should frustrate use-after-reallocation attacks by delaying reuse. + * When combined with random_preserve, below, it additionally ensures that at + * least some shuffling of free objects is possible, and, hence, that there + * is at least some unpredictability of reuse. + * + * TODO: should this be split? mjp: Would require changing some thresholds. + * The min waking count needs to be ensure we have enough objects on a slab, + * hence is related to the min count on a slab. Currently we without this, we + * have min count of slab of 16, and a min waking count with this enabled + * of 32. So we would leak memory. + */ + constexpr mitigation::type random_larger_thresholds{1 << 1}; + /** + * + * Obfuscate forward-edge pointers in intra-slab free lists. + * + * This helps prevent a UAF write from re-pointing the free list arbitrarily, + * as the de-obfuscation of a corrupted pointer will generate a wild address. + * + * This is not available on StrictProvenance architectures. + */ + constexpr mitigation::type freelist_forward_edge{1 << 2}; + /** + * Store obfuscated backward-edge addresses in intra-slab free lists. + * + * Ordinarily, these lists are singly-linked. Storing backward-edges allows + * the allocator to verify the well-formedness of the links and, importantly, + * the acyclicity of the list itself. These backward-edges are also + * obfuscated in an attempt to frustrate an attacker armed with UAF + * attempting to construct a new well-formed list. + * + * Because the backward-edges are not traversed, this is available on + * StrictProvenance architectures, unlike freelist_forward_edge. + * + * This is required to detect double frees as it will break the doubly linked + * nature of the free list. + */ + constexpr mitigation::type freelist_backward_edge{1 << 3}; + /** + * When de-purposing a slab (releasing its address space for reuse at a + * different size class or allocation), walk the free list and validate the + * domestication of all nodes along it. + * + * If freelist_forward_edge is also enabled, this will probe the + * domestication status of the de-obfuscated pointers before traversal. + * Each of domestication and traversal may probabilistically catch UAF + * corruption of the free list. + * + * If freelist_backward_edge is also enabled, this will verify the integrity + * of the free list links. + * + * This gives the allocator "one last chance" to catch UAF corruption of a + * slab's free list before the slab is de-purposed. + * + * This is required to comprehensively detect double free. + */ + constexpr mitigation::type freelist_teardown_validate{1 << 4}; + /** + * When initializing a slab, shuffle its free list. + * + * This guards against attackers relying on object-adjacency or address-reuse + * properties of the allocation stream. + */ + constexpr mitigation::type random_initial{1 << 5}; + /** + * When a slab is operating, randomly assign freed objects to one of two + * intra-slab free lists. When selecting a slab's free list for allocations, + * select the longer of the two. + * + * This guards against attackers relying on object-adjacency or address-reuse + * properties of the allocation stream. + */ + constexpr mitigation::type random_preserve{1 << 6}; + /** + * Randomly introduce another slab for a given size-class, rather than use + * the last available to an allocator. + * + * This guards against attackers relying on address-reuse, especially in the + * pathological case of a size-class having only one slab with free entries. + */ + constexpr mitigation::type random_extra_slab{1 << 7}; + /** + * Use a LIFO queue, rather than a stack, of slabs with free elements. + * + * This generally increases the time between address reuse. + */ + constexpr mitigation::type reuse_LIFO{1 << 8}; + /** + * This performs a variety of inexpensive "sanity" tests throughout the + * allocator: + * + * - Requests to free objects must + * - not be interior pointers + * - be of allocated address space + * - Requests to free objects which also specify the size must specify a size + * that agrees with the current allocation. + * + * This guards gainst various forms of client misbehavior. + * + * TODO: Should this be split? mjp: It could, but let's not do this until + * we have performance numbers to see what this costs. + */ + constexpr mitigation::type sanity_checks{1 << 9}; + /** + * On CHERI, perform a series of well-formedness tests on capabilities given + * when requesting to free an object. + */ + constexpr mitigation::type cheri_checks{1 << 10}; + /** + * Erase intra-slab free list metadata before completing an allocation. + * + * This mitigates information disclosure. + */ + constexpr mitigation::type clear_meta{1 << 11}; + /** + * Protect meta data blocks by allocating separate from chunks for + * user allocations. This involves leaving gaps in address space. + * This is less efficient, so should only be applied for the checked + * build. + */ + constexpr mitigation::type metadata_protection{1 << 12}; + /** + * If this mitigation is enabled, then Pal implementations should provide + * exceptions/segfaults if accesses do not obey the + * - using + * - using_readonly + * - not_using + * model. + */ + static constexpr mitigation::type pal_enforce_access{1 << 13}; + + constexpr mitigation::type full_checks = random_pagemap + + random_larger_thresholds + freelist_forward_edge + freelist_backward_edge + + freelist_teardown_validate + random_initial + random_preserve + + metadata_protection + random_extra_slab + reuse_LIFO + sanity_checks + + clear_meta + pal_enforce_access; + + constexpr mitigation::type no_checks{0}; + + using namespace mitigation; + constexpr mitigation::type mitigations = +#ifdef SNMALLOC_CHECK_CLIENT_MITIGATIONS + no_checks + SNMALLOC_CHECK_CLIENT_MITIGATIONS; +#elif defined(OPEN_ENCLAVE) + /** + * On Open Enclave the address space is limited, so we disable + * metadata-protection feature. + */ + CHECK_CLIENT ? full_checks - metadata_protection - random_pagemap : + no_checks; +#elif defined(__NetBSD__) + /** + * pal_enforce_access was failing on NetBSD, so we disable it. + */ + CHECK_CLIENT ? full_checks - pal_enforce_access : no_checks; +#elif defined(__CHERI_PURE_CAPABILITY__) + CHECK_CLIENT ? + /** + * freelist_forward_edge should not be used on CHERI as we cannot encode + * pointers as the tag will be destroyed. + * + * TODO: There is a known bug in CheriBSD that means round-tripping through + * PROT_NONE sheds capability load and store permissions (while restoring + * data read/write, for added excitement). For the moment, just force this + * down on CHERI. + */ + full_checks + cheri_checks + clear_meta - freelist_forward_edge - + pal_enforce_access : + /** + * clear_meta is important on CHERI to avoid leaking capabilities. + */ + sanity_checks + cheri_checks + clear_meta; +#else + CHECK_CLIENT ? full_checks : no_checks; +#endif +} // namespace snmalloc + +#define snmalloc_check_client(mitigation, test, str, ...) \ + if constexpr (mitigation) \ + { \ + snmalloc::check_client_impl(test, str, ##__VA_ARGS__); \ + } diff --git a/src/snmalloc/mem/corealloc.h b/src/snmalloc/mem/corealloc.h index 4b5b9828f..534fb6043 100644 --- a/src/snmalloc/mem/corealloc.h +++ b/src/snmalloc/mem/corealloc.h @@ -220,73 +220,78 @@ namespace snmalloc auto& b = meta->free_queue; -#ifdef SNMALLOC_CHECK_CLIENT - // Structure to represent the temporary list elements - struct PreAllocObject + if constexpr (mitigations(random_initial)) { - capptr::AllocFull next; - }; - // The following code implements Sattolo's algorithm for generating - // random cyclic permutations. This implementation is in the opposite - // direction, so that the original space does not need initialising. This - // is described as outside-in without citation on Wikipedia, appears to be - // Folklore algorithm. - - // Note the wide bounds on curr relative to each of the ->next fields; - // curr is not persisted once the list is built. - capptr::Chunk curr = - pointer_offset(bumpptr, 0).template as_static(); - curr->next = Aal::capptr_bound( - curr, rsize); - - uint16_t count = 1; - for (curr = - pointer_offset(curr, rsize).template as_static(); - curr.as_void() < slab_end; - curr = - pointer_offset(curr, rsize).template as_static()) - { - size_t insert_index = entropy.sample(count); - curr->next = std::exchange( - pointer_offset(bumpptr, insert_index * rsize) - .template as_static() - ->next, + // Structure to represent the temporary list elements + struct PreAllocObject + { + capptr::AllocFull next; + }; + // The following code implements Sattolo's algorithm for generating + // random cyclic permutations. This implementation is in the opposite + // direction, so that the original space does not need initialising. + // This is described as outside-in without citation on Wikipedia, + // appears to be Folklore algorithm. + + // Note the wide bounds on curr relative to each of the ->next fields; + // curr is not persisted once the list is built. + capptr::Chunk curr = + pointer_offset(bumpptr, 0).template as_static(); + curr->next = Aal::capptr_bound( - curr, rsize)); - count++; - } + curr, rsize); + + uint16_t count = 1; + for (curr = + pointer_offset(curr, rsize).template as_static(); + curr.as_void() < slab_end; + curr = + pointer_offset(curr, rsize).template as_static()) + { + size_t insert_index = entropy.sample(count); + curr->next = std::exchange( + pointer_offset(bumpptr, insert_index * rsize) + .template as_static() + ->next, + Aal::capptr_bound( + curr, rsize)); + count++; + } - // Pick entry into space, and then build linked list by traversing cycle - // to the start. Use ->next to jump from Chunk to Alloc. - auto start_index = entropy.sample(count); - auto start_ptr = pointer_offset(bumpptr, start_index * rsize) - .template as_static() - ->next; - auto curr_ptr = start_ptr; - do - { - b.add( - // Here begins our treatment of the heap as containing Wild pointers - freelist::Object::make( - capptr_to_user_address_control(curr_ptr.as_void())), - key, - entropy); - curr_ptr = curr_ptr->next; - } while (curr_ptr != start_ptr); -#else - auto p = bumpptr; - do + // Pick entry into space, and then build linked list by traversing cycle + // to the start. Use ->next to jump from Chunk to Alloc. + auto start_index = entropy.sample(count); + auto start_ptr = pointer_offset(bumpptr, start_index * rsize) + .template as_static() + ->next; + auto curr_ptr = start_ptr; + do + { + b.add( + // Here begins our treatment of the heap as containing Wild pointers + freelist::Object::make( + capptr_to_user_address_control(curr_ptr.as_void())), + key, + entropy); + curr_ptr = curr_ptr->next; + } while (curr_ptr != start_ptr); + } + else { - b.add( - // Here begins our treatment of the heap as containing Wild pointers - freelist::Object::make( - capptr_to_user_address_control( - Aal::capptr_bound( - p.as_void(), rsize))), - key); - p = pointer_offset(p, rsize); - } while (p < slab_end); -#endif + auto p = bumpptr; + do + { + b.add( + // Here begins our treatment of the heap as containing Wild pointers + freelist::Object::make( + capptr_to_user_address_control( + Aal::capptr_bound( + p.as_void(), rsize))), + key, + entropy); + p = pointer_offset(p, rsize); + } while (p < slab_end); + } // This code consumes everything up to slab_end. bumpptr = slab_end; } @@ -306,48 +311,44 @@ namespace snmalloc capptr::Alloc p = finish_alloc_no_zero(fl.take(key, domesticate), sizeclass); -#ifdef SNMALLOC_CHECK_CLIENT - // Check free list is well-formed on platforms with - // integers as pointers. - size_t count = 1; // Already taken one above. - while (!fl.empty()) + // If clear_meta is requested, we should also walk the free list to clear + // it. + // TODO: we could optimise the clear_meta case to not walk the free list + // and instead just clear the whole slab, but that requires amplification. + if constexpr ( + mitigations(freelist_teardown_validate) || mitigations(clear_meta)) { - fl.take(key, domesticate); - count++; - } - // Check the list contains all the elements - SNMALLOC_CHECK( - (count + more) == snmalloc::sizeclass_to_slab_object_count(sizeclass)); - - if (more > 0) - { - auto no_more = meta->free_queue.close(fl, key); - SNMALLOC_ASSERT(no_more == 0); - UNUSED(no_more); - + // Check free list is well-formed on platforms with + // integers as pointers. + size_t count = 1; // Already taken one above. while (!fl.empty()) { fl.take(key, domesticate); count++; } + // Check the list contains all the elements + SNMALLOC_CHECK( + (count + more) == + snmalloc::sizeclass_to_slab_object_count(sizeclass)); + + if (more > 0) + { + auto no_more = meta->free_queue.close(fl, key); + SNMALLOC_ASSERT(no_more == 0); + UNUSED(no_more); + + while (!fl.empty()) + { + fl.take(key, domesticate); + count++; + } + } + SNMALLOC_CHECK( + count == snmalloc::sizeclass_to_slab_object_count(sizeclass)); } - SNMALLOC_CHECK( - count == snmalloc::sizeclass_to_slab_object_count(sizeclass)); -#endif - // TODO: This is a capability amplification as we are saying we - // have the whole chunk. auto start_of_slab = pointer_align_down( p, snmalloc::sizeclass_to_slab_size(sizeclass)); -#if defined(__CHERI_PURE_CAPABILITY__) && !defined(SNMALLOC_CHECK_CLIENT) - // Zero the whole slab. For CHERI we at least need to clear the freelist - // pointers to avoid leaking capabilities but we do not need to do it in - // the freelist order as for SNMALLOC_CHECK_CLIENT. Zeroing the whole slab - // may be more friendly to hw because it does not involve pointer chasing - // and is amenable to prefetching. - // FIXME: This should be a back-end method guarded on a feature flag. -#endif - #ifdef SNMALLOC_TRACING message<1024>( "Slab {} is unused, Object sizeclass {}", @@ -701,6 +702,7 @@ namespace snmalloc SNMALLOC_ASSERT(!meta->is_unused()); snmalloc_check_client( + mitigations(sanity_checks), is_start_of_object(entry.get_sizeclass(), address_cast(p)), "Not deallocating start of an object"); @@ -722,23 +724,18 @@ namespace snmalloc auto& sl = alloc_classes[sizeclass].available; if (SNMALLOC_LIKELY(alloc_classes[sizeclass].length > 0)) { -#ifdef SNMALLOC_CHECK_CLIENT - // Occassionally don't use the last list. - if (SNMALLOC_UNLIKELY(alloc_classes[sizeclass].length == 1)) + if constexpr (mitigations(random_extra_slab)) { - // If the slab has a lot of free space, then we shouldn't allocate a - // new slab. - auto min = alloc_classes[sizeclass] - .available.peek() - ->free_queue.min_list_length(); - if ((min * 2) < threshold_for_waking_slab(sizeclass)) + // Occassionally don't use the last list. + if (SNMALLOC_UNLIKELY(alloc_classes[sizeclass].length == 1)) + { if (entropy.next_bit() == 0) return small_alloc_slow(sizeclass, fast_free_list); + } } -#endif - // If CHECK_CLIENT, we use FIFO operations on the list. This reduces - // perf slightly, but increases randomness. - auto meta = sl.template pop(); + + // Mitigations use LIFO to increase time to reuse. + auto meta = sl.template pop(); // Drop length of sl, and empty count if it was empty. alloc_classes[sizeclass].length--; if (meta->needed() == 0) diff --git a/src/snmalloc/mem/freelist.h b/src/snmalloc/mem/freelist.h index 2830f9b8f..1841a5af3 100644 --- a/src/snmalloc/mem/freelist.h +++ b/src/snmalloc/mem/freelist.h @@ -122,18 +122,47 @@ namespace snmalloc friend class Object; + class Empty + { + public: + void check_prev(address_t) {} + + void set_prev(address_t) {} + }; + + class Prev + { + address_t prev_encoded; + + public: + /** + * Check the signature of this free Object + */ + void check_prev(address_t signed_prev) + { + snmalloc_check_client( + mitigations(freelist_backward_edge), + signed_prev == prev_encoded, + "Heap corruption - free list corrupted!"); + UNUSED(signed_prev); + } + + void set_prev(address_t signed_prev) + { + prev_encoded = signed_prev; + } + }; + union { BQueuePtr next_object; // TODO: Should really use C++20 atomic_ref rather than a union. BAtomicQueuePtr atomic_next_object; }; -#ifdef SNMALLOC_CHECK_CLIENT - // Encoded representation of a back pointer. - // Hard to fake, and provides consistency on - // the next pointers. - address_t prev_encoded; -#endif + + SNMALLOC_NO_UNIQUE_ADDRESS + std::conditional_t + prev; public: template< @@ -148,13 +177,14 @@ namespace snmalloc this->atomic_next_object.load(std::memory_order_acquire), key); auto n_tame = domesticate(n_wild); -#ifdef SNMALLOC_CHECK_CLIENT - if (n_tame != nullptr) + if constexpr (mitigations(freelist_backward_edge)) { - n_tame->check_prev( - signed_prev(address_cast(this), address_cast(n_tame), key)); + if (n_tame != nullptr) + { + n_tame->prev.check_prev( + signed_prev(address_cast(this), address_cast(n_tame), key)); + } } -#endif return n_tame; } @@ -177,25 +207,22 @@ namespace snmalloc */ void check_prev(address_t signed_prev) { - UNUSED(signed_prev); - snmalloc_check_client( - signed_prev == this->prev_encoded, - "Heap corruption - free list corrupted!"); + prev.check_prev(signed_prev); } /** - * Clean up this object when removing it from the list. This is - * important on CHERI to avoid leaking capabilities. On CHECK_CLIENT - * builds it might increase the difficulty to bypass the checks. + * Clean up this object when removing it from the list. */ void cleanup() { -#if defined(__CHERI_PURE_CAPABILITY__) || defined(SNMALLOC_CHECK_CLIENT) - this->next_object = nullptr; -# ifdef SNMALLOC_CHECK_CLIENT - this->prev_encoded = 0; -# endif -#endif + if constexpr (mitigations(clear_meta)) + { + this->next_object = nullptr; + if constexpr (mitigations(freelist_backward_edge)) + { + this->prev.set_prev(0); + } + } } }; @@ -233,7 +260,8 @@ namespace snmalloc // Curr is not used in the current encoding scheme. UNUSED(curr); - if constexpr (CHECK_CLIENT && !aal_supports) + if constexpr ( + mitigations(freelist_forward_edge) && !aal_supports) { return unsafe_from_uintptr>( unsafe_to_uintptr>(next) ^ key.key_next); @@ -331,12 +359,14 @@ namespace snmalloc { assert_view_queue_bounds(); -#ifdef SNMALLOC_CHECK_CLIENT - next->prev_encoded = - signed_prev(address_cast(curr), address_cast(next), key); -#else - UNUSED(key); -#endif + if constexpr (mitigations(freelist_backward_edge)) + { + next->prev.set_prev( + signed_prev(address_cast(curr), address_cast(next), key)); + } + else + UNUSED(key); + *curr = encode_next(address_cast(curr), next, key); return &(next->next_object); } @@ -363,12 +393,14 @@ namespace snmalloc { static_assert(BView::wildness == capptr::dimension::Wildness::Tame); -#ifdef SNMALLOC_CHECK_CLIENT - next->prev_encoded = - signed_prev(address_cast(curr), address_cast(next), key); -#else - UNUSED(key); -#endif + if constexpr (mitigations(freelist_backward_edge)) + { + next->prev.set_prev( + signed_prev(address_cast(curr), address_cast(next), key)); + } + else + UNUSED(key); + // Signature needs to be visible before item is linked in // so requires release semantics. curr->atomic_next_object.store( @@ -418,6 +450,39 @@ namespace snmalloc */ using AtomicQueuePtr = Object::BAtomicQueuePtr; + class Prev + { + address_t prev{0}; + + protected: + constexpr Prev(address_t prev) : prev(prev) {} + constexpr Prev() = default; + + address_t replace(address_t next) + { + auto p = prev; + prev = next; + return p; + } + }; + + class NoPrev + { + protected: + constexpr NoPrev(address_t){}; + constexpr NoPrev() = default; + + address_t replace(address_t t) + { + // This should never be called. + SNMALLOC_CHECK(false); + return t; + } + }; + + using IterBase = + std::conditional_t; + /** * Used to iterate a free list in object space. * @@ -426,20 +491,14 @@ namespace snmalloc template< SNMALLOC_CONCEPT(capptr::IsBound) BView = capptr::bounds::Alloc, SNMALLOC_CONCEPT(capptr::IsBound) BQueue = capptr::bounds::AllocWild> - class Iter + class Iter : IterBase { Object::BHeadPtr curr{nullptr}; -#ifdef SNMALLOC_CHECK_CLIENT - address_t prev{0}; -#endif public: constexpr Iter(Object::BHeadPtr head, address_t prev_value) - : curr(head) + : IterBase(prev_value), curr(head) { -#ifdef SNMALLOC_CHECK_CLIENT - prev = prev_value; -#endif UNUSED(prev_value); } @@ -473,12 +532,16 @@ namespace snmalloc Aal::prefetch(next.unsafe_ptr()); curr = next; -#ifdef SNMALLOC_CHECK_CLIENT - c->check_prev(prev); - prev = signed_prev(address_cast(c), address_cast(next), key); -#else - UNUSED(key); -#endif + + if constexpr (mitigations(freelist_backward_edge)) + { + auto p = + replace(signed_prev(address_cast(c), address_cast(next), key)); + c->check_prev(p); + } + else + UNUSED(key); + c->cleanup(); return c; } @@ -544,7 +607,7 @@ namespace snmalloc static_cast*>(head[ix])); } - std::array length{}; + SNMALLOC_NO_UNIQUE_ADDRESS std::array length{}; public: constexpr Builder() = default; @@ -683,7 +746,7 @@ namespace snmalloc for (size_t i = 0; i < LENGTH; i++) { end[i] = &head[i]; - if (RANDOM) + if constexpr (RANDOM) { length[i] = 0; } @@ -726,48 +789,37 @@ namespace snmalloc SNMALLOC_FAST_PATH void validate(const FreeListKey& key, Domesticator domesticate) { -#ifdef SNMALLOC_CHECK_CLIENT - for (uint32_t i = 0; i < LENGTH; i++) + if constexpr (mitigations(freelist_teardown_validate)) { - if (&head[i] == end[i]) + for (uint32_t i = 0; i < LENGTH; i++) { - SNMALLOC_CHECK(length[i] == 0); - continue; + if (&head[i] == end[i]) + { + SNMALLOC_CHECK(!RANDOM || (length[i] == 0)); + continue; + } + + size_t count = 1; + auto curr = read_head(i, key); + auto prev = get_fake_signed_prev(i, key); + while (true) + { + curr->check_prev(prev); + if (address_cast(&(curr->next_object)) == address_cast(end[i])) + break; + count++; + auto next = curr->read_next(key, domesticate); + prev = signed_prev(address_cast(curr), address_cast(next), key); + curr = next; + } + SNMALLOC_CHECK(!RANDOM || (count == length[i])); } - - size_t count = 1; - auto curr = read_head(i, key); - auto prev = get_fake_signed_prev(i, key); - while (true) - { - curr->check_prev(prev); - if (address_cast(&(curr->next_object)) == address_cast(end[i])) - break; - count++; - auto next = curr->read_next(key, domesticate); - prev = signed_prev(address_cast(curr), address_cast(next), key); - curr = next; - } - SNMALLOC_CHECK(count == length[i]); } -#else - UNUSED(key); - UNUSED(domesticate); -#endif - } - - /** - * Returns length of the shorter free list. - * - * This method is only usable if the free list is adding randomisation - * as that is when it has two lists. - */ - template - [[nodiscard]] std::enable_if_t min_list_length() const - { - static_assert(RANDOM_ == RANDOM, "Don't set SFINAE parameter!"); - - return length[0] < length[1] ? length[0] : length[1]; + else + { + UNUSED(key); + UNUSED(domesticate); + } } }; } // namespace freelist diff --git a/src/snmalloc/mem/localalloc.h b/src/snmalloc/mem/localalloc.h index f2827be88..bd1222ac2 100644 --- a/src/snmalloc/mem/localalloc.h +++ b/src/snmalloc/mem/localalloc.h @@ -467,9 +467,9 @@ namespace snmalloc * point where we know, from the pagemap, or by explicitly testing, that the * pointer under test is not nullptr. */ -#if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) - SNMALLOC_SLOW_PATH void dealloc_cheri_checks(void* p) + SNMALLOC_FAST_PATH void dealloc_cheri_checks(void* p) { +#if defined(__CHERI_PURE_CAPABILITY__) /* * Enforce the use of an unsealed capability. * @@ -477,7 +477,9 @@ namespace snmalloc * elide this test in that world. */ snmalloc_check_client( - !__builtin_cheri_sealed_get(p), "Sealed capability in deallocation"); + mitigations(cheri_checks), + !__builtin_cheri_sealed_get(p), + "Sealed capability in deallocation"); /* * Enforce permissions on the returned pointer. These pointers end up in @@ -496,6 +498,7 @@ namespace snmalloc static const size_t reqperm = CHERI_PERM_LOAD | CHERI_PERM_STORE | CHERI_PERM_LOAD_CAP | CHERI_PERM_STORE_CAP; snmalloc_check_client( + mitigations(cheri_checks), (__builtin_cheri_perms_get(p) & reqperm) == reqperm, "Insufficient permissions on capability in deallocation"); @@ -510,13 +513,16 @@ namespace snmalloc * elide this test. */ snmalloc_check_client( - __builtin_cheri_tag_get(p), "Untagged capability in deallocation"); + mitigations(cheri_checks), + __builtin_cheri_tag_get(p), + "Untagged capability in deallocation"); /* * Verify that the capability is not zero-length, ruling out the other * edge case around monotonicity. */ snmalloc_check_client( + mitigations(cheri_checks), __builtin_cheri_length_get(p) > 0, "Zero-length capability in deallocation"); @@ -585,8 +591,10 @@ namespace snmalloc * acceptable security posture for the allocator and between clients; * misbehavior is confined to the misbehaving client. */ - } +#else + UNUSED(p); #endif + } SNMALLOC_FAST_PATH void dealloc(void* p_raw) { @@ -638,9 +646,8 @@ namespace snmalloc if (SNMALLOC_LIKELY(local_cache.remote_allocator == entry.get_remote())) { -# if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) dealloc_cheri_checks(p_tame.unsafe_ptr()); -# endif + if (SNMALLOC_LIKELY(CoreAlloc::dealloc_local_object_fast( entry, p_tame, local_cache.entropy))) return; @@ -651,13 +658,13 @@ namespace snmalloc RemoteAllocator* remote = entry.get_remote(); if (SNMALLOC_LIKELY(remote != nullptr)) { -# if defined(__CHERI_PURE_CAPABILITY__) && defined(SNMALLOC_CHECK_CLIENT) dealloc_cheri_checks(p_tame.unsafe_ptr()); -# endif // Detect double free of large allocations here. snmalloc_check_client( - !entry.is_backend_owned(), "Memory corruption detected"); + mitigations(sanity_checks), + !entry.is_backend_owned(), + "Memory corruption detected"); // Check if we have space for the remote deallocation if (local_cache.remote_dealloc_cache.reserve_space(entry)) @@ -678,7 +685,10 @@ namespace snmalloc // If p_tame is not null, then dealloc has been call on something // it shouldn't be called on. // TODO: Should this be tested even in the !CHECK_CLIENT case? - snmalloc_check_client(p_tame == nullptr, "Not allocated by snmalloc."); + snmalloc_check_client( + mitigations(sanity_checks), + p_tame == nullptr, + "Not allocated by snmalloc."); # ifdef SNMALLOC_TRACING message<1024>("nullptr deallocation"); @@ -689,17 +699,26 @@ namespace snmalloc void check_size(void* p, size_t size) { -#ifdef SNMALLOC_CHECK_CLIENT - size = size == 0 ? 1 : size; - auto sc = size_to_sizeclass_full(size); - auto pm_sc = - Config::Backend::get_metaentry(address_cast(p)).get_sizeclass(); - auto rsize = sizeclass_full_to_size(sc); - auto pm_size = sizeclass_full_to_size(pm_sc); - snmalloc_check_client( - sc == pm_sc, "Dealloc rounded size mismatch: {} != {}", rsize, pm_size); -#else +#ifdef SNMALLOC_PASS_THROUGH UNUSED(p, size); +#else + if constexpr (mitigations(sanity_checks)) + { + size = size == 0 ? 1 : size; + auto sc = size_to_sizeclass_full(size); + auto pm_sc = + Config::Backend::get_metaentry(address_cast(p)).get_sizeclass(); + auto rsize = sizeclass_full_to_size(sc); + auto pm_size = sizeclass_full_to_size(pm_sc); + snmalloc_check_client( + mitigations(sanity_checks), + sc == pm_sc, + "Dealloc rounded size mismatch: {} != {}", + rsize, + pm_size); + } + else + UNUSED(p, size); #endif } diff --git a/src/snmalloc/mem/metadata.h b/src/snmalloc/mem/metadata.h index 9dc21eb13..8b1314e2e 100644 --- a/src/snmalloc/mem/metadata.h +++ b/src/snmalloc/mem/metadata.h @@ -396,11 +396,8 @@ namespace snmalloc /** * Data-structure for building the free list for this slab. */ -#ifdef SNMALLOC_CHECK_CLIENT - freelist::Builder free_queue; -#else - freelist::Builder free_queue; -#endif + SNMALLOC_NO_UNIQUE_ADDRESS freelist::Builder + free_queue; /** * The number of deallocation required until we hit a slow path. This @@ -566,11 +563,10 @@ namespace snmalloc auto p = tmp_fl.take(key, domesticate); fast_free_list = tmp_fl; -#ifdef SNMALLOC_CHECK_CLIENT - entropy.refresh_bits(); -#else - UNUSED(entropy); -#endif + if constexpr (mitigations(random_preserve)) + entropy.refresh_bits(); + else + UNUSED(entropy); // This marks the slab as sleeping, and sets a wakeup // when sufficient deallocations have occurred to this slab. diff --git a/src/snmalloc/mem/remotecache.h b/src/snmalloc/mem/remotecache.h index 90e1eee55..44eef4981 100644 --- a/src/snmalloc/mem/remotecache.h +++ b/src/snmalloc/mem/remotecache.h @@ -110,6 +110,7 @@ namespace snmalloc // set implies this is used by the backend, and we should not be // deallocating memory here. snmalloc_check_client( + mitigations(sanity_checks), !entry.is_backend_owned(), "Delayed detection of attempt to free internal structure."); if constexpr (Config::Options.QueueHeadsAreTame) diff --git a/src/snmalloc/mem/sizeclasstable.h b/src/snmalloc/mem/sizeclasstable.h index 85ec3d497..203744322 100644 --- a/src/snmalloc/mem/sizeclasstable.h +++ b/src/snmalloc/mem/sizeclasstable.h @@ -225,12 +225,9 @@ namespace snmalloc meta_slow.capacity = static_cast((meta.slab_mask + 1) / rsize); - meta_slow.waking = -#ifdef SNMALLOC_CHECK_CLIENT - static_cast(meta_slow.capacity / 4); -#else + meta_slow.waking = mitigations(random_larger_thresholds) ? + static_cast(meta_slow.capacity / 4) : static_cast(bits::min((meta_slow.capacity / 4), 32)); -#endif if (meta_slow.capacity > max_capacity) { diff --git a/src/snmalloc/pal/pal_apple.h b/src/snmalloc/pal/pal_apple.h index 69f4e5da0..f023e195a 100644 --- a/src/snmalloc/pal/pal_apple.h +++ b/src/snmalloc/pal/pal_apple.h @@ -125,7 +125,7 @@ namespace snmalloc while (madvise(p, size, MADV_FREE_REUSABLE) == -1 && errno == EAGAIN) ; - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { // This must occur after `MADV_FREE_REUSABLE`. // @@ -180,7 +180,7 @@ namespace snmalloc } } - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { // Mark pages as writable for `madvise` below. // @@ -220,7 +220,7 @@ namespace snmalloc // must be initialized to 0 or addr is interepreted as a lower-bound. mach_vm_address_t addr = 0; - vm_prot_t prot = (state_using || !PalEnforceAccess) ? + vm_prot_t prot = (state_using || !mitigations(pal_enforce_access)) ? VM_PROT_READ | VM_PROT_WRITE : VM_PROT_NONE; diff --git a/src/snmalloc/pal/pal_bsd.h b/src/snmalloc/pal/pal_bsd.h index 4689b43c0..59bcdc0ab 100644 --- a/src/snmalloc/pal/pal_bsd.h +++ b/src/snmalloc/pal/pal_bsd.h @@ -40,7 +40,7 @@ namespace snmalloc madvise(p, size, MADV_FREE); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { mprotect(p, size, PROT_NONE); } diff --git a/src/snmalloc/pal/pal_bsd_aligned.h b/src/snmalloc/pal/pal_bsd_aligned.h index 94e9fd5e1..48b28db53 100644 --- a/src/snmalloc/pal/pal_bsd_aligned.h +++ b/src/snmalloc/pal/pal_bsd_aligned.h @@ -38,8 +38,9 @@ namespace snmalloc int log2align = static_cast(bits::next_pow2_bits(size)); - auto prot = - state_using || !PalEnforceAccess ? PROT_READ | PROT_WRITE : PROT_NONE; + auto prot = state_using || !mitigations(pal_enforce_access) ? + PROT_READ | PROT_WRITE : + PROT_NONE; void* p = mmap( nullptr, diff --git a/src/snmalloc/pal/pal_consts.h b/src/snmalloc/pal/pal_consts.h index dde946dc7..83aa52ef2 100644 --- a/src/snmalloc/pal/pal_consts.h +++ b/src/snmalloc/pal/pal_consts.h @@ -7,27 +7,6 @@ namespace snmalloc { - /** - * Pal implementations should query this flag to see whether they - * are allowed to optimise memory access, or that they must provide - * exceptions/segfaults if accesses do not obey the - * - using - * - using_readonly - * - not_using - * model. - * - * TODO: There is a known bug in CheriBSD that means round-tripping through - * PROT_NONE sheds capability load and store permissions (while restoring data - * read/write, for added excitement). For the moment, just force this down on - * CHERI. - */ -#if defined(SNMALLOC_CHECK_CLIENT) && !defined(__CHERI_PURE_CAPABILITY__) && \ - !defined(__NetBSD__) - static constexpr bool PalEnforceAccess = true; -#else - static constexpr bool PalEnforceAccess = false; -#endif - /** * Flags in a bitfield of optional features that a PAL may support. These * should be set in the PAL's `pal_features` static constexpr field. diff --git a/src/snmalloc/pal/pal_freebsd.h b/src/snmalloc/pal/pal_freebsd.h index 5d208c3b4..86a6576e4 100644 --- a/src/snmalloc/pal/pal_freebsd.h +++ b/src/snmalloc/pal/pal_freebsd.h @@ -83,7 +83,7 @@ namespace snmalloc madvise(p, size, MADV_NOCORE); madvise(p, size, MADV_FREE); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { mprotect(p, size, PROT_NONE); } diff --git a/src/snmalloc/pal/pal_linux.h b/src/snmalloc/pal/pal_linux.h index 5e1289a13..6f131b0cc 100644 --- a/src/snmalloc/pal/pal_linux.h +++ b/src/snmalloc/pal/pal_linux.h @@ -128,7 +128,7 @@ namespace snmalloc madvise(p, size, MADV_DONTDUMP); madvise(p, size, madvise_free_flags); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { mprotect(p, size, PROT_NONE); } diff --git a/src/snmalloc/pal/pal_posix.h b/src/snmalloc/pal/pal_posix.h index 8ad79958d..6c9ae05e8 100644 --- a/src/snmalloc/pal/pal_posix.h +++ b/src/snmalloc/pal/pal_posix.h @@ -204,7 +204,7 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) { // Fill memory so that when we switch the pages back on we don't make // assumptions on the content. @@ -232,7 +232,7 @@ namespace snmalloc SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) mprotect(p, size, PROT_READ | PROT_WRITE); else { @@ -253,7 +253,7 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); - if constexpr (PalEnforceAccess) + if constexpr (mitigations(pal_enforce_access)) mprotect(p, size, PROT_READ); else { @@ -326,7 +326,8 @@ namespace snmalloc // If enforcing access, map pages initially as None, and then // add permissions as required. Otherwise, immediately give all // access as this is the most efficient to implement. - auto prot = PalEnforceAccess ? PROT_NONE : PROT_READ | PROT_WRITE; + auto prot = + mitigations(pal_enforce_access) ? PROT_NONE : PROT_READ | PROT_WRITE; void* p = mmap( nullptr, diff --git a/src/test/func/domestication/domestication.cc b/src/test/func/domestication/domestication.cc index f4d2e0671..b8697e731 100644 --- a/src/test/func/domestication/domestication.cc +++ b/src/test/func/domestication/domestication.cc @@ -129,11 +129,8 @@ namespace snmalloc int main() { -# if defined(SNMALLOC_CHECK_CLIENT) - static constexpr bool pagemap_randomize = !aal_supports; -# else - static constexpr bool pagemap_randomize = false; -# endif + static constexpr bool pagemap_randomize = + mitigations(random_pagemap) & !aal_supports; snmalloc::CustomConfig::Pagemap::concretePagemap.init(); snmalloc::CustomConfig::Authmap::init(); diff --git a/src/test/func/pagemap/pagemap.cc b/src/test/func/pagemap/pagemap.cc index 84ba536fa..dca7bf382 100644 --- a/src/test/func/pagemap/pagemap.cc +++ b/src/test/func/pagemap/pagemap.cc @@ -81,11 +81,8 @@ void test_pagemap(bool bounded) } else { -#if defined(SNMALLOC_CHECK_CLIENT) - static constexpr bool pagemap_randomize = !aal_supports; -#else - static constexpr bool pagemap_randomize = false; -#endif + static constexpr bool pagemap_randomize = + mitigations(random_pagemap) && !aal_supports; pagemap_test_unbound.init(); pagemap_test_unbound.register_range(low, high - low);