Skip to content

Commit

Permalink
BUG/MEDIUM: resolvers: Insert a non-executed resulution in front of t…
Browse files Browse the repository at this point in the history
…he 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()
  • Loading branch information
capflam committed Nov 13, 2024
1 parent 72e5298 commit 8f28dbe
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/resolvers.c
Original file line number Diff line number Diff line change
Expand Up @@ -2531,7 +2531,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);
}
}

Expand Down

0 comments on commit 8f28dbe

Please sign in to comment.