From: Willy Tarreau <w@1wt.eu>
Date: Wed, 9 Feb 2022 15:49:16 +0000 (+0100)
Subject: DEBUG: pools: replace the link pointer with the caller's address on pool_free()
X-Git-Tag: v2.6-dev2~156
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=27c8da1fd5e4f906fd3a9f96a4284c886b559305;p=thirdparty%2Fhaproxy.git

DEBUG: pools: replace the link pointer with the caller's address on pool_free()

Along recent evolutions of the pools, we've lost the ability to reliably
detect double-frees because while in the past the same pointer was being
used to chain the objects in the cache and to store the pool's address,
since 2.0 they're different so the pool's address is never overwritten on
free() and a double-free will rarely be detected.

This patch sets the caller's return address there. It can never be equal
to a pool's address and will help guess what was the previous call path.
It will not work on exotic architectures nor with very old compilers but
these are not the environments where we're trying to get detailed bug
reports, and this is not done by default anyway so we don't care about
this limitation. Note that depending on the inlining status of the
function, the result may differ but that's no big deal either.

A test by placing a double free of an appctx inside the release handler
itself successfully reported the trouble during appctx_free() and showed
that the return address was in stream_int_shutw_applet() (this one calls
the release handler).
---

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index 9b80842051..9aec880bc8 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -62,6 +62,13 @@
 		*(typeof(pool)*)(((char *)__i) + __p->size) = __p;	\
 	} while (0)
 
+# define POOL_DEBUG_RESET_MARK(pool, item)				\
+	do {								\
+		typeof(pool) __p = (pool);				\
+		typeof(item) __i = (item);				\
+		*(typeof(pool)*)(((char *)__i) + __p->size) = __builtin_return_address(0); \
+	} while (0)
+
 # define POOL_DEBUG_CHECK_MARK(pool, item)				\
 	do {								\
 		typeof(pool) __p = (pool);				\
@@ -74,6 +81,7 @@
 
 # define POOL_EXTRA_MARK (0)
 # define POOL_DEBUG_SET_MARK(pool, item)   do { } while (0)
+# define POOL_DEBUG_RESET_MARK(pool, item) do { } while (0)
 # define POOL_DEBUG_CHECK_MARK(pool, item) do { } while (0)
 
 #endif // DEBUG_MEMORY_POOLS
diff --git a/src/pool.c b/src/pool.c
index 8d9c17fd5b..cc92245c7a 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -653,6 +653,7 @@ void __pool_free(struct pool_head *pool, void *ptr)
 #endif
 	/* we'll get late corruption if we refill to the wrong pool or double-free */
 	POOL_DEBUG_CHECK_MARK(pool, ptr);
+	POOL_DEBUG_RESET_MARK(pool, ptr);
 
 	if (unlikely(mem_poison_byte >= 0))
 		memset(ptr, mem_poison_byte, pool->size);