BUG/MAJOR: backend: fix idle conn crash under low FD
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 24 Oct 2023 16:31:55 +0000 (18:31 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 25 Oct 2023 08:30:45 +0000 (10:30 +0200)
commit394bd4eb39e60d33ed3e2a7449c00153cfe3d351
tree7563df68dea60d61eb96b8f418b4cd2be916f44d
parentb9fbbaf2a88f775f73496c20e944b124292215e5
BUG/MAJOR: backend: fix idle conn crash under low FD

Since the following commit, idle conns are stored in a list as secondary
storage to retrieve them in usage order :
  5afcb686b93c3811bd859a331efd6a8341a61218
  MAJOR: connection: purge idle conn by last usage

The list usage has been extended wherever connections lookup are done
both on idle and safe trees. This reduced the code size by replacing a
two tree loops by a single list loop.

LIST_ELEM() is used in this context to retrieve the first idle list
element from the server list head. However, macro usage was wrong due to
an extra '&' operator which returns an invalid connection reference.
This will most of the time caused a crash on conn_delete_from_tree() or
affiliated functions.

This bug only occurs if the FD pool is exhausted and some idle
connections are selected to be killed.

It can be reproduced using the following config and h2load command :
$ h2load -t 8 -c 800 -m 10 -n 800 "http://127.0.0.1:21080/?s=10k"

global
maxconn 100

defaults
mode http
timeout connect 20s
timeout client  20s
timeout server  20s

listen li
bind :21080 proto h2
server nginx 127.99.0.1:30080 proto h1

This bug has been introduced by the above commit. Thus no need to
backport this fix.

Note that LIST_ELEM() macro usage was slightly adjusted also in
srv_migrate_conns_to_remove(). The function used toremove_list instead
of idle_list connection list element. This is not a bug as they are
stored in the same union. However, the new code is clearer as it intends
to move connection from the idle_list only into the toremove_list
mt-list.
src/backend.c
src/server.c