mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-06 15:17:01 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: server: dont depend on proxy for server cleanup in srv_drop()

Browse Source 
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.
This commit is contained in:
Aurelien DARRAGON 2025-05-12 16:09:15 +02:00
parent be4d816be2
commit 736151556c
2 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/proxy.c
View File

@ -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) */

6
src/server.c
View File

@ -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, <srv.next> 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: