]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Prevent passing NULL to dns_dispatch_resume()
authorMichał Kępień <michal@isc.org>
Wed, 15 May 2024 19:24:24 +0000 (21:24 +0200)
committerMichał Kępień <michal@isc.org>
Wed, 15 May 2024 19:24:24 +0000 (21:24 +0200)
If a query sent using the dns_request API times out when the view it was
associated with gets torn down, the dns_dispatch_resume() call in
req_response() may be issued with the 'resp' argument set to NULL,
triggering an assertion failure.  Consider the following scenario ([A]
and [B] are thread identifiers):

 1. [A] Read timeout for a dispatch query fires.

 2. [A] udp_recv() is called.  It locks the dispatch, determines it
    timed out, prepares for calling the higher-level callback with
    ISC_R_TIMEDOUT, and unlocks the dispatch (lib/dns/dispatch.c:633).

 3. [B] The last reference to a view is released.
    dns_requestmgr_shutdown() is called, canceling all in-flight
    requests for that view.  (Note that udp_recv() in thread [A] already
    unlocked the dispatch, so its state can be modified.)  As a part of
    this process, request_cancel() calls dns_dispatch_done() on
    request->dispentry, setting it to NULL.

 4. [A] udp_recv() calls the higher-level callback (req_response()) with
    ISC_R_TIMEDOUT.

 5. [A] Since the request timed out, req_response() retries sending it.
    In the process, it calls dns_dispatch_resume(), passing
    request->dispentry as the 'resp' argument.

 6. [A] Since 'resp' is NULL, the REQUIRE(VALID_RESPONSE(resp));
    assertion in dns_dispatch_resume() fails.

Fix by checking whether the request has been canceled before calling
dns_dispatch_resume(), similarly to how it is done in req_connected()
and req_senddone().

lib/dns/request.c

index b2b7b119b990554e4f1736555be10406331c5fb0..fb17ed2262ee78492b1705a87a0010cd9915fb9e 100644 (file)
@@ -1061,7 +1061,7 @@ req_response(isc_result_t result, isc_region_t *region, void *arg) {
 
        if (result == ISC_R_TIMEDOUT) {
                LOCK(&request->requestmgr->locks[request->hash]);
-               if (request->udpcount > 1 &&
+               if (!DNS_REQUEST_CANCELED(request) && request->udpcount > 1 &&
                    (request->flags & DNS_REQUEST_F_TCP) == 0)
                {
                        request->udpcount -= 1;