diff --git a/CMakeLists.txt b/CMakeLists.txt index d44a456a..7aa337ef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,6 +17,7 @@ option(BUILD_TESTING "Build test programs as well as shims" ON) option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF) # Options that apply globally +option(SNMALLOC_CLEAN_POINTERS "Zero freelist pointers in allocations before returning to user." OFF) option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory on Linux" Off) option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) @@ -221,6 +222,7 @@ function(add_as_define FLAG) target_compile_definitions(snmalloc INTERFACE $<$:${FLAG}>) endfunction() +add_as_define(SNMALLOC_CLEAN_POINTERS) add_as_define(SNMALLOC_QEMU_WORKAROUND) add_as_define(SNMALLOC_TRACING) add_as_define(SNMALLOC_CI_BUILD) diff --git a/src/snmalloc/backend_helpers/largebuddyrange.h b/src/snmalloc/backend_helpers/largebuddyrange.h index d1446d72..cb73bd19 100644 --- a/src/snmalloc/backend_helpers/largebuddyrange.h +++ b/src/snmalloc/backend_helpers/largebuddyrange.h @@ -134,6 +134,11 @@ namespace snmalloc return k; } + static void clear(Contents k) + { + UNUSED(k); + } + /** * Convert the pointer wrapper into something that the snmalloc debug * printing code can print. diff --git a/src/snmalloc/backend_helpers/smallbuddyrange.h b/src/snmalloc/backend_helpers/smallbuddyrange.h index 83796e1e..641c8cd9 100644 --- a/src/snmalloc/backend_helpers/smallbuddyrange.h +++ b/src/snmalloc/backend_helpers/smallbuddyrange.h @@ -146,6 +146,22 @@ namespace snmalloc UNUSED(k, size); return true; } + + /** + * Erases the pointers stored in this node when we are removing it from + * the RBTree. Useful so that returned memory does not leak internal + * pointers. + * @ptr must not be NULL and must not point to a NULL node. + */ + static void clear(Contents p) + { +#ifdef SNMALLOC_CLEAN_POINTERS + p->left = nullptr; + p->right = nullptr; +#else + UNUSED(p); +#endif + } }; struct SmallBuddyRange diff --git a/src/snmalloc/ds_core/redblacktree.h b/src/snmalloc/ds_core/redblacktree.h index 0d684698..7e85e464 100644 --- a/src/snmalloc/ds_core/redblacktree.h +++ b/src/snmalloc/ds_core/redblacktree.h @@ -76,6 +76,10 @@ namespace snmalloc } } ->ConceptSame; + { + Rep::clear(k) + } + ->ConceptSame; }; template @@ -479,6 +483,7 @@ namespace snmalloc bool remove_path(RBPath& path) { ChildRef splice = path.curr(); + K removed = path.curr(); SNMALLOC_ASSERT(!(splice.is_null())); debug_log("Removing", path); @@ -518,7 +523,8 @@ namespace snmalloc debug_log("Splice done", path); - // TODO: Clear node contents? + // Clear node contents + Rep::clear(removed); // Red leaf removal requires no rebalancing. if (leaf_red) diff --git a/src/snmalloc/mem/freelist.h b/src/snmalloc/mem/freelist.h index 2830f9b8..0fda358e 100644 --- a/src/snmalloc/mem/freelist.h +++ b/src/snmalloc/mem/freelist.h @@ -185,12 +185,14 @@ namespace snmalloc /** * 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. + * important on CHERI to avoid leaking capabilities. On other + * architectures it might be desirable to avoid leaking these pointers, + * especially if CHECK_CLIENT is on because it might increase the + * difficulty to bypass the checks. */ void cleanup() { -#if defined(__CHERI_PURE_CAPABILITY__) || defined(SNMALLOC_CHECK_CLIENT) +#ifdef SNMALLOC_CLEAN_POINTERS this->next_object = nullptr; # ifdef SNMALLOC_CHECK_CLIENT this->prev_encoded = 0; diff --git a/src/snmalloc/mem/remoteallocator.h b/src/snmalloc/mem/remoteallocator.h index 2b92e9f6..a3ba6384 100644 --- a/src/snmalloc/mem/remoteallocator.h +++ b/src/snmalloc/mem/remoteallocator.h @@ -149,6 +149,7 @@ namespace snmalloc break; // We want this element next, so start it loading. Aal::prefetch(next.unsafe_ptr()); + curr->cleanup(); if (SNMALLOC_UNLIKELY(!cb(curr))) { /* diff --git a/src/test/func/redblack/redblack.cc b/src/test/func/redblack/redblack.cc index f13c72eb..a8c58d02 100644 --- a/src/test/func/redblack/redblack.cc +++ b/src/test/func/redblack/redblack.cc @@ -12,7 +12,8 @@ # define SNMALLOC_TRACING #endif // Redblack tree needs some libraries with trace enabled. -#include "snmalloc/snmalloc.h" +#include "snmalloc/ds_core/ds_core.h" +#include "snmalloc/pal/pal.h" struct NodeRef { @@ -102,6 +103,17 @@ public: array[k].left ^= 1; } + static void clear(key k) + { + array[k].left = 0; + array[k].right = 0; + } + + static bool is_clear(key k) + { + return (array[k].left == 0) && (array[k].right == 0); + } + static bool compare(key k1, key k2) { return k1 > k2; @@ -169,6 +181,12 @@ void test(size_t size, unsigned int seed) std::cout << "Failed to remove element: " << elem << std::endl; abort(); } + if (!Rep::is_clear(elem)) + { + std::cout << "Removed entry is not cleared " << elem << " @ " + << &array[elem] << std::endl; + abort(); + } entries.erase(entries.begin() + static_cast(index)); } }