]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: server: always call ssl->destroy_srv when available
authorAurelien DARRAGON <adarragon@haproxy.com>
Thu, 9 Mar 2023 13:28:00 +0000 (14:28 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 5 Apr 2023 06:58:16 +0000 (08:58 +0200)
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

index 1b52f36e6076eda034ae0c80425488bb9d72d94d..9c12288126d2c5166acb181f0e04b17100e7c9b0 100644 (file)
@@ -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);