From: Willy Tarreau <w@1wt.eu>
Date: Fri, 22 Oct 2021 14:00:02 +0000 (+0200)
Subject: BUG/MEDIUM: lua: fix memory leaks with realloc() on non-glibc systems
X-Git-Tag: v2.5-dev11~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5efdff93c36f75345a2a18f18bffee9b602bc7b;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: lua: fix memory leaks with realloc() on non-glibc systems

In issue #1406, Lev Petrushchak reported a nasty memory leak on Alpine
since haproxy 2.4 when using Lua, that memory profiling didn't detect.
After inspecting the code and Lua's code, it appeared that Lua's default
allocator does an explicit free() on size zero, while since 2.4 commit
d36c7fa5e ("MINOR: lua: simplify hlua_alloc() to only rely on realloc()"),
haproxy only calls realloc(ptr,0) that performs a free() on glibc but not
on other systems as it's not required by POSIX...

This patch reinstalls the explicit test for nsize==0 to call free().

Thanks to Lev for the very documented report, and to Tim for the links
to a musl thread on the same subject that confirms the diagnostic.

This must be backported to 2.4.
---

diff --git a/src/hlua.c b/src/hlua.c
index 758341a4f6..ac61a3171b 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11451,6 +11451,12 @@ int hlua_post_init()
  * indicated by <ptr> being NULL. A free is indicated by <nsize> being
  * zero. This one verifies that the limits are respected but is optimized
  * for the fast case where limits are not used, hence stats are not updated.
+ *
+ * Warning: while this API ressembles glibc's realloc() a lot, glibc surpasses
+ * POSIX by making realloc(ptr,0) an effective free(), but others do not do
+ * that and will simply allocate zero as if it were the result of malloc(0),
+ * so mapping this onto realloc() will lead to memory leaks on non-glibc
+ * systems.
  */
 static void *hlua_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
 {
@@ -11463,8 +11469,13 @@ static void *hlua_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
 	/* a limit of ~0 means unlimited and boot complete, so there's no need
 	 * for accounting anymore.
 	 */
-	if (likely(~zone->limit == 0))
-		return realloc(ptr, nsize);
+	if (likely(~zone->limit == 0)) {
+		if (!nsize)
+			ha_free(&ptr);
+		else
+			ptr = realloc(ptr, nsize);
+		return ptr;
+	}
 
 	if (!ptr)
 		osize = 0;
@@ -11478,7 +11489,10 @@ static void *hlua_alloc(void *ud, void *ptr, size_t osize, size_t nsize)
 			return NULL;
 	} while (!_HA_ATOMIC_CAS(&zone->allocated, &old, new));
 
-	ptr = realloc(ptr, nsize);
+	if (!nsize)
+		ha_free(&ptr);
+	else
+		ptr = realloc(ptr, nsize);
 
 	if (unlikely(!ptr && nsize)) // failed
 		_HA_ATOMIC_SUB(&zone->allocated, nsize - osize);