Replace dead link in dnsmasq.conf.example.

Thanks to Timo van Roermund for spotting this.

CHANGELOG

@@ -1,7 +1,21 @@
+version 2.89
+        Fix bug introduced in 2.88 (commit fe91134b) which can result
+	in corruption of the DNS cache internal data structures and
+	logging of "cache internal error". This has only been seen
+	in one place in the wild, and it took considerable effort
+	to even generate a test case to reproduce it, but there's
+	no way to be sure it won't strike, and the effect is to break
+	the cache badly. Installations with DNSSEC enabled are more
+	likely to see the problem, but not running DNSSEC does not
+	guarantee that it won't happen. Thanks to Timo van Roermund
+	for reporting the bug and for his great efforts in chasing
+	it down.
+
+
 version 2.88
 	Fix bug in --dynamic-host when an interface has /16 IPv4
-	address. Thanks to Mark Dietzer for spotting this.
-	
+  	address. Thanks to Mark Dietzer for spotting this.
+
 	Add --fast-dns-retry option. This gives dnsmasq the ability
 	to originate retries for upstream DNS queries itself, rather
 	than relying on the downstream client. This is most useful
@@ -12,12 +26,18 @@ version 2.88
 	in the cache, but its time-to-live has expired, dnsmasq will
 	return the data anyway. (It attempts to refresh the
 	data with an upstream query after returning the stale data.)
-	This can improve speed and reliability. It comes 
+	This can improve speed and reliability. It comes
 	at the expense of sometimes returning out-of-date data and
 	less efficient cache utilisation, since old data cannot be
 	flushed when its TTL expires, so the cache becomes
 	strictly least-recently-used.
 
+	Add --port-limit option which allows tuning for robustness in
+	the face of some upstream network errors. Thanks to
+	Prashant Kumar Singh, Ravi Nagayach and Mike Danilov,
+	all of Amazon Web Services, for their efforts in developing this
+	and the stale-cache and fast-retry options.
+
 	Make --hostsdir (but NOT --dhcp-hostsdir and --dhcp-optsdir)
 	handle removal of whole files or entries within files.
 	Thanks to Dominik Derigs for the initial patches for this.

debian/changelog

@@ -1,3 +1,9 @@
+dnsmasq (2.89-1) unstable; urgency=low
+
+   * New upstream.
+
+ -- Simon Kelley <simon@thekelleys.org.uk>  Fri, 13 Jan 2023 21:57:01 +0000
+
 dnsmasq (2.88-1) unstable; urgency=low
 
    * New upstream.

dnsmasq.conf.example

@@ -394,7 +394,7 @@
 
 # The following DHCP options set up dnsmasq in the same way as is specified
 # for the ISC dhcpcd in
-# http://www.samba.org/samba/ftp/docs/textdocs/DHCP-Server-Configuration.txt
+# https://web.archive.org/web/20040313070105/http://us1.samba.org/samba/ftp/docs/textdocs/DHCP-Server-Configuration.txt
 # adapted for a typical dnsmasq installation where the host running
 # dnsmasq is also the host running samba.
 # you may want to uncomment some or all of them if you use

man/dnsmasq.8

@@ -2215,6 +2215,20 @@ exit 0
 
 and /share/ads-domains.gz containing a compressed
 list of ad server domains will save disk space with large ad-server blocklists.
+.TP
+.B --no-ident
+Do not respond to class CHAOS and type TXT in domain bind queries.
+
+Without this option being set, the cache statistics are also available in the
+DNS as answers to queries of class CHAOS and type TXT in domain bind. The domain
+names are cachesize.bind, insertions.bind, evictions.bind, misses.bind,
+hits.bind, auth.bind and servers.bind unless disabled at compile-time. An
+example command to query this, using the
+.B dig
+utility would be
+
+dig +short chaos txt cachesize.bind
+
 .SH CONFIG FILE
 At startup, dnsmasq reads
 .I /etc/dnsmasq.conf,
@@ -2264,15 +2278,6 @@ resulted in an error. In
 mode or when full logging is enabled (\fB--log-queries\fP), a complete dump of the
 contents of the cache is made. 
 
-The cache statistics are also available in the DNS as answers to 
-queries of class CHAOS and type TXT in domain bind. The domain names are cachesize.bind, insertions.bind, evictions.bind, 
-misses.bind, hits.bind, auth.bind and servers.bind. An example command to query this, using the 
-.B dig
-utility would be
-
-dig +short chaos txt cachesize.bind
-
-.PP 
 When it receives SIGUSR2 and it is logging direct to a file (see
 .B --log-facility
 ) 

src/cache.c

@@ -28,6 +28,7 @@ static int bignames_left, hash_size;
 static void make_non_terminals(struct crec *source);
 static struct crec *really_insert(char *name, union all_addr *addr, unsigned short class,
 				  time_t now,  unsigned long ttl, unsigned int flags);
+static void dump_cache_entry(struct crec *cache, time_t now);
 
 /* type->string mapping: this is also used by the name-hash function as a mixing table. */
 /* taken from https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml */
@@ -235,19 +236,23 @@ static void cache_hash(struct crec *crecp)
 
   char *name = cache_get_name(crecp);
   struct crec **up = hash_bucket(name);
-
-  if (!(crecp->flags & F_REVERSE))
+  unsigned int flags = crecp->flags & (F_IMMORTAL | F_REVERSE);
+  
+  if (!(flags & F_REVERSE))
     {
       while (*up && ((*up)->flags & F_REVERSE))
 	up = &((*up)->hash_next); 
       
-      if (crecp->flags & F_IMMORTAL)
+      if (flags & F_IMMORTAL)
 	while (*up && !((*up)->flags & F_IMMORTAL))
 	  up = &((*up)->hash_next);
     }
 
-  /* Preserve order when inserting the same name multiple times. */
-  while (*up && hostname_isequal(cache_get_name(*up), name))
+  /* Preserve order when inserting the same name multiple times.
+     Do not mess up the flag invariants. */
+  while (*up &&
+	 hostname_isequal(cache_get_name(*up), name) &&
+	 flags == ((*up)->flags & (F_IMMORTAL | F_REVERSE)))
     up = &((*up)->hash_next);
   
   crecp->hash_next = *up;
@@ -594,7 +599,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho
   struct crec *new, *target_crec = NULL;
   union bigname *big_name = NULL;
   int freed_all = (flags & F_REVERSE);
-  int free_avail = 0;
+  struct crec *free_avail = NULL;
   unsigned int target_uid;
   
   /* if previous insertion failed give up now. */
@@ -642,7 +647,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho
       
       /* Free entry at end of LRU list, use it. */
       if (!(new->flags & (F_FORWARD | F_REVERSE)))
-	break;
+	break; 
 
       /* End of LRU list is still in use: if we didn't scan all the hash
 	 chains for expired entries do that now. If we already tried that
@@ -654,12 +659,9 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho
 	 insert. Once in this state, all inserts will probably fail. */
       if (free_avail)
 	{
-	  static int warned = 0;
-	  if (!warned)
-	    {
-	      my_syslog(LOG_ERR, _("Internal error in cache."));
-	      warned = 1;
-	    }
+	  my_syslog(LOG_ERR, _("Internal error in cache."));
+	  /* Log the entry we tried to delete. */
+	  dump_cache_entry(free_avail, now);
 	  insert_error = 1;
 	  return NULL;
 	}
@@ -667,13 +669,13 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho
       if (freed_all)
 	{
 	  /* For DNSSEC records, uid holds class. */
-	  free_avail = 1; /* Must be free space now. */
+	  free_avail = new; /* Must be free space now. */
 	  
 	  /* condition valid when stale-caching */
 	  if (difftime(now, new->ttd) < 0)
 	    daemon->metrics[METRIC_DNS_CACHE_LIVE_FREED]++;
 	  
-	  cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL);
+	  cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL); 
 	}
       else
 	{
@@ -1080,7 +1082,6 @@ static void add_hosts_entry(struct crec *cache, union all_addr *addr, int addrle
      the array rhash, hashed on address. Note that rhash and the values
      in ->next are only valid  whilst reading hosts files: the buckets are
      then freed, and the ->next pointer used for other things. 
-
      Only insert each unique address once into this hashing structure.
 
      This complexity avoids O(n^2) divergent CPU use whilst reading
@@ -1760,6 +1761,94 @@ static char *sanitise(char *name)
   return name;
 }
 
+static void dump_cache_entry(struct crec *cache, time_t now)
+{
+  (void)now;
+  static char *buff = NULL;
+  
+  char *p, *t = " ";
+  char *a = daemon->addrbuff, *n = cache_get_name(cache);
+
+  /* String length is limited below */
+  if (!buff && !(buff = whine_malloc(150)))
+    return;
+  
+  p = buff;
+  
+  *a = 0;
+  if (strlen(n) == 0 && !(cache->flags & F_REVERSE))
+    n = "<Root>";
+  p += sprintf(p, "%-30.30s ", sanitise(n));
+  if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache))
+    a = sanitise(cache_get_cname_target(cache));
+  else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG))
+    {
+      int targetlen = cache->addr.srv.targetlen;
+      ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority,
+			    cache->addr.srv.weight, cache->addr.srv.srvport);
+      
+      if (targetlen > (40 - len))
+	targetlen = 40 - len;
+      blockdata_retrieve(cache->addr.srv.target, targetlen, a + len);
+      a[len + targetlen] = 0;		
+    }
+#ifdef HAVE_DNSSEC
+  else if (cache->flags & F_DS)
+    {
+      if (!(cache->flags & F_NEG))
+	sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag,
+		cache->addr.ds.algo, cache->addr.ds.digest);
+    }
+  else if (cache->flags & F_DNSKEY)
+    sprintf(a, "%5u %3u %3u", cache->addr.key.keytag,
+	    cache->addr.key.algo, cache->addr.key.flags);
+#endif
+  else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD))
+    { 
+      a = daemon->addrbuff;
+      if (cache->flags & F_IPV4)
+	inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN);
+      else if (cache->flags & F_IPV6)
+	inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN);
+    }
+  
+  if (cache->flags & F_IPV4)
+    t = "4";
+  else if (cache->flags & F_IPV6)
+    t = "6";
+  else if (cache->flags & F_CNAME)
+    t = "C";
+  else if (cache->flags & F_SRV)
+    t = "V";
+#ifdef HAVE_DNSSEC
+  else if (cache->flags & F_DS)
+    t = "S";
+  else if (cache->flags & F_DNSKEY)
+    t = "K";
+#endif
+  else if (!(cache->flags & F_NXDOMAIN)) /* non-terminal */
+    t = "!";
+  
+  p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t,
+	       cache->flags & F_FORWARD ? "F" : " ",
+	       cache->flags & F_REVERSE ? "R" : " ",
+	       cache->flags & F_IMMORTAL ? "I" : " ",
+	       cache->flags & F_DHCP ? "D" : " ",
+	       cache->flags & F_NEG ? "N" : " ",
+	       cache->flags & F_NXDOMAIN ? "X" : " ",
+	       cache->flags & F_HOSTS ? "H" : " ",
+	       cache->flags & F_CONFIG ? "C" : " ",
+	       cache->flags & F_DNSSECOK ? "V" : " ");
+#ifdef HAVE_BROKEN_RTC
+  p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now));
+#else
+  p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd)));
+#endif
+  if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0)
+    p += sprintf(p, " %-40.40s", record_source(cache->uid));
+  
+  my_syslog(LOG_INFO, "%s", buff);
+}
 
 void dump_cache(time_t now)
 {
@@ -1807,90 +1896,14 @@ void dump_cache(time_t now)
 
   if (option_bool(OPT_DEBUG) || option_bool(OPT_LOG))
     {
-      struct crec *cache ;
+      struct crec *cache;
       int i;
       my_syslog(LOG_INFO, "Host                           Address                                  Flags      Expires                  Source");
       my_syslog(LOG_INFO, "------------------------------ ---------------------------------------- ---------- ------------------------ ------------");
     
       for (i=0; i<hash_size; i++)
 	for (cache = hash_table[i]; cache; cache = cache->hash_next)
-	  {
-	    char *t = " ";
-	    char *a = daemon->addrbuff, *p = daemon->namebuff, *n = cache_get_name(cache);
-	    *a = 0;
-	    if (strlen(n) == 0 && !(cache->flags & F_REVERSE))
-	      n = "<Root>";
-	    p += sprintf(p, "%-30.30s ", sanitise(n));
-	    if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache))
-	      a = sanitise(cache_get_cname_target(cache));
-	    else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG))
-	      {
-		int targetlen = cache->addr.srv.targetlen;
-		ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority,
-				      cache->addr.srv.weight, cache->addr.srv.srvport);
-
-		if (targetlen > (40 - len))
-		  targetlen = 40 - len;
-		blockdata_retrieve(cache->addr.srv.target, targetlen, a + len);
-		a[len + targetlen] = 0;		
-	      }
-#ifdef HAVE_DNSSEC
-	    else if (cache->flags & F_DS)
-	      {
-		if (!(cache->flags & F_NEG))
-		  sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag,
-			  cache->addr.ds.algo, cache->addr.ds.digest);
-	      }
-	    else if (cache->flags & F_DNSKEY)
-	      sprintf(a, "%5u %3u %3u", cache->addr.key.keytag,
-		      cache->addr.key.algo, cache->addr.key.flags);
-#endif
-	    else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD))
-	      { 
-		a = daemon->addrbuff;
-		if (cache->flags & F_IPV4)
-		  inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN);
-		else if (cache->flags & F_IPV6)
-		  inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN);
-	      }
-
-	    if (cache->flags & F_IPV4)
-	      t = "4";
-	    else if (cache->flags & F_IPV6)
-	      t = "6";
-	    else if (cache->flags & F_CNAME)
-	      t = "C";
-	    else if (cache->flags & F_SRV)
-	      t = "V";
-#ifdef HAVE_DNSSEC
-	    else if (cache->flags & F_DS)
-	      t = "S";
-	    else if (cache->flags & F_DNSKEY)
-	      t = "K";
-#endif
-	    else /* non-terminal */
-	      t = "!";
-
-	    p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t,
-			 cache->flags & F_FORWARD ? "F" : " ",
-			 cache->flags & F_REVERSE ? "R" : " ",
-			 cache->flags & F_IMMORTAL ? "I" : " ",
-			 cache->flags & F_DHCP ? "D" : " ",
-			 cache->flags & F_NEG ? "N" : " ",
-			 cache->flags & F_NXDOMAIN ? "X" : " ",
-			 cache->flags & F_HOSTS ? "H" : " ",
-			 cache->flags & F_CONFIG ? "C" : " ",
-			 cache->flags & F_DNSSECOK ? "V" : " ");
-#ifdef HAVE_BROKEN_RTC
-	    p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now));
-#else
-	    p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd)));
-#endif
-	    if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0)
-		p += sprintf(p, " %s", record_source(cache->uid));
-
-	    my_syslog(LOG_INFO, "%s", daemon->namebuff);
-	  }
+	  dump_cache_entry(cache, now);
     }
 }
 

src/dnsmasq.h

@@ -281,7 +281,8 @@ struct event_desc {
 #define OPT_STRIP_ECS      69
 #define OPT_STRIP_MAC      70
 #define OPT_NORR           71
-#define OPT_LAST           72
+#define OPT_NO_IDENT       72
+#define OPT_LAST           73
 
 #define OPTION_BITS (sizeof(unsigned int)*8)
 #define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )

src/option.c

@@ -185,6 +185,7 @@ struct myoption {
 #define LOPT_FAST_RETRY    376
 #define LOPT_STALE_CACHE   377
 #define LOPT_NORR          378
+#define LOPT_NO_IDENT      379
 
 #ifdef HAVE_GETOPT_LONG
 static const struct option opts[] =  
@@ -374,6 +375,7 @@ static const struct myoption opts[] =
     { "port-limit", 1, 0, LOPT_RANDPORT_LIM },
     { "fast-dns-retry", 2, 0, LOPT_FAST_RETRY },
     { "use-stale-cache", 2, 0 , LOPT_STALE_CACHE },
+    { "no-ident", 0, 0, LOPT_NO_IDENT },
     { NULL, 0, 0, 0 }
   };
 
@@ -570,6 +572,7 @@ static struct {
   { LOPT_UMBRELLA, ARG_ONE, "[=<optspec>]", gettext_noop("Send Cisco Umbrella identifiers including remote IP."), NULL },
   { LOPT_QUIET_TFTP, OPT_QUIET_TFTP, NULL, gettext_noop("Do not log routine TFTP."), NULL },
   { LOPT_NORR, OPT_NORR, NULL, gettext_noop("Suppress round-robin ordering of DNS records."), NULL },
+  { LOPT_NO_IDENT, OPT_NO_IDENT, NULL, gettext_noop("Do not add CHAOS TXT records."), NULL },
   { 0, 0, NULL, NULL, NULL }
 }; 
 
@@ -5757,21 +5760,6 @@ void read_opts(int argc, char **argv, char *compile_opts)
   daemon->randport_limit = 1;
   daemon->host_index = SRC_AH;
   
-#ifndef NO_ID
-  add_txt("version.bind", "dnsmasq-" VERSION, 0 );
-  add_txt("authors.bind", "Simon Kelley", 0);
-  add_txt("copyright.bind", COPYRIGHT, 0);
-  add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE);
-  add_txt("insertions.bind", NULL, TXT_STAT_INSERTS);
-  add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS);
-  add_txt("misses.bind", NULL, TXT_STAT_MISSES);
-  add_txt("hits.bind", NULL, TXT_STAT_HITS);
-#ifdef HAVE_AUTH
-  add_txt("auth.bind", NULL, TXT_STAT_AUTH);
-#endif
-  add_txt("servers.bind", NULL, TXT_STAT_SERVERS);
-#endif
-  
   /* See comment above make_servers(). Optimises server-read code. */
   mark_servers(0);
   
@@ -5869,6 +5857,25 @@ void read_opts(int argc, char **argv, char *compile_opts)
   else
     one_file(CONFFILE, LOPT_CONF_OPT);
 
+  /* Add TXT records if wanted */
+#ifndef NO_ID
+  if (!option_bool(OPT_NO_IDENT))
+    {
+      add_txt("version.bind", "dnsmasq-" VERSION, 0 );
+      add_txt("authors.bind", "Simon Kelley", 0);
+      add_txt("copyright.bind", COPYRIGHT, 0);
+      add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE);
+      add_txt("insertions.bind", NULL, TXT_STAT_INSERTS);
+      add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS);
+      add_txt("misses.bind", NULL, TXT_STAT_MISSES);
+      add_txt("hits.bind", NULL, TXT_STAT_HITS);
+#ifdef HAVE_AUTH
+      add_txt("auth.bind", NULL, TXT_STAT_AUTH);
+#endif
+      add_txt("servers.bind", NULL, TXT_STAT_SERVERS);
+    }
+#endif
+
   /* port might not be known when the address is parsed - fill in here */
   if (daemon->servers)
     {