MEDIUM: resolvers: use a kill list to preserve the list consistency
authorWilly Tarreau <w@1wt.eu>
Mon, 18 Oct 2021 14:46:38 +0000 (16:46 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 3 Nov 2021 14:37:28 +0000 (15:37 +0100)
commit810e1c03ce2b33caf88478e65d292c5d55a7f4b2
tree7f6e0c452196dd3d89ff181ccfdf4ddcf585be11
parent4a0e1647d662df66345433afed7307808d133f88
MEDIUM: resolvers: use a kill list to preserve the list consistency

When scanning resolution.curr it's possible to try to free some
resolutions which will themselves result in freeing other ones. If
one of these other ones is exactly the next one in the list, the list
walk visits deleted nodes and causes memory corruption, double-frees
and so on. The approach taken using the "safe" argument to some
functions seems to work but it's extremely brittle as it is required
to carefully check all call paths from process_ressolvers() and pass
the argument to 1 there to refrain from deleting entries, so the bug
is very likely to come back after some tiny changes to this code.

A variant was tried, checking at various places that the current task
corresponds to process_resolvers() but this is also quite brittle even
though a bit less.

This patch uses another approach which consists in carefully unlinking
elements from the list and deferring their removal by placing it in a
kill list instead of deleting them synchronously. The real benefit here
is that the complexity only has to be placed where the complications
are.

A thread-local list is fed with elements to be deleted before scanning
the resolutions, and it's flushed at the end by picking the first one
until the list is empty. This way we never dereference the next element
and do not care about its presence or not in the list. One function,
resolv_unlink_resolution(), is exported and used outside, so it had to
be modified to use this list as well. Internal code has to use
_resolv_unlink_resolution() instead.

(cherry picked from commit f766ec6b536725870db2ef48300c371c70c7dda0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 2075165e20e5aa08382bf290eaa9bf08ad28f579)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
src/dns.c