]> git.ipfire.org Git - thirdparty/haproxy.git/commit
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)
committerWilly Tarreau <w@1wt.eu>
Wed, 20 Oct 2021 15:54:22 +0000 (17:54 +0200)
commitf766ec6b536725870db2ef48300c371c70c7dda0
treeb20d6f48a03ae8e1bc54e4761e08e0315ab144d8
parentaae7320b0dd9a379792d7cbe3304b23bdb045740
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.
src/resolvers.c