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 <[email protected]>
tbarbette · May 20, 2023 · 9e2a22d · 9e2a22d
1 parent 5c77c0b
commit 9e2a22d
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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