]> git.ipfire.org Git - thirdparty/haproxy.git/commit
BUG/MEDIUM: server/cli: don't delete a dynamic server that has streams
authorAurelien DARRAGON <adarragon@haproxy.com>
Thu, 21 Sep 2023 12:18:50 +0000 (14:18 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Thu, 21 Sep 2023 12:57:01 +0000 (14:57 +0200)
commit95c4d24825b64df03e20559b8d79eea11f1f52d0
tree1879b09fe102c87cdd0a0fe2904ca526540624d8
parent0189a4679e25929b87fe37eea5c02d0c59a73440
BUG/MEDIUM: server/cli: don't delete a dynamic server that has streams

In cli_parse_delete_server(), we take care of checking that the server is
in MAINT and that the cur_sess counter is set to 0, in the hope that no
connection/stream ressources continue to point to the server, else we
refuse to delete it.

As shown in GH #2298, this is not sufficient.

Indeed, when the server option "on-marked-down shutdown-sessions" is not
used, server streams are not purged when srv enters maintenance mode.

As such, there could be remaining streams that point to the server. To
detect this, a secondary check on srv->cur_sess counter was performed in
cli_parse_delete_server(). Unfortunately, there are some code paths that
could lead to cur_sess being decremented, and not resulting in a stream
being actually shutdown. As such, if the delete_server cli is handled
right after cur_sess has been decremented with streams still pointing to
the server, we could face some nasty bugs where stream->srv_conn could
point to garbage memory area, as described in the original github report.

To make the check more reliable prior to deleting the server, we don't
rely exclusively on cur_sess and directly check that the server is not
used in any stream through the srv_has_stream() helper function.

Thanks to @capflam which found out the root cause for the bug and greatly
helped to provide the fix.

This should be backported up to 2.6.
src/server.c