Cheri-research/snmalloc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor error reporting for bounds checks. (#464)

Browse Source 
This introduces a very limited formatter that can embed strings and hex
representations of pointers / integers in an internal buffer. This is
used to format error strings for passing to `Pal::error`.  This is used,
in turn, by a wrapper for reporting bounds checks, which can be used by
external functions to implement bounds checks.

This removes the sprintf_l usage from the bounds checks.

This provides enough of a format implementation that the tests
introduced in #465 can be refactored to use this, instead of their
custom `printf` wrapper and that can be used by SNMALLOC_CHECK.  This
will be a follow-on PR.
This commit is contained in:
David Chisnall
2022-02-25 09:59:51 +00:00
committed by GitHub
parent 86aa28644c
commit 93efbb4807
6 changed files with 296 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

141
src/ds/helpers.h
View File

@@ -5,6 +5,7 @@
#include <array>
#include <atomic>
#include <string_view>
#include <type_traits>
namespace snmalloc
@@ -249,4 +250,144 @@ namespace snmalloc
static_assert(sizeof(TrivialInitAtomic<char>) == sizeof(char));
static_assert(alignof(TrivialInitAtomic<char>) == alignof(char));
/**
* Helper class for building fatal errors. Used by `report_fatal_error` to
* build an on-stack buffer containing the formatted string.
*/
template<size_t BufferSize>
class FatalErrorBuilder
{
/**
* The buffer that is used to store the formatted output.
*/
std::array<char, BufferSize> buffer;
/**
* Space in the buffer, excluding a trailing null terminator.
*/
static constexpr size_t SafeLength = BufferSize - 1;
/**
* The insert position within `buffer`.
*/
size_t insert = 0;
/**
* Add argument `i` from the tuple `args` to the output. This is
* implemented recursively because the different tuple elements can have
* different types and so the code for dispatching will depend on the type
* at the index. The compiler will lower this to a jump table in optimised
* builds.
*/
template<size_t I, typename... Args>
void add_tuple_arg(size_t i, const std::tuple<Args...>& args)
{
if (i == I)
{
append(std::get<I>(args));
}
else if constexpr (I != 0)
{
add_tuple_arg<I - 1>(i, args);
}
}
/**
* Append a single character into the buffer. This is the single primitive
* operation permitted on the buffer and performs bounds checks to ensure
* that there is space for the character and for a null terminator.
*/
void append_char(char c)
{
if (insert < SafeLength)
{
buffer[insert++] = c;
}
}
/**
* Append a string to the buffer.
*/
void append(std::string_view sv)
{
for (auto c : sv)
{
append_char(c);
}
}
/**
* Append a raw pointer to the buffer as a hex string.
*/
void append(void* ptr)
{
append(static_cast<size_t>(reinterpret_cast<uintptr_t>(ptr)));
}
/**
* Append a size to the buffer, as a hex string.
*/
void append(size_t s)
{
append_char('0');
append_char('x');
std::array<char, sizeof(size_t) * 2> buf;
const char hexdigits[] = "0123456789abcdef";
// Length of string including null terminator
static_assert(sizeof(hexdigits) == 0x11);
for (long i = long(buf.size() - 1); i >= 0; i--)
{
buf[static_cast<size_t>(i)] = hexdigits[s & 0xf];
s >>= 4;
}
bool skipZero = true;
for (auto c : buf)
{
if (skipZero && (c == '0'))
{
continue;
}
skipZero = false;
append_char(c);
}
if (skipZero)
{
append_char('0');
}
}
public:
/**
* Constructor. Takes a format string and the arguments to output.
*/
template<typename... Args>
SNMALLOC_FAST_PATH FatalErrorBuilder(const char* fmt, Args... args)
{
buffer[SafeLength] = 0;
size_t arg = 0;
auto args_tuple = std::forward_as_tuple(args...);
for (const char* s = fmt; *s != 0; ++s)
{
if (s[0] == '{' && s[1] == '}')
{
add_tuple_arg<sizeof...(Args) - 1>(arg++, args_tuple);
++s;
}
else
{
append_char(*s);
}
}
append_char('\0');
}
/**
* Return the error buffer.
*/
const char* get_message()
{
return buffer.data();
}
};
} // namespace snmalloc

108
src/mem/bounds_checks.h Normal file
View File

@@ -0,0 +1,108 @@
#pragma once
#include "../snmalloc.h"
namespace snmalloc
{
/**
* Should we check loads? This defaults to on in debug builds, off in
* release (store-only checks) and can be overridden by defining the macro
* `SNMALLOC_CHECK_LOADS` to true or false.
*/
static constexpr bool CheckReads =
#ifdef SNMALLOC_CHECK_LOADS
SNMALLOC_CHECK_LOADS
#else
DEBUG
#endif
;
/**
* Should we fail fast when we encounter an error? With this set to true, we
* just issue a trap instruction and crash the process once we detect an
* error. With it set to false we print a helpful error message and then crash
* the process. The process may be in an undefined state by the time the
* check fails, so there are potentially security implications to turning this
* off. It defaults to true for debug builds, false for release builds and
* can be overridden by defining the macro `SNMALLOC_FAIL_FAST` to true or
* false.
*/
static constexpr bool FailFast =
#ifdef SNMALLOC_FAIL_FAST
SNMALLOC_FAIL_FAST
#else
!DEBUG
#endif
;
/**
* Report an error message for a failed bounds check and then abort the
* program.
* `p` is the input pointer and `len` is the offset from this pointer of the
* bounds. `msg` is the message that will be reported along with the
* start and end of the real object's bounds.
*/
SNMALLOC_SLOW_PATH SNMALLOC_UNUSED_FUNCTION inline void
report_fatal_bounds_error [[noreturn]] (
void* p, size_t len, const char* msg, decltype(ThreadAlloc::get())& alloc)
{
report_fatal_error(
"{}: {} is in allocation {}--{}, offset {} is past the end\n",
msg,
p,
alloc.template external_pointer<Start>(p),
alloc.template external_pointer<OnePastEnd>(p),
len);
}
/**
* The direction for a bounds check.
*/
enum class CheckDirection
{
/**
* A read bounds check, performed only when read checks are enabled.
*/
Read,
/**
* A write bounds check, performed unconditionally.
*/
Write
};
/**
* Check whether a pointer + length is in the same object as the pointer.
* Fail with the error message from the third argument if not.
*
* The template parameter indicates whether this is a read. If so, this
* function is a no-op when `CheckReads` is false.
*/
template<CheckDirection Direction = CheckDirection::Write>
SNMALLOC_FAST_PATH_INLINE void
check_bounds(const void* ptr, size_t len, const char* msg = "")
{
if constexpr ((Direction == CheckDirection::Write) || CheckReads)
{
auto& alloc = ThreadAlloc::get();
void* p = const_cast<void*>(ptr);
if (SNMALLOC_UNLIKELY(!alloc.check_bounds(ptr, len)))
{
if constexpr (FailFast)
{
UNUSED(p, len, msg);
SNMALLOC_FAST_FAIL();
}
else
{
report_fatal_bounds_error(p, len, msg, alloc);
}
}
}
else
{
UNUSED(ptr, len, msg);
}
}
}

1
src/mem/chunkallocator.h
View File

@@ -1,5 +1,6 @@
#pragma once
#include "../backend/backend_concept.h"
#include "../ds/mpmcstack.h"
#include "../ds/spmcstack.h"
#include "../mem/metaslab.h"

105
src/override/memcpy.cc
View File

@@ -1,55 +1,10 @@
#include "../mem/bounds_checks.h"
#include "override.h"
#include <errno.h>
#include <stdio.h>
#include <string.h>
#if __has_include(<xlocale.h>)
# include <xlocale.h>
#endif
using namespace snmalloc;
// glibc lacks snprintf_l
#if defined(__linux__) || defined(__OpenBSD__) || defined(__DragonFly__) || \
defined(__HAIKU__) || defined(__sun)
# define snprintf_l(buf, size, loc, msg, ...) \
snprintf(buf, size, msg, __VA_ARGS__)
// Windows has it with an underscore prefix
#elif defined(_MSC_VER)
# define snprintf_l(buf, size, loc, msg, ...) \
_snprintf_s_l(buf, size, _TRUNCATE, msg, loc, __VA_ARGS__)
#endif
namespace
{
/**
* Should we check loads? This defaults to on in debug builds, off in
* release (store-only checks)
*/
static constexpr bool CheckReads =
#ifdef SNMALLOC_CHECK_LOADS
SNMALLOC_CHECK_LOADS
#else
DEBUG
#endif
;
/**
* Should we fail fast when we encounter an error? With this set to true, we
* just issue a trap instruction and crash the process once we detect an
* error. With it set to false we print a helpful error message and then crash
* the process. The process may be in an undefined state by the time the
* check fails, so there are potentially security implications to turning this
* off. It defaults to true for debug builds, false for release builds.
*/
static constexpr bool FailFast =
#ifdef SNMALLOC_FAIL_FAST
SNMALLOC_FAIL_FAST
#else
!DEBUG
#endif
;
/**
* The largest register size that we can use for loads and stores. These
* types are expected to work for overlapping copies: we can always load them
@@ -92,62 +47,6 @@ namespace
#endif
}
SNMALLOC_SLOW_PATH SNMALLOC_UNUSED_FUNCTION void crashWithMessage
[[noreturn]] (
void* p, size_t len, const char* msg, decltype(ThreadAlloc::get())& alloc)
{
// We're going to crash the program now, but try to avoid heap
// allocations if possible, since the heap may be in an undefined
// state.
std::array<char, 1024> buffer;
snprintf_l(
buffer.data(),
buffer.size(),
/* Force C locale */ nullptr,
"%s: %p is in allocation %p--%p, offset 0x%zx is past the end.\n",
msg,
p,
alloc.template external_pointer<Start>(p),
alloc.template external_pointer<OnePastEnd>(p),
len);
Pal::error(buffer.data());
}
/**
* Check whether a pointer + length is in the same object as the pointer.
* Fail with the error message from the third argument if not.
*
* The template parameter indicates whether this is a read. If so, this
* function is a no-op when `CheckReads` is false.
*/
template<bool IsRead = false>
SNMALLOC_FAST_PATH_INLINE void
check_bounds(const void* ptr, size_t len, const char* msg = "")
{
if constexpr (!IsRead || CheckReads)
{
auto& alloc = ThreadAlloc::get();
void* p = const_cast<void*>(ptr);
if (SNMALLOC_UNLIKELY(!alloc.check_bounds(ptr, len)))
{
if constexpr (FailFast)
{
UNUSED(p, len, msg);
SNMALLOC_FAST_FAIL();
}
else
{
crashWithMessage(p, len, msg, alloc);
}
}
}
else
{
UNUSED(ptr, len, msg);
}
}
/**
* Copy a block using the specified size. This copies as many complete
* chunks of size `Size` as are possible from `len`.
@@ -205,7 +104,7 @@ namespace
// Check the bounds of the arguments.
check_bounds(
dst, len, "memcpy with destination out of bounds of heap allocation");
check_bounds<true>(
check_bounds<CheckDirection::Read>(
src, len, "memcpy with source out of bounds of heap allocation");
}
// If this is a small size, do byte-by-byte copies.

26
src/pal/pal.h
View File

@@ -142,4 +142,30 @@ namespace snmalloc
# endif
"Page size from system header does not match snmalloc config page size.");
#endif
/**
* Report a fatal error via a PAL-specific error reporting mechanism. This
* takes a format string and a set of arguments. The format string indicates
* the remaining arguments with "{}". This could be extended later to
* support indexing fairly easily, if we ever want to localise these error
* messages.
*
* The following are supported as arguments:
*
* - Characters (`char`), printed verbatim.
* - Strings (anything convertible to `std::string_view`), typically string
* literals because nothing on this path should be performing heap
* allocations. Printed verbatim.
* - Raw pointers (void*), printed as hex strings.
* - Integers (convertible to `size_t`), printed as hex strings.
*
* These types should be sufficient for allocator-related error messages.
*/
template<size_t BufferSize = 1024, typename... Args>
[[noreturn]] inline void report_fatal_error(Args... args)
{
FatalErrorBuilder<BufferSize> msg{std::forward<Args>(args)...};
Pal::error(msg.get_message());
}
} // namespace snmalloc

18
src/test/func/malloc/malloc.cc
View File

@@ -237,6 +237,24 @@ int main(int argc, char** argv)
setup();
// Smoke test the fatal error builder. Check that it can generate strings
// including all of the kinds of things that it expects to be able to format.
void* fakeptr = reinterpret_cast<void*>(static_cast<uintptr_t>(0x42));
FatalErrorBuilder<1024> b{
"testing pointer {} size_t {} message, {} world, null is {}",
fakeptr,
size_t(42),
"hello",
nullptr};
if (
strcmp(
"testing pointer 0x42 size_t 0x2a message, hello world, null is 0x0",
b.get_message()) != 0)
{
printf("Incorrect rendering of fatal error message: %s\n", b.get_message());
abort();
}
our_free(nullptr);
/* A very large allocation size that we expect to fail. */