From b5ee8bebfc9f7cba6063b7e3424f96c0d7ed3d47 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Thu, 9 Mar 2023 14:28:00 +0100 Subject: [PATCH] MINOR: server: always call ssl->destroy_srv when available In srv_drop(), we only call the ssl->destroy_srv() method on specific conditions. But this has two downsides: First, destroy_srv() is reponsible for freeing data that may have been allocated in prepare_srv(), but not exclusively: it also frees ssl-related parameters allocated when parsing a server entry, such as ca-file for instance. So this is quite error-prone, we could easily miss a condition where some data needs to be deallocated using destroy_srv() even if prepare_srv() was not used (since prepare_srv() is also conditional), thus resulting in memory leaks. Moreover, depending on srv->proxy to guard the check is probably not a good idea here, since srv_drop() could be called in late de-init paths in which related proxy could be freed already. srv_drop() should only take care of freeing local server data without external logic. Thankfully, destroy_srv() function performs the necessary checks to ensure that a systematic call to the function won't result in invalid reads or double frees. No backport needed. --- src/server.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/server.c b/src/server.c index 1b52f36e60..9c12288126 100644 --- a/src/server.c +++ b/src/server.c @@ -2452,10 +2452,8 @@ struct server *srv_drop(struct server *srv) free(srv->addr_node.key); free(srv->lb_nodes); - if (srv->use_ssl == 1 || srv->check.use_ssl == 1 || (srv->proxy->options & PR_O_TCPCHK_SSL)) { - if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->destroy_srv) - xprt_get(XPRT_SSL)->destroy_srv(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); -- 2.39.5