Introduce a new compilation option to zero inline metadata pointers in

allocations before returning to user. This is important on CHERI to
avoid leaking capabilities and may also reduce the attack surface on
other architecutres. This includes: Freelist pointers. RBTree metadata
used by smallbuddyallocator.
This commit is contained in:
Robert Norton 2022-04-04 10:16:10 +01:00 коммит произвёл Matthew Parkinson
Родитель e5d2ac95da
Коммит 255f54729b
7 изменённых файлов: 55 добавлений и 5 удалений

Просмотреть файл

@ -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 $<$<BOOL:${${FLAG}}>:${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)

Просмотреть файл

@ -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.

Просмотреть файл

@ -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

Просмотреть файл

@ -76,6 +76,10 @@ namespace snmalloc
}
}
->ConceptSame<typename Rep::Handle>;
{
Rep::clear(k)
}
->ConceptSame<void>;
};
template<typename Rep>
@ -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)

Просмотреть файл

@ -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;

Просмотреть файл

@ -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)))
{
/*

Просмотреть файл

@ -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<int>(index));
}
}