diff --git a/backport-BUG-MEDIUM-server-addr-fix-tune.events.max-events-at.patch b/backport-BUG-MEDIUM-server-addr-fix-tune.events.max-events-at.patch new file mode 100644 index 0000000000000000000000000000000000000000..1b41a7597d95cc8f128d485087f2aa9485bd2647 --- /dev/null +++ b/backport-BUG-MEDIUM-server-addr-fix-tune.events.max-events-at.patch @@ -0,0 +1,136 @@ +From d9b6fa6b251b2db654ac42b623b86c06735978bd Mon Sep 17 00:00:00 2001 +From: Aurelien DARRAGON +Date: Tue, 6 Aug 2024 14:29:56 +0200 +Subject: [PATCH] BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once + event miss and leak + +An issue has been introduced with cd99440 ("BUG/MAJOR: server/addr: fix +a race during server addr:svc_port updates"). + +Indeed, in the above commit we implemented the atomic_sync task which is +responsible for consuming pending server events to apply the changes +atomically. For now only server's addr updates are concerned. + +To prevent the task from causing contention, a budget was assigned to it. +It can be controlled with the global tunable +'tune.events.max-events-at-once': the task may not process more than this +number of events at once. + +However, a bug was introduced with this budget logic: each time the task +has to be interrupted because it runs out of budget, we reschedule the +task to finish where it left off, but the current event which was already +removed from the queue wasn't processed yet. This means that this pending +event (each tune.events.max-events-at-once) is effectively lost. + +When the atomic_sync task deals with large number of concurrent events, +this bug has 2 known consequences: first a server's addr/port update +will be lost every 'tune.events.max-events-at-once'. This can of course +cause reliability issues because if the event is not republished +periodically, the server could stay in a stale state for indefinite amount +of time. This is the case when the DNS server flaps for instance: some +servers may not come back UP after the incident as described in GH #2666. + +Another issue is that the lost event was not cleaned up, resulting in a +small memory leak. So in the end, it means that the bug is likely to +cause more and more degradation over time until haproxy is restarted. + +As a workaround, 'tune.events.max-events-at-once' may be set to the +maximum number of events expected per batch. Note however that this value +cannot exceed 10 000, otherwise it could cause the watchdog to trigger due +to the task being busy for too long and preventing other threads from +making any progress. Setting higher values may not be optimal for common +workloads so it should only be used to mitigate the bug while waiting for +this fix. + +Since tune.events.max-events-at-once defaults to 100, this bug only +affects configs that involve more than 100 servers whose addr:port +properties are likely to be updated at the same time (batched updates +from cli, lua, dns..) + +To fix the bug, we move the budget check after the current event is fully +handled. For that we went from a basic 'while' to 'do..while' loop as we +assume from the config that 'tune.events.max-events-at-once' cannot be 0. +While at it, we reschedule the task once thread isolation ends (it was not +required to perform the reschedule while under isolation) to give the hand +back faster to waiting threads. + +This patch should be backported up to 2.9 with cd99440. It should fix +GH #2666. + +(cherry picked from commit 8f1fd96d17588fb571959901bd20d4239b1a96af) +Signed-off-by: Christopher Faulet +(cherry picked from commit b2dabc930c0f6c231b0a757fd8e7e7b01818f398) +Signed-off-by: Christopher Faulet + +Conflict: NA +Reference: https://git.haproxy.org/?p=haproxy-2.9.git;a=commit;h=d9b6fa6b251b2db654ac42b623b86c06735978bd +--- + src/server.c | 34 ++++++++++++++++++---------------- + 1 file changed, 18 insertions(+), 16 deletions(-) + +diff --git a/src/server.c b/src/server.c +index fa5c4c704406b..5989a9373fada 100644 +--- a/src/server.c ++++ b/src/server.c +@@ -230,7 +230,11 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne + struct event_hdl_async_event *event; + + /* check for new server events that we care about */ +- while ((event = event_hdl_async_equeue_pop(&server_atomic_sync_queue))) { ++ do { ++ event = event_hdl_async_equeue_pop(&server_atomic_sync_queue); ++ if (!event) ++ break; /* no events in queue */ ++ + if (event_hdl_sub_type_equal(event->type, EVENT_HDL_SUB_END)) { + /* ending event: no more events to come */ + event_hdl_async_free_event(event); +@@ -239,20 +243,6 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne + break; + } + +- if (!remain) { +- /* STOP: we've already spent all our budget here, and +- * considering we possibly are under isolation, we cannot +- * keep blocking other threads any longer. +- * +- * Reschedule the task to finish where we left off if +- * there are remaining events in the queue. +- */ +- if (!event_hdl_async_equeue_isempty(&server_atomic_sync_queue)) +- task_wakeup(task, TASK_WOKEN_OTHER); +- break; +- } +- remain--; +- + /* new event to process */ + if (event_hdl_sub_type_equal(event->type, EVENT_HDL_SUB_SERVER_INETADDR)) { + struct sockaddr_storage new_addr; +@@ -313,7 +303,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne + srv_set_addr_desc(srv, 1); + } + event_hdl_async_free_event(event); +- } ++ } while (--remain); // event_hdl_tune.max_events_at_once is expected to be > 0 + + /* some events possibly required thread_isolation: + * now that we are done, we must leave thread isolation before +@@ -322,6 +312,18 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne + if (thread_isolated()) + thread_release(); + ++ if (!remain) { ++ /* we stopped because we've already spent all our budget here, ++ * and considering we possibly were under isolation, we cannot ++ * keep blocking other threads any longer. ++ * ++ * Reschedule the task to finish where we left off if ++ * there are remaining events in the queue. ++ */ ++ if (!event_hdl_async_equeue_isempty(&server_atomic_sync_queue)) ++ task_wakeup(task, TASK_WOKEN_OTHER); ++ } ++ + return task; + } + diff --git a/backport-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch b/backport-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch new file mode 100644 index 0000000000000000000000000000000000000000..0598cfc268d195766dd053c13a6084f697f8d37c --- /dev/null +++ b/backport-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch @@ -0,0 +1,47 @@ +From 02819d2e36611d2e19c3ca084f75199c8c215067 Mon Sep 17 00:00:00 2001 +From: Valentine Krasnobaeva +Date: Mon, 6 May 2024 14:24:41 +0200 +Subject: [PATCH] BUG/MINOR: haproxy: only tid 0 must not sleep if got signal + +This patch fixes the commit eea152ee68 +("BUG/MINOR: signals/poller: ensure wakeup from signals"). + +There is some probability that run_poll_loop() becomes inifinite, if +TH_FL_SLEEPING is withdrawn from all threads in the second signal_queue_len +check, when a signal has received just after the first one. + +In such particular case, the 'wake' variable, which is used to terminate +thread's poll loop is never reset to 0. So, we never enter to the "stopping" +part of the run_poll_loop() and threads, except the one with id 0 (tid 0 +handles signals), will continue to call _do_poll() eternally and will never +sleep, as its TH_FL_SLEEPING flag was unset. + +This flag needs to be removed only for the tid 0, as it was done in the first +signal_queue_len check. + +This fixes an issue #2537 "infinite loop when shutting down". + +This fix must be backported in every stable version. + +(cherry picked from commit 4a9e3e102e192b9efd17e3241a6cc659afb7e7dc) +Signed-off-by: Amaury Denoyelle + +Conflict: NA +Reference: https://git.haproxy.org/?p=haproxy-2.9.git;a=commit;h=02819d2e36611d2e19c3ca084f75199c8c215067 +--- + src/haproxy.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/haproxy.c b/src/haproxy.c +index d0c3c4348ae12..8c20dfe22fa80 100644 +--- a/src/haproxy.c ++++ b/src/haproxy.c +@@ -3074,7 +3074,7 @@ void run_poll_loop() + if (thread_has_tasks()) { + activity[tid].wake_tasks++; + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING); +- } else if (signal_queue_len) { ++ } else if (signal_queue_len && tid == 0) { + /* this check is required after setting TH_FL_SLEEPING to avoid + * a race with wakeup on signals using wake_threads() */ + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING); diff --git a/haproxy.spec b/haproxy.spec index 4420f53a3674247a14b96a76026657c460a41c00..0b38ce83b51ba4b1be7ede39c53de6329ce223ae 100644 --- a/haproxy.spec +++ b/haproxy.spec @@ -5,7 +5,7 @@ Name: haproxy Version: 2.9.5 -Release: 10 +Release: 11 Summary: The Reliable, High Performance TCP/HTTP Load Balancer License: GPLv2+ @@ -27,6 +27,8 @@ Patch8: backport-BUG-MEDIUM-queues-Make-sure-we-call-process_srv_queu. Patch9: backport-BUG-MEDIUM-queue-Make-process_srv_queue-return-the-n.patch Patch10: backport-CVE-2025-32464.patch Patch11: backport-BUG-MINOR-server-fix-slowstart-behavior.patch +Patch12: backport-BUG-MEDIUM-server-addr-fix-tune.events.max-events-at.patch +Patch13: backport-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch BuildRequires: gcc lua-devel pcre2-devel openssl-devel systemd-devel systemd libatomic Requires(pre): shadow-utils @@ -131,6 +133,13 @@ exit 0 %{_mandir}/man1/* %changelog +* Fri Jul 18 2025 xinghe - 2.9.5-11 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:haproxy: only tid 0 must not sleep if got signal + server/addr: fix tune.events.max-events-at-once event miss and leak + * Mon Jun 23 2025 xinghe - 2.9.5-10 - Type:bugfix - CVE:NA