From 736151556c1e2e892bc067736f4aa1205d386208 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Mon, 12 May 2025 16:09:15 +0200 Subject: [PATCH] BUG/MINOR: server: dont depend on proxy for server cleanup in srv_drop() In commit b5ee8bebfc ("MINOR: server: always call ssl->destroy_srv when available"), we made it so srv_drop() doesn't depend on proxy to perform server cleanup. It turns out this is now mandatory, because during deinit, free_proxy() can occur before the final srv_drop(). This is the case when using Lua scripts for instance. In 2a9436f96 ("MINOR: lbprm: Add method to deinit server and proxy") we added a freeing check under srv_drop() that depends on the proxy. Because of that UAF may occur during deinit when using a Lua script that manipulate server objects. To fix the issue, let's perform the lbprm server deinit logic under free_proxy() directly, where the DEINIT server hooks are evaluated. Also, to prevent similar bugs in the future, let's explicitly document in srv_drop() that server cleanups should assume that the proxy may already be freed. No backport needed unless 2a9436f96 is. --- src/proxy.c | 4 ++++ src/server.c | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/proxy.c b/src/proxy.c index 1b4b9367d..326ff3eb6 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -371,6 +371,10 @@ void deinit_proxy(struct proxy *p) while (s) { list_for_each_entry(srvdf, &server_deinit_list, list) srvdf->fct(s); + + if (p->lbprm.server_deinit) + p->lbprm.server_deinit(s); + s = srv_drop(s); }/* end while(s) */ diff --git a/src/server.c b/src/server.c index 6b984029a..8ccb1bc42 100644 --- a/src/server.c +++ b/src/server.c @@ -3113,6 +3113,9 @@ void srv_free_params(struct server *srv) * dynamic servers, its refcount is decremented first. The free operations are * conducted only if the refcount is nul. * + * A general rule is to assume that proxy may already be freed, so cleanup checks + * must not depend on the proxy + * * As a convenience, is returned if srv is not NULL. It may be useful * when calling srv_drop on the list of servers. */ @@ -3156,9 +3159,6 @@ struct server *srv_drop(struct server *srv) EXTRA_COUNTERS_FREE(srv->extra_counters); - if (srv->proxy->lbprm.server_deinit) - srv->proxy->lbprm.server_deinit(srv); - ha_free(&srv); end: -- 2.39.5