From: Willy Tarreau <w@1wt.eu>
Date: Thu, 10 Jun 2021 15:20:19 +0000 (+0200)
Subject: BUG/MINOR: pools: make DEBUG_UAF always write to the to-be-freed location
X-Git-Tag: v2.5-dev1~166
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9a7aa3b4a19e2117a2d31257388933f5fc0c6d4a;p=thirdparty%2Fhaproxy.git

BUG/MINOR: pools: make DEBUG_UAF always write to the to-be-freed location

Since the code was reorganized, DEBUG_UAF was still tested in the locked
pool code despite pools being disabled when DEBUG_UAF is used. Let's move
the test to pool_put_to_os() which is the one that is always called in
this condition.

The impact is only a possible misleading analysis during a troubleshooting
session due to a missing double-frees or free of const area test that is
normally already dealt with by the underlying code anyway. In practice it's
unlikely anyone will ever notice.

This should only be backported to 2.4.
---

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index 4d13d4c2f8..5cbd84cf59 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -194,8 +194,6 @@ static inline void pool_put_to_shared_cache(struct pool_head *pool, void *ptr)
 {
 	_HA_ATOMIC_DEC(&pool->used);
 
-#ifndef DEBUG_UAF /* normal pool behaviour */
-
 	HA_SPIN_LOCK(POOL_LOCK, &pool->lock);
 	if (!pool_is_crowded(pool)) {
 		*POOL_LINK(pool, ptr) = (void *)pool->free_list;
@@ -204,13 +202,6 @@ static inline void pool_put_to_shared_cache(struct pool_head *pool, void *ptr)
 	}
 	HA_SPIN_UNLOCK(POOL_LOCK, &pool->lock);
 
-#else
-	/* release the entry for real to detect use after free */
-	/* ensure we crash on double free or free of a const area */
-	*(uint32_t *)ptr = 0xDEADADD4;
-
-#endif /* DEBUG_UAF */
-
 	if (ptr) {
 		/* still not freed */
 		pool_put_to_os(pool, ptr);
diff --git a/src/pool.c b/src/pool.c
index 827cab457d..d788c6168a 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -147,6 +147,14 @@ void *pool_get_from_os(struct pool_head *pool)
  */
 void pool_put_to_os(struct pool_head *pool, void *ptr)
 {
+#ifdef DEBUG_UAF
+	/* This object will be released for real in order to detect a use after
+	 * free. We also force a write to the area to ensure we crash on double
+	 * free or free of a const area.
+	 */
+	*(uint32_t *)ptr = 0xDEADADD4;
+#endif /* DEBUG_UAF */
+
 	pool_free_area(ptr, pool->size + POOL_EXTRA);
 	_HA_ATOMIC_DEC(&pool->allocated);
 }