diff options
author | Azat Khuzhin <a3at.mail@gmail.com> | 2014-11-12 20:16:18 +0300 |
---|---|---|
committer | Azat Khuzhin <a3at.mail@gmail.com> | 2014-11-12 20:20:11 +0300 |
commit | 61262a0f08097718e566c0aa7e2878b1d868f3ed (patch) | |
tree | b7eeec5d491ee5104882540f556e4cadfe1b0249 | |
parent | 6dba1694c89119c44cef03528945e5a5978ab43a (diff) | |
download | libevent-61262a0f08097718e566c0aa7e2878b1d868f3ed.tar.gz |
evdns: avoid read-after-free in evdns_request_timeout_callback()
In evdns_request_timeout_callback() in case we a giving up, we call
request_finished() which will free() req structure, however we ns from
it to fail it, so save pointer to ns to call nameserver_failed() on
them.
Founded with valgrind:
$ valgrind regress dns/retry
==10497== Memcheck, a memory error detector
==10497== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==10497== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==10497== Command: regress dns/retry
==10497==
dns/retry: [forking] ==10498== Invalid read of size 8
==10498== at 0x4C309D: evdns_request_timeout_callback (evdns.c:2179)
==10498== by 0x49EA95: event_process_active_single_queue (event.c:1576)
==10498== by 0x49EFDD: event_process_active (event.c:1668)
==10498== by 0x49F6DD: event_base_loop (event.c:1891)
==10498== by 0x49F063: event_base_dispatch (event.c:1702)
==10498== by 0x44C7F1: dns_retry_test_impl (regress_dns.c:724)
==10498== by 0x44CF60: dns_retry_test (regress_dns.c:749)
==10498== by 0x48A8A1: testcase_run_bare_ (tinytest.c:105)
==10498== by 0x48A94E: testcase_run_forked_ (tinytest.c:189)
==10498== by 0x48AB73: testcase_run_one (tinytest.c:247)
==10498== by 0x48B4C2: tinytest_main (tinytest.c:434)
==10498== by 0x477FC7: main (regress_main.c:459)
==10498== Address 0x6176ef8 is 40 bytes inside a block of size 342 free'd
==10498== at 0x4C29E90: free (vg_replace_malloc.c:473)
==10498== by 0x4A4411: event_mm_free_ (event.c:3443)
==10498== by 0x4BE8C5: request_finished (evdns.c:702)
==10498== by 0x4C3098: evdns_request_timeout_callback (evdns.c:2178)
==10498== by 0x49EA95: event_process_active_single_queue (event.c:1576)
==10498== by 0x49EFDD: event_process_active (event.c:1668)
==10498== by 0x49F6DD: event_base_loop (event.c:1891)
==10498== by 0x49F063: event_base_dispatch (event.c:1702)
==10498== by 0x44C7F1: dns_retry_test_impl (regress_dns.c:724)
==10498== by 0x44CF60: dns_retry_test (regress_dns.c:749)
==10498== by 0x48A8A1: testcase_run_bare_ (tinytest.c:105)
==10498== by 0x48A94E: testcase_run_forked_ (tinytest.c:189)
==10498==
==10498==
==10498== HEAP SUMMARY:
==10498== in use at exit: 0 bytes in 0 blocks
==10498== total heap usage: 83 allocs, 83 frees, 10,020 bytes allocated
==10498==
==10498== All heap blocks were freed -- no leaks are possible
==10498==
==10498== For counts of detected and suppressed errors, rerun with: -v
==10498== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
OK
1 tests ok. (0 skipped)
==10497==
==10497== HEAP SUMMARY:
==10497== in use at exit: 0 bytes in 0 blocks
==10497== total heap usage: 3 allocs, 3 frees, 96 bytes allocated
==10497==
==10497== All heap blocks were freed -- no leaks are possible
==10497==
==10497== For counts of detected and suppressed errors, rerun with: -v
==10497== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Bug was introduced in 97c750d6602517f22a1100f16592b421c38f2a45 ("evdns:
fail ns after we are failing/retrasmitting request").
-rw-r--r-- | evdns.c | 13 |
1 files changed, 8 insertions, 5 deletions
@@ -2175,7 +2175,10 @@ evdns_request_timeout_callback(evutil_socket_t fd, short events, void *arg) { log(EVDNS_LOG_DEBUG, "Giving up on request %p; tx_count==%d", arg, req->tx_count); reply_schedule_callback(req, 0, DNS_ERR_TIMEOUT, NULL); + + struct nameserver *ns = req->ns; request_finished(req, &REQ_HEAD(req->base, req->trans_id), 1); + nameserver_failed(ns, "request timed out."); } else { /* retransmit it */ log(EVDNS_LOG_DEBUG, "Retransmitting request %p; tx_count==%d", @@ -2183,12 +2186,12 @@ evdns_request_timeout_callback(evutil_socket_t fd, short events, void *arg) { (void) evtimer_del(&req->timeout_event); request_swap_ns(req, nameserver_pick(base)); evdns_request_transmit(req); - } - req->ns->timedout++; - if (req->ns->timedout > req->base->global_max_nameserver_timeout) { - req->ns->timedout = 0; - nameserver_failed(req->ns, "request timed out."); + req->ns->timedout++; + if (req->ns->timedout > req->base->global_max_nameserver_timeout) { + req->ns->timedout = 0; + nameserver_failed(req->ns, "request timed out."); + } } EVDNS_UNLOCK(base); |