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 9b808420513588bb9eb011b4b5486516a6c215ee..9aec880bc82f2107127f71d252e9c8d8b376edcd 100644 (file)
--- 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 8d9c17fd5b26e9b369a41e99028d70a651f5ebb5..cc92245c7a8354257dded12af169f4e2e2a2b928 100644 (file)
--- 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);