RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-01-31 02:22:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MAJOR: resolvers: add other missing references during resolution removal

Browse Source 
There is a fundamental design bug in the resolvers code which is that
a list of active resolutions is being walked to try to delete outdated
entries, and that the code responsible for removing them also removes
other elements, including the next one which will be visited by the
list iterator. This randomly causes a use-after-free condition leading
to crashes, infinite loops and various other issues such as random memory
corruption.

A first fix for the memory fix for this was brought by commit 0efc0993e
("BUG/MEDIUM: resolvers: Don't release resolution from a requester
callbacks"). While preparing for more fixes, some code was factored by
commit 11c6c3965 ("MINOR: resolvers: Clean server in a dedicated function
when removing a SRV item"), which inadvertently passed "0" as the "safe"
argument all the time, missing one case of removal protection, instead
of always using "safe". This patch reintroduces the correct argument.

This must be backported with all fixes above.

Cc: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Willy Tarreau 2021-10-18 16:49:14 +02:00
parent 62e467c667
commit 2a67aa0a51
1 changed files with 5 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/resolvers.c
View File

@ -564,9 +564,9 @@ int resolv_read_name(unsigned char *buffer, unsigned char *bufend,
*
* Must be called with the DNS lock held.
*/
static void resolv_srvrq_cleanup_srv(struct server *srv)
static void resolv_srvrq_cleanup_srv(struct server *srv, int safe)
{
resolv_unlink_resolution(srv->resolv_requester, 0);
resolv_unlink_resolution(srv->resolv_requester, safe);
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
srvrq_update_srv_status(srv, 1);
ha_free(&srv->hostname);
@ -597,7 +597,7 @@ static struct task *resolv_srvrq_expire_task(struct task *t, void *context, unsi
goto end;
HA_SPIN_LOCK(DNS_LOCK, &srv->srvrq->resolvers->lock);
resolv_srvrq_cleanup_srv(srv);
resolv_srvrq_cleanup_srv(srv, 0);
HA_SPIN_UNLOCK(DNS_LOCK, &srv->srvrq->resolvers->lock);
end:
@ -638,7 +638,7 @@ static void resolv_check_response(struct resolv_resolution *res)
else if (item->type == DNS_RTYPE_SRV) {
/* Remove any associated server */
list_for_each_entry_safe(srv, srvback, &item->attached_servers, srv_rec_item)
resolv_srvrq_cleanup_srv(srv);
resolv_srvrq_cleanup_srv(srv, 0);
}
LIST_DELETE(&item->list);
@ -1944,7 +1944,7 @@ void resolv_detach_from_resolution_answer_items(struct resolv_resolution *res,
if (item->type == DNS_RTYPE_SRV) {
list_for_each_entry_safe(srv, srvback, &item->attached_servers, srv_rec_item) {
if (srv->srvrq == srvrq)
resolv_srvrq_cleanup_srv(srv);
resolv_srvrq_cleanup_srv(srv, safe);
}
}
}