From 14f84bbdc77d90b1d936076661443cdbf516c593 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 23 Nov 2015 15:36:30 +0300 Subject: [PATCH] evdns: evdns_base_free(): free requests before namservers Otherwise we will trigger next UAF: $ valgrind --vgdb-error=1 regress --no-fork +dns/client_fail_requests ==24733== Memcheck, a memory error detector ==24733== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==24733== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info ==24733== Command: regress --no-fork +dns/client_fail_requests ==24733== ==24733== ==24733== TO DEBUG THIS PROCESS USING GDB: start GDB like this ==24733== /path/to/gdb regress ==24733== and then give GDB the following command ==24733== target remote | /usr/lib/valgrind/../../bin/vgdb --pid=24733 ==24733== --pid is optional if only one valgrind process is running ==24733== dns/client_fail_requests: ==24733== Invalid read of size 4 ==24733== at 0x4C3352: request_finished (evdns.c:662) ==24733== by 0x4CC8B7: evdns_base_free_and_unlock (evdns.c:4048) ==24733== by 0x4CCAFD: evdns_base_free (evdns.c:4088) ==24733== by 0x458E95: dns_client_fail_requests_test (regress_dns.c:2039) ==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105) ==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252) ==24733== by 0x48F67E: tinytest_main (tinytest.c:434) ==24733== by 0x47C0DA: main (regress_main.c:461) ==24733== Address 0x61e6f70 is 448 bytes inside a block of size 456 free'd ==24733== at 0x4C29EAB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==24733== by 0x4A8F4D: event_mm_free_ (event.c:3512) ==24733== by 0x4CC7A1: evdns_nameserver_free (evdns.c:4021) ==24733== by 0x4CC7DC: evdns_base_free_and_unlock (evdns.c:4037) ==24733== by 0x4CCAFD: evdns_base_free (evdns.c:4088) ==24733== by 0x458E95: dns_client_fail_requests_test (regress_dns.c:2039) ==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105) ==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252) ==24733== by 0x48F67E: tinytest_main (tinytest.c:434) ==24733== by 0x47C0DA: main (regress_main.c:461) ==24733== Block was alloc'd at ==24733== at 0x4C28C4F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==24733== by 0x4A8D5A: event_mm_malloc_ (event.c:3437) ==24733== by 0x4C8B96: evdns_nameserver_add_impl_ (evdns.c:2505) ==24733== by 0x4C916D: evdns_base_nameserver_ip_add (evdns.c:2629) ==24733== by 0x458DA3: dns_client_fail_requests_test (regress_dns.c:2031) ==24733== by 0x48EA5D: testcase_run_bare_ (tinytest.c:105) ==24733== by 0x48ED3F: testcase_run_one (tinytest.c:252) ==24733== by 0x48F67E: tinytest_main (tinytest.c:434) ==24733== by 0x47C0DA: main (regress_main.c:461) ==24733== ==24733== (action on error) vgdb me ... Fixes: regress dns/client_fail_requests Fixes: #269 --- evdns.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/evdns.c b/evdns.c index 311e0a73..38cfd783 100644 --- a/evdns.c +++ b/evdns.c @@ -4032,15 +4032,6 @@ evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests) /* TODO(nickm) we might need to refcount here. */ - for (server = base->server_head; server; server = server_next) { - server_next = server->next; - evdns_nameserver_free(server); - if (server_next == base->server_head) - break; - } - base->server_head = NULL; - base->global_good_nameservers = 0; - for (i = 0; i < base->n_req_heads; ++i) { while (base->req_heads[i]) { if (fail_requests) @@ -4055,6 +4046,14 @@ evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests) } base->global_requests_inflight = base->global_requests_waiting = 0; + for (server = base->server_head; server; server = server_next) { + server_next = server->next; + evdns_nameserver_free(server); + if (server_next == base->server_head) + break; + } + base->server_head = NULL; + base->global_good_nameservers = 0; if (base->global_search_state) { for (dom = base->global_search_state->head; dom; dom = dom_next) { -- 2.40.0