From 9e2a22d8563e86e67ead5fa3a6a0cec49957b83b Mon Sep 17 00:00:00 2001 From: Andy Heffernan <ahh@meraki.com> Date: Thu, 31 Aug 2017 16:48:48 -0700 Subject: [PATCH] HashAllocator: poison freed blocks When the Click build is configured with the option --enable-hash-allocator-poisoning, this change will cause the HashAllocator to write a "poison" byte value to the block being returned to a HashAllocator pool. This ensures that when a stale reference to a freed block is followed, the code will be much less likely to interpret the block as a valid object or struct. In particular, pointer values will be non-NULL but bad, leading to immediate failure with a clear signature indicating the presence of a stale reference bug. Signed-off-by: Andy Heffernan <ahh@meraki.com> --- config-linuxmodule.h.in | 3 +++ config-userlevel.h.in | 3 +++ configure.in | 5 +++++ include/click/hashallocator.hh | 8 ++++++++ 4 files changed, 19 insertions(+) diff --git a/config-linuxmodule.h.in b/config-linuxmodule.h.in index cd88e4f89..e9d1e5300 100644 --- a/config-linuxmodule.h.in +++ b/config-linuxmodule.h.in @@ -178,6 +178,9 @@ /* Define to 1 if Linux defines the type 'uintptr_t'. */ #undef HAVE_UINTPTR_T_LINUXMODULE +/* Define to 1 to enable poisoning of freed HashAllocator blocks. */ +#undef HAVE_HASH_ALLOCATOR_POISONING + /* The size of a `click_jiffies_t', as computed by sizeof. */ #define SIZEOF_CLICK_JIFFIES_T SIZEOF_LONG diff --git a/config-userlevel.h.in b/config-userlevel.h.in index 7bf180e7e..68be606eb 100644 --- a/config-userlevel.h.in +++ b/config-userlevel.h.in @@ -316,6 +316,9 @@ /* Define if you have the <valgrind/memcheck.h> header file. */ #undef HAVE_VALGRIND_MEMCHECK_H +/* Define to 1 to enable poisoning of freed HashAllocator blocks. */ +#undef HAVE_HASH_ALLOCATOR_POISONING + /* Define if you have the vsnprintf function. */ #undef HAVE_VSNPRINTF diff --git a/configure.in b/configure.in index 57f3995be..f9391acb0 100644 --- a/configure.in +++ b/configure.in @@ -1482,6 +1482,11 @@ if test "$value" != 0; then AC_DEFINE_UNQUOTED([CLICK_DEBUG_SCHEDULING], [$value], [Define to enable debugging support for Click scheduling.]) fi +AC_ARG_ENABLE(hash-allocator-poisoning, [ --enable-hash-allocator-poisoning enable HashAllocator block poisoning], :, enable_hash_allocator_poisoning=no) +if test $enable_hash_allocator_poisoning = yes; then + AC_DEFINE(HAVE_HASH_ALLOCATOR_POISONING) +fi + dnl Compile for the native architecture AC_ARG_ENABLE(portable-binary, [AS_HELP_STRING([--enable-portable-binary], [disable compiler optimizations that would produce unportable binaries])], diff --git a/include/click/hashallocator.hh b/include/click/hashallocator.hh index 38d55cef0..8d8d88c63 100644 --- a/include/click/hashallocator.hh +++ b/include/click/hashallocator.hh @@ -23,6 +23,11 @@ class HashAllocator { public: private: +#if HAVE_HASH_ALLOCATOR_POISONING + // Freed blocks are poisoned with this byte value. + static const uint8_t poison_byte = 0x0d; +#endif + struct link { link *next; }; @@ -91,6 +96,9 @@ inline void *HashAllocator::allocate() inline void HashAllocator::deallocate(void *p) { if (p) { +#if HAVE_HASH_ALLOCATOR_POISONING + memset(p, poison_byte, _size); +#endif reinterpret_cast<link *>(p)->next = _free; _free = reinterpret_cast<link *>(p); #ifdef VALGRIND_MEMPOOL_FREE -- GitLab