From: Willy Tarreau <w@1wt.eu>
Date: Sat, 17 Apr 2021 16:19:51 +0000 (+0200)
Subject: MINOR: pools: move pool_free_area() out of the lock in the locked version
X-Git-Tag: v2.4-dev17~63
X-Git-Url: http://git.haproxy.org/?a=commitdiff_plain;h=acf0c5449157a4d6d8cc43e753fc59479414432a;p=haproxy-3.0.git

MINOR: pools: move pool_free_area() out of the lock in the locked version

Calling pool_free_area() inside a lock in pool_put_to_shared_cache() is
a very bad idea. Fortunately this only happens on the lowest end platforms
which almost never use threads or in very small counts.

This change consists in zeroing the pointer once already released to the
cache in the first test so that the second stage knows if it needs to
pass it to the OS or not. This has slightly reduced the length of the
---

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index a440625..8fce593 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -237,26 +237,31 @@ static inline void *pool_get_from_shared_cache(struct pool_head *pool)
  */
 static inline void pool_put_to_shared_cache(struct pool_head *pool, void *ptr)
 {
-#ifndef DEBUG_UAF /* normal pool behaviour */
 	_HA_ATOMIC_DEC(&pool->used);
+
+#ifndef DEBUG_UAF /* normal pool behaviour */
+
 	HA_SPIN_LOCK(POOL_LOCK, &pool->lock);
-	if (pool_is_crowded(pool)) {
-		pool_free_area(ptr, pool->size + POOL_EXTRA);
-		_HA_ATOMIC_DEC(&pool->allocated);
-	} else {
+	if (!pool_is_crowded(pool)) {
 		*POOL_LINK(pool, ptr) = (void *)pool->free_list;
 		pool->free_list = (void *)ptr;
+		ptr = NULL;
 	}
-	swrate_add(&pool->needed_avg, POOL_AVG_SAMPLES, pool->used);
 	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*/
+
+#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;
-	pool_free_area(ptr, pool->size + POOL_EXTRA);
-	_HA_ATOMIC_DEC(&pool->allocated);
-	_HA_ATOMIC_DEC(&pool->used);
-	swrate_add(&pool->needed_avg, POOL_AVG_SAMPLES, pool->used);
+
 #endif /* DEBUG_UAF */
+
+	if (ptr) {
+		/* still not freed */
+		pool_free_area(ptr, pool->size + POOL_EXTRA);
+		_HA_ATOMIC_DEC(&pool->allocated);
+	}
+	swrate_add(&pool->needed_avg, POOL_AVG_SAMPLES, pool->used);
 }
 
 #endif /* CONFIG_HAP_LOCKLESS_POOLS */