From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Thu, 1 Jun 2023 10:07:43 +0000 (+0200)
Subject: BUG/MINOR: proxy/server: free default-server on deinit
X-Git-Tag: v2.9-dev1~64
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b7f8af3ca9424984e557f2c95c639dd4f57dfe61;p=thirdparty%2Fhaproxy.git

BUG/MINOR: proxy/server: free default-server on deinit

proxy default-server is a specific type of server that is not allocated
using new_server(): it is directly stored within the parent proxy
structure. However, since it may contain some default config options that
may be inherited by regular servers, it is also subject to dynamic members
(strings, structures..) that needs to be deallocated when the parent proxy
is cleaned up.

Unfortunately, srv_drop() may not be used directly from p->defsrv since
this function is meant to be used on regular servers only (those created
using new_server()).

To circumvent this, we're splitting srv_drop() to make a new function
called srv_free_params() that takes care of the member cleaning which
originally takes place in srv_drop(). This function is exposed through
server.h, so it may be called from outside server.c.

Thanks to this, calling srv_free_params(&p->defsrv) from free_proxy()
prevents any memory leaks due to dynamic parameters allocated when
parsing a default-server line from a proxy section.

This partially fixes GH #2173 and may be backported to 2.8.

[While it could also be relevant for other stable versions, the patch
won't apply due to architectural changes / name changes between 2.4 => 2.6
and then 2.6 => 2.8. Considering this is a minor fix that only makes
memory analyzers happy during deinit paths (at least for <= 2.8), it might
not be worth the trouble to backport them any further?]
---

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 4c382fa695..3829bce2be 100644
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -62,6 +62,7 @@ struct server *cli_find_server(struct appctx *appctx, char *arg);
 struct server *new_server(struct proxy *proxy);
 void srv_take(struct server *srv);
 struct server *srv_drop(struct server *srv);
+void srv_free_params(struct server *srv);
 int srv_init_per_thr(struct server *srv);
 void srv_set_ssl(struct server *s, int use_ssl);
 const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
diff --git a/src/proxy.c b/src/proxy.c
index bc086865c0..a15acc13b4 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -310,6 +310,11 @@ void free_proxy(struct proxy *p)
 		s = srv_drop(s);
 	}/* end while(s) */
 
+	/* also free default-server parameters since some of them might have
+	 * been dynamically allocated (e.g.: config hints, cookies, ssl..)
+	 */
+	srv_free_params(&p->defsrv);
+
 	list_for_each_entry_safe(l, l_next, &p->conf.listeners, by_fe) {
 		LIST_DELETE(&l->by_fe);
 		LIST_DELETE(&l->by_bind);
diff --git a/src/server.c b/src/server.c
index eefb3bccce..1eb13c42e9 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2465,6 +2465,24 @@ void srv_take(struct server *srv)
 	HA_ATOMIC_INC(&srv->refcount);
 }
 
+/* deallocate common server parameters (may be used by default-servers) */
+void srv_free_params(struct server *srv)
+{
+	free(srv->cookie);
+	free(srv->hostname);
+	free(srv->hostname_dn);
+	free((char*)srv->conf.file);
+	free(srv->per_thr);
+	free(srv->per_tgrp);
+	free(srv->curr_idle_thr);
+	free(srv->resolvers_id);
+	free(srv->addr_node.key);
+	free(srv->lb_nodes);
+
+	if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->destroy_srv)
+		xprt_get(XPRT_SSL)->destroy_srv(srv);
+}
+
 /* Deallocate a server <srv> and its member. <srv> must be allocated. For
  * dynamic servers, its refcount is decremented first. The free operations are
  * conducted only if the refcount is nul.
@@ -2498,19 +2516,8 @@ struct server *srv_drop(struct server *srv)
 	task_destroy(srv->srvrq_check);
 
 	free(srv->id);
-	free(srv->cookie);
-	free(srv->hostname);
-	free(srv->hostname_dn);
-	free((char*)srv->conf.file);
-	free(srv->per_thr);
-	free(srv->per_tgrp);
-	free(srv->curr_idle_thr);
-	free(srv->resolvers_id);
-	free(srv->addr_node.key);
-	free(srv->lb_nodes);
+	srv_free_params(srv);
 
-	if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->destroy_srv)
-		xprt_get(XPRT_SSL)->destroy_srv(srv);
 	HA_SPIN_DESTROY(&srv->lock);
 
 	LIST_DELETE(&srv->global_list);