[Bug 3129] Unknown hosts can put resolver thread into a hard loop

bk: 5826a08fupEsxCc1FwBffh2HLAkPgA

diff --git a/ChangeLog b/ChangeLog
index 0805467dc6b9b1ce7768a039f6a2d87af37546b9..70473d8b325abb40e52008066612a6f39037dfc6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+---
+* [Bug 3129] Unknown hosts can put resolver thread into a hard loop
+  - moved retry decision where it belongs. <perlinger@ntp.org>
+
 ---
 (4.2.8p8) 2016/06/02 Released by Harlan Stenn <stenn@ntp.org>
 

diff --git a/include/ntp_intres.h b/include/ntp_intres.h
index 1b6bd66e0b1170d0439c482a968c9ec7636e281f..1109130076027b6ce9376b31a295925ff5a19b40 100644 (file)
--- a/include/ntp_intres.h
+++ b/include/ntp_intres.h
@@ -9,6 +9,9 @@
 #ifdef WORKER
 #define        INITIAL_DNS_RETRY       2       /* seconds between queries */
 
+/* flags for extended addrinfo version */
+#define GAIR_F_IGNDNSERR       0x0001  /* ignore DNS errors */
+
 /*
  * you call getaddrinfo_sometime(name, service, &hints, retry, callback_func, context);
  * later (*callback_func)(rescode, gai_errno, context, name, service, hints, ai_result) is called.
@@ -19,6 +22,9 @@ typedef void  (*gai_sometime_callback)
 extern int     getaddrinfo_sometime(const char *, const char *,
                                     const struct addrinfo *, int,
                                     gai_sometime_callback, void *);
+extern int     getaddrinfo_sometime_ex(const char *, const char *,
+                                    const struct addrinfo *, int,
+                                    gai_sometime_callback, void *, u_int);
 /*
  * In gai_sometime_callback routines, the resulting addrinfo list is
  * only available until the callback returns.  To hold on to the list

diff --git a/libntp/ntp_intres.c b/libntp/ntp_intres.c
index 0b5bb753425691a1d039f1930ff9033a769a6908..7aa288af57b2bc0e0651f8ca343227ac553580ee 100644 (file)
--- a/libntp/ntp_intres.c
+++ b/libntp/ntp_intres.c
@@ -118,14 +118,16 @@
  * is managed by the code which calls the *_complete routines.
  */
 
+
 /* === typedefs === */
 typedef struct blocking_gai_req_tag {  /* marshalled args */
        size_t                  octets;
        u_int                   dns_idx;
        time_t                  scheduled;
        time_t                  earliest;
-       struct addrinfo         hints;
        int                     retry;
+       struct addrinfo         hints;
+       u_int                   qflags;
        gai_sometime_callback   callback;
        void *                  context;
        size_t                  nodesize;
@@ -205,8 +207,8 @@ static      dnsworker_ctx * get_worker_context(blocking_child *, u_int);
 static void            scheduled_sleep(time_t, time_t,
                                        dnsworker_ctx *);
 static void            manage_dns_retry_interval(time_t *, time_t *,
-                                                 int *,
-                                                 time_t *);
+                                                 int *, time_t *,
+                                                 int/*BOOL*/);
 static int             should_retry_dns(int, int);
 #ifdef HAVE_RES_INIT
 static void            reload_resolv_conf(dnsworker_ctx *);
@@ -230,13 +232,14 @@ static    void            getnameinfo_sometime_complete(blocking_work_req,
  *                       invokes provided callback completion function.
  */
 int
-getaddrinfo_sometime(
+getaddrinfo_sometime_ex(
        const char *            node,
        const char *            service,
        const struct addrinfo * hints,
        int                     retry,
        gai_sometime_callback   callback,
-       void *                  context
+       void *                  context,
+       u_int                   qflags
        )
 {
        blocking_gai_req *      gai_req;
@@ -277,6 +280,7 @@ getaddrinfo_sometime(
        gai_req->context = context;
        gai_req->nodesize = nodesize;
        gai_req->servsize = servsize;
+       gai_req->qflags = qflags;
 
        memcpy((char *)gai_req + sizeof(*gai_req), node, nodesize);
        memcpy((char *)gai_req + sizeof(*gai_req) + nodesize, service,
@@ -451,6 +455,20 @@ blocking_getaddrinfo(
        return 0;
 }
 
+int
+getaddrinfo_sometime(
+       const char *            node,
+       const char *            service,
+       const struct addrinfo * hints,
+       int                     retry,
+       gai_sometime_callback   callback,
+       void *                  context
+       )
+{
+       return getaddrinfo_sometime_ex(node, service, hints, retry,
+                                      callback, context, 0);
+}
+
 
 static void
 getaddrinfo_sometime_complete(
@@ -470,7 +488,7 @@ getaddrinfo_sometime_complete(
        char *                  service;
        char *                  canon_start;
        time_t                  time_now;
-       int                     again;
+       int                     again, noerr;
        int                     af;
        const char *            fam_spec;
        int                     i;
@@ -498,8 +516,9 @@ getaddrinfo_sometime_complete(
                                  gai_req->dns_idx, humantime(time_now)));
                }
        } else {
-               again = should_retry_dns(gai_resp->retcode,
-                                        gai_resp->gai_errno);
+               noerr = !!(gai_req->qflags & GAIR_F_IGNDNSERR);
+               again = noerr || should_retry_dns(
+                                       gai_resp->retcode, gai_resp->gai_errno);
                /*
                 * exponential backoff of DNS retries to 64s
                 */
@@ -528,9 +547,10 @@ getaddrinfo_sometime_complete(
                                                        gai_strerror(gai_resp->retcode),
                                                        gai_resp->retcode);
                                }
-                       manage_dns_retry_interval(&gai_req->scheduled,
-                           &gai_req->earliest, &gai_req->retry,
-                           &child_ctx->next_dns_timeslot);
+                       manage_dns_retry_interval(
+                               &gai_req->scheduled, &gai_req->earliest,
+                               &gai_req->retry, &child_ctx->next_dns_timeslot,
+                               noerr);
                        if (!queue_blocking_request(
                                        BLOCKING_GETADDRINFO,
                                        gai_req,
@@ -826,7 +846,7 @@ getnameinfo_sometime_complete(
                if (gni_req->retry > 0)
                        manage_dns_retry_interval(&gni_req->scheduled,
                            &gni_req->earliest, &gni_req->retry,
-                           &child_ctx->next_dns_timeslot);
+                                                 &child_ctx->next_dns_timeslot, FALSE);
 
                if (gni_req->retry > 0 && again) {
                        if (!queue_blocking_request(
@@ -1033,18 +1053,32 @@ manage_dns_retry_interval(
        time_t *        pscheduled,
        time_t *        pwhen,
        int *           pretry,
-       time_t *        pnext_timeslot
+       time_t *        pnext_timeslot,
+       int             forever
        )
 {
        time_t  now;
        time_t  when;
        int     retry;
+       int     retmax;
                
        now = time(NULL);
        retry = *pretry;
        when = max(now + retry, *pnext_timeslot);
        *pnext_timeslot = when;
-       retry = min(64, retry << 1);
+
+       /* this exponential backoff is slower than doubling up: The
+        * sequence goes 2-3-4-6-8-12-16-24-32... and the upper limit is
+        * 64 seconds for things that should not repeat forever, and
+        * 1024 when repeated forever.
+        */
+       retmax = forever ? 1024 : 64;
+       retry <<= 1;
+       if (retry & (retry - 1))
+               retry &= (retry - 1);
+       else
+               retry -= (retry >> 2);
+       retry = min(retmax, retry);
 
        *pscheduled = now;
        *pwhen = when;

diff --git a/ntpd/ntp_config.c b/ntpd/ntp_config.c
index 2d4ab527743487d1bc7231e10e9b5a5c6af8934d..73c834041801125ec323396623317f0e3b708197 100644 (file)
--- a/ntpd/ntp_config.c
+++ b/ntpd/ntp_config.c
@@ -53,6 +53,12 @@
 #include "ntp_parser.h"
 #include "ntpd-opts.h"
 
+#ifndef IGNORE_DNS_ERRORS
+# define DNSFLAGS 0
+#else
+# define DNSFLAGS GAIR_F_IGNDNSERR
+#endif
+
 extern int yyparse(void);
 
 /* Bug 2817 */
@@ -3813,11 +3819,11 @@ config_peers(
                        hints.ai_socktype = SOCK_DGRAM;
                        hints.ai_protocol = IPPROTO_UDP;
 
-                       getaddrinfo_sometime(*cmdline_servers,
+                       getaddrinfo_sometime_ex(*cmdline_servers,
                                             "ntp", &hints,
                                             INITIAL_DNS_RETRY,
                                             &peer_name_resolved,
-                                            (void *)ctx);
+                                            (void *)ctx, DNSFLAGS);
 # else /* !WORKER follows */
                        msyslog(LOG_ERR,
                                "hostname %s can not be used, please use IP address instead.",
@@ -3891,10 +3897,11 @@ config_peers(
                        hints.ai_socktype = SOCK_DGRAM;
                        hints.ai_protocol = IPPROTO_UDP;
 
-                       getaddrinfo_sometime(curr_peer->addr->address,
+                       getaddrinfo_sometime_ex(curr_peer->addr->address,
                                             "ntp", &hints,
                                             INITIAL_DNS_RETRY,
-                                            &peer_name_resolved, ctx);
+                                            &peer_name_resolved, ctx,
+                                            DNSFLAGS);
 # else /* !WORKER follows */
                        msyslog(LOG_ERR,
                                "hostname %s can not be used, please use IP address instead.",
@@ -3935,16 +3942,10 @@ peer_name_resolved(
        DPRINTF(1, ("peer_name_resolved(%s) rescode %d\n", name, rescode));
 
        if (rescode) {
-#ifndef IGNORE_DNS_ERRORS
                free(ctx);
                msyslog(LOG_ERR,
                        "giving up resolving host %s: %s (%d)",
                        name, gai_strerror(rescode), rescode);
-#else  /* IGNORE_DNS_ERRORS follows */
-               getaddrinfo_sometime(name, service, hints,
-                                    INITIAL_DNS_RETRY,
-                                    &peer_name_resolved, context);
-#endif
                return;
        }