If you call evdns_base_free() with @fail_requests == 1, then it will defer
callback with DNS_ERR_SHUTDOWN, but that callback (internal) uses
data->evdns_base, but we already freed that evdns base, so we can't do
this, fix this by checking @result to DNS_ERR_SHUTDOWN.
Fixes: regress dns/client_fail_requests_getaddrinfo
Fixes: #269
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
Interesting that this wasn't found by regression tests since they respond with
that SoME-rAndDom-CaSe domains, and no case-insensitive mode is required during
comparing response from the server and request.
Fixes#288
Covered-by: regress dns/search_lower
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").
In case we are failing request (evdns_request_timeout_callback()), we
delete timeout_event in request_finished(), while just before calling
request_finished() (for failing request) there was a call to
nameserver_failed(), that add event for timeout_event, IOW we must fail
ns after request because otherwise we will not have timeout_event
actived, and we will waiting forever.
Before this patch the dns/retry_disable_when_inactive will wait forever,
after - OK.
When user install EVDNS_BASE_DISABLE_WHEN_INACTIVE flag for evdns base,
we must remove the timer that is used for probing, if current dns server
failed, otherwise it won't break the loop.
Since evutil_snprintf() (actually evutil_vsnprintf() called by it) will
make sure the buffer is null-terminated by placing a null byte at
len_out - 1, we need to pass the full length of the buffer; otherwise
the path will end in "\\host" instead of "\\hosts".
Hosts files are not loaded in evdns_base_config_windows_nameservers() if
load_nameservers_with_getnetworkparams() succeeds on Windows. Parse and
load it first before setting up nameservers.
"flush" can imply writing something out to a file or connection before
clearing it; "clear" always means "remove". It's also potentially
misleading to say "outdated" here, since the function removes _all_
addresses regardless, not just certain outdated ones.
Also, don't free the lock in this function. Also reindent the function.
As mentioned at https://sourceforge.net/p/levent/bugs/293/
created a small function "evdns_base_flush_outdated_host_addresses" which removes all the previous requests of hosts , if user wants to clean up the list of hosts can call and use this function.
Requires function declaration to be added in include/event2/dns.h
Adding it in another patch for the same bug.
If there is no nameservers installed, using
evdns_base_nameserver_ip_add(), than evdns_base_resume() will SEGFAULT,
because of NULL dereference in evdns_requests_pump_waiting_queue()
Conflicts:
evdns.c
If there is no nameservers installed, using
evdns_base_nameserver_ip_add(), than evdns_base_resume() will SEGFAULT,
because of NULL dereference in evdns_requests_pump_waiting_queue()
Here is the brief description of problem:
When you are use evdns to resolve domains to IP adresses (see
./sample/dns-example) you loop never returns from event_base_dispatch(),
and because of this the program will never terminated.
Because existing programs may be depending on the old behavior, we
only apply the fix when evdns_base_new() is created with a new flag -
EVDNS_BASE_DISABLE_WHEN_INACTIVE.
(Commit message edited by Nick while squashing the branch.)
If an evdns_getaddrinfo timeout happens while pending_cb is set, and
a callback is about to run, but we get a call to
evdns_getaddrinfo_gotresolve before it finishes.
Github issue #60. Thanks to Greg Hazel for patch and patience.
Back when deferred_cb stuff had its own queue, the queue was always
executed, but we never ran more than 16 callbacks per iteration.
That made for two problems:
1: Because deferred_cb stuff would always run, and had no priority,
it could cause priority inversion.
2: It doesn't respect the max_dispatch_interval code.
Then, when I refactored deferred_cb to be a special case of
event_callback, that solved the above issues, but made for two more
issues:
3: Because deferred_cb stuff would always get the default priority,
it could could low-priority bufferevents to get too much priority.
4: With code like bufferevent_pair, it's easy to get into a
situation where two deferreds keep adding one another, preventing
the event loop from ever actually scanning for more events.
This commit fixes the above by giving deferreds a better notion of
priorities, and by limiting the number of deferreds that can be
added to the _current_ loop iteration's active queues. (Extra
deferreds are put into the active_later state.)
That isn't an all-purpose priority inversion solution, of course: for
that, you may need to mess around with max_dispatch_interval.
Otherwise, requests initially sent to a failing nameserver would
stay there indefinitely, even if other nameservers would work.
Fix for sourceforge bug 3518439