Factor checks under separate feature flags. (#587)

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 <nfilardo@microsoft.com>
microsoft · Mar 23, 2023 · ccca98a · ccca98a
1 parent 848db74
commit ccca98a
Show file tree

Hide file tree

Showing 25 changed files with 753 additions and 435 deletions.
diff --git 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
@@ -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)
     $<$<PLATFORM_ID:Windows>:$<${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
@@ -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
@@ -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<Pal, Pagemap, Base>;
-#  else
-    using LocalState = StandardLocalState<Pal, Pagemap, Base>;
-#  endif
+    using LocalState = std::conditional_t<
+      mitigations(metadata_protection),
+      MetaProtectedRangeLocalState<Pal, Pagemap, Base>,
+      StandardLocalState<Pal, Pagemap, Base>>;
 
     /**
      * 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<StrictProvenance>;
-#  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<StrictProvenance>;
 
       Pagemap::concretePagemap.template init<pagemap_randomize>();
 

diff --git 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
@@ -213,6 +213,10 @@ namespace snmalloc
       }
       else
       {
+        if constexpr (pal_supports<LazyCommit, PAL>)
+        {
+          PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE);
+        }
         new_body = static_cast<T*>(new_body_untyped);
       }
       // Ensure bottom page is committed