From 7aa4f75325ed5558a8593805406cb1681a4e9cbb Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Tue, 12 Nov 2024 18:51:20 +0100 Subject: [PATCH] BUG/MEDIUM: resolvers: Insert a non-executed resulution in front of the wait list When a resolver is woken up to process DNS resolutions, it is possible to trigger an infinite loop on the resolver's wait list because delayed resolutions are always reinserted at the end of this list. This leads the watchdog to kill the process. By re-inserting them in front of the list, that fixes the bug. When a resolver tries to send the queries for the resolutions in its wait list, it may be unable to proceed for a resolution. This may happen because the resolution must be skipped (no hostname to resolv, a resolution already in-progress) or when an error occurred. In that case, the resolution is re-inserted in the resolver's wait list to be retry later, on a next wakeup. However, the resolution is inserted at the end of the wait list. So it is immediately reevaluated, in the same execution loop, instead of to be delayed. Most of time, it is not an issue because the resolution is considered as not expired on the second run. But it is an problem when the internal time wraps and is equal to 0. In that case, the resolution expiration date is badly computed and it is always considered as expired. If two or more resolutions are in that state, the resolver loops for ever on its wait list, until the process is killed by the watchdog. So we can argue that the way the resolution expiration date is computed must be fixed. And it would be true in a perfect world. However, the resolvers code is so crapy that it is hard to be sure to not introduce regressions. It is farly easier to re-insert delayed resolutions in front of the wait list. This fixes the issue and at worst, these resolutions will be evaluated one time too many on the next wakeup and only if now_ms was equal to 0 on the prior wakeup. This patch should be backported to all stable versions. On 2.2, LIST_ADD() must be used instead of LIST_INSERT() (cherry picked from commit 8f28dbeea94e11e2327362755f16d18b301fd153) Signed-off-by: Christopher Faulet (cherry picked from commit 33b0ca4440a8f09fe42ff34f27e7113552053ae1) Signed-off-by: Christopher Faulet --- src/resolvers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolvers.c b/src/resolvers.c index 3275cd22d094..bb0211f297cd 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -2447,7 +2447,7 @@ struct task *process_resolvers(struct task *t, void *context, unsigned int state if (resolv_run_resolution(res) != 1) { res->last_resolution = now_ms; LIST_DEL_INIT(&res->list); - LIST_APPEND(&resolvers->resolutions.wait, &res->list); + LIST_INSERT(&resolvers->resolutions.wait, &res->list); } }