]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
correct TCP error handling in dispatch and resolver
authorEvan Hunt <each@isc.org>
Wed, 9 Feb 2022 22:56:04 +0000 (14:56 -0800)
committerOndřej Surý <ondrej@isc.org>
Thu, 17 Feb 2022 08:59:12 +0000 (09:59 +0100)
- certain TCP result codes, including ISC_R_EOF and
  ISC_R_CONNECTIONRESET, were being mapped to ISC_R_SHUTTINGDOWN
  before calling the response handler in tcp_recv_cancelall().
  the result codes should be passed through to the response handler
  without being changed.

- the response handlers, resquery_response() and req_response(), had
  code to return immediately if encountering ISC_R_EOF, but this is
  not the correct behavior; that should only happen in the case of
  ISC_R_CANCELED when it was the caller that canceled the operation

- ISC_R_CONNECTIONRESET was not being caught in rctx_dispfail().

- removed code in rctx_dispfail() to retry queries without EDNS
  when receiving ISC_R_EOF; this is now treated the same as any
  other connection failure.

lib/dns/dispatch.c
lib/dns/request.c
lib/dns/resolver.c

index c13d91d0642555f4c675e41e4c2bc2f4b22ec597..81e2f0658d72626c9196520a0bff3b2508e0348c 100644 (file)
@@ -713,13 +713,14 @@ tcp_recv_done(dns_dispentry_t *resp, isc_result_t eresult,
 }
 
 static void
-tcp_recv_cancelall(dns_displist_t *resps, isc_region_t *region) {
+tcp_recv_cancelall(dns_displist_t *resps, isc_region_t *region,
+                  isc_result_t result) {
        dns_dispentry_t *resp = NULL, *next = NULL;
 
        for (resp = ISC_LIST_HEAD(*resps); resp != NULL; resp = next) {
                next = ISC_LIST_NEXT(resp, rlink);
                ISC_LIST_UNLINK(*resps, resp, rlink);
-               resp->response(ISC_R_SHUTTINGDOWN, region, resp->arg);
+               resp->response(result, region, resp->arg);
                dispentry_detach(&resp);
        }
 }
@@ -831,7 +832,7 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
                break;
        default:
                /* We're being shut down; cancel all outstanding resps. */
-               tcp_recv_cancelall(&resps, region);
+               tcp_recv_cancelall(&resps, region, result);
        }
 
        dns_dispatch_detach(&disp);
index b4348e469c0eed325eed30fd15c9166528d7bb34..bd268a7717fa733565e6d693edf452a6c0b1500b 100644 (file)
@@ -1058,7 +1058,7 @@ req_response(isc_result_t result, isc_region_t *region, void *arg) {
        req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request,
                isc_result_totext(result));
 
-       if (result == ISC_R_CANCELED || result == ISC_R_EOF) {
+       if (result == ISC_R_CANCELED) {
                return;
        }
 
index b558485157ca56145dde3a17523db3539be23baf..9610608fb839d0bda78b974054d5077a0b70a19e 100644 (file)
@@ -7411,7 +7411,7 @@ resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) {
        fetchctx_t *fctx = NULL;
        respctx_t rctx;
 
-       if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) {
+       if (eresult == ISC_R_CANCELED) {
                return;
        }
 
@@ -7886,45 +7886,40 @@ rctx_answer_init(respctx_t *rctx) {
 static isc_result_t
 rctx_dispfail(respctx_t *rctx) {
        fetchctx_t *fctx = rctx->fctx;
-       resquery_t *query = rctx->query;
 
        if (rctx->result == ISC_R_SUCCESS) {
                return (ISC_R_SUCCESS);
        }
 
-       if (rctx->result == ISC_R_EOF &&
-           (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) {
-               /*
-                * The problem might be that they don't understand
-                * EDNS0. Turn it off and try again.
-                */
-               rctx->retryopts |= DNS_FETCHOPT_NOEDNS0;
-               rctx->resend = true;
-               add_bad_edns(fctx, &query->addrinfo->sockaddr);
-       } else {
-               /*
-                * There's no hope for this response.
-                */
-               rctx->next_server = true;
+       /*
+        * There's no hope for this response.
+        */
+       rctx->next_server = true;
 
-               /*
-                * If this is a network error, mark the server as bad so
-                * that we won't try it for this fetch again.  Also
-                * adjust finish and no_response so that we penalize
-                * this address in SRTT adjustment later.
-                */
-               if (rctx->result == ISC_R_HOSTUNREACH ||
-                   rctx->result == ISC_R_NETUNREACH ||
-                   rctx->result == ISC_R_CONNREFUSED ||
-                   rctx->result == ISC_R_CANCELED ||
-                   rctx->result == ISC_R_SHUTTINGDOWN)
-               {
-                       rctx->broken_server = rctx->result;
-                       rctx->broken_type = badns_unreachable;
-                       rctx->finish = NULL;
-                       rctx->no_response = true;
-               }
+       /*
+        * If this is a network failure, the operation is cancelled,
+        * or the network manager is being shut down, we mark the server
+        * as bad so that we won't try it for this fetch again. Also
+        * adjust finish and no_response so that we penalize this
+        * address in SRTT adjustments later.
+        */
+       switch (rctx->result) {
+       case ISC_R_EOF:
+       case ISC_R_HOSTUNREACH:
+       case ISC_R_NETUNREACH:
+       case ISC_R_CONNREFUSED:
+       case ISC_R_CONNECTIONRESET:
+       case ISC_R_CANCELED:
+       case ISC_R_SHUTTINGDOWN:
+               rctx->broken_server = rctx->result;
+               rctx->broken_type = badns_unreachable;
+               rctx->finish = NULL;
+               rctx->no_response = true;
+               break;
+       default:
+               break;
        }
+
        FCTXTRACE3("dispatcher failure", rctx->result);
        rctx_done(rctx, ISC_R_SUCCESS);
        return (ISC_R_COMPLETE);